On Sun, Feb 18, 2007 at 10:27:19PM -0800, Ben Greear wrote: > Jarek Poplawski wrote: > >On Fri, Feb 16, 2007 at 11:04:02AM -0800, Ben Greear wrote: > > > >>Stephen Hemminger wrote: > >> > >>>On Thu, 15 Feb 2007 23:40:32 -0800 > >>>Ben Greear <[EMAIL PROTECTED]> wrote: > >>> > >>>>Maybe there should be something like an ASSERT_NOT_RTNL() in the > >>>>flush_scheduled_work() > >>>>method? If it's performance criticial, #ifdef it out if we're not > >>>>debugging locks? > >>>> > >>>> > >>>You can't safely add a check like that. What if another cpu had acquired > >>>RTNL and was unrelated. > >>> > >>I guess there isn't a way to see if *this* thread is the owner of the RTNL > >>currently? I think lockdep knows the owner...maybe could query it > >>somehow, > >>or just save the owner in the mutex object when debugging is enabled... > >> > > > >Here is my patch proposal to enable such thing > >(and to make ASSERT_RTNL simpler btw.). > > > For performance reasons, I'd leave the rtnl_owner inside the > #if debugging locking code....
This is needed with my second patch. But it is only proposal, so all could be enhanced of course. But I don't thing current ASSERT_RTNL has anything to do with performance. And after all it's for mutex (slow) path, so I'm not sure if performance is such a problem. > You are also changing the semantics of ASSERT_RTNL (assert *this thread* > has rtnl, from the > old behaviour: assert *some thread* has rtnl). It may be better this > way, but it could break code that assumes the old behaviour. Sure, this should be verified. But this old behavior isn't very fast and reliable (there is a possibility, we are asserted wrongly because RTNL lock was held at the moment by somebody else). Cheers, Jarek P. - 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
