On Sat, 15 Dec 2007 14:10:21 +0800 Herbert Xu <[EMAIL PROTECTED]> wrote:
> On Fri, Dec 14, 2007 at 09:44:18PM -0800, Andrew Morton wrote: > > > > That sounds like a bug in mutex_trylock() to me. > > I was relying on > > http://kerneltrap.org/mailarchive/linux-netdev/2007/9/28/325129 > > which seems to be a bogus claim now that I actually look at the > source code. So in that case I'm OK with your patch as long as > it warns about hard IRQ usage. When Eric said > Way way deep in mutex debugging on the slowpath there is a unreadable > and incomprehensible WARN_ON in muxtex_trylock that will trigger if > you have 10 tons of debugging turned on, and you are in, > interrupt context, and you manage to hit the slow path. I think that > is a pretty unlikely scenario. I think he's still right. That's if the warning which he managed to find even still exists. I think the change which Eric proposed is a good one: it converts ASSERT_RTNL() from an atomic rmw which dirties a cacheline which will sometimes be owned by a different CPU into a plain old read. It's going to make ASSERT_RTNL() heaps cheaper. <looks at mutex_is_locked(), rofls at "static inline fastcall", fixes it> Now as a separate issue we (ie: you) need to work out what _other_ things you want ASSERT_RTNL to check apart from "rtnl must be held". If you want to check that no locks are held (which I think is a bit weird, but whatever) then add might_sleep(). If you want to check that we're not in interrupt context or whatever, then add the checks and be happy. might_sleep() will of course check for in_interrupt(), in_irq(), etc so if you go with a might_sleep() then nothing else needs to be added. While doing this I'd also suggest that the thing should be uninlined - it'll probably generate less text and it'll give considerably more flexibility for adding new debug fetures. Ones which might be controlled at compile time or runtime. ie: void __assert_rtnl(const char *file, int line); #define ASSERT_RTNL() __assert_rtnl(__FILE__, __LINE__) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
