Messages in this thread Patch in this message | | | Date | Fri, 16 Mar 2007 22:19:00 +0200 | From | Samuel Ortiz <> | Subject | Re: irda rmmod lockdep trace. |
| |
On Tue, Mar 13, 2007 at 07:22:37PM -0700, David Miller wrote: > From: Samuel Ortiz <samuel@sortiz.org> > Date: Wed, 14 Mar 2007 02:50:03 +0200 > > > On Mon, Mar 12, 2007 at 04:49:21PM -0700, David Miller wrote: > > > I would strongly caution against adding any run-time overhead just to > > > cure a false lockdep warning. Even adding a new function argument > > > is too much IMHO. > > > > > > Make the cost show up for lockdep only, perhaps by putting each > > > hashbin lock into a seperate locking class? > > Does that look better to you: > > Yes, it does.:) Unfortunately, it doesn't work, as the lock key is not on the stack. We get hit by the lockdep code checking if our lock key is static:
if (!static_obj(key)) { printk("BUG: key %p not in .data!\n", key); DEBUG_LOCKS_WARN_ON(1); return; }
So, instead, I propose the following, which does work, and adds runtime overhead only when LOCKDEP is enabled:
--- net/irda/irqueue.c | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/net/irda/irqueue.c b/net/irda/irqueue.c index 9266233..d058b46 100644 --- a/net/irda/irqueue.c +++ b/net/irda/irqueue.c @@ -384,6 +384,9 @@ EXPORT_SYMBOL(hashbin_new); * for deallocating this structure if it's complex. If not the user can * just supply kfree, which should take care of the job. */ +#ifdef CONFIG_LOCKDEP +static int hashbin_lock_depth = 0; +#endif int hashbin_delete( hashbin_t* hashbin, FREE_FUNC free_func) { irda_queue_t* queue; @@ -395,7 +398,8 @@ int hashbin_delete( hashbin_t* hashbin, FREE_FUNC free_func) /* Synchronize */ if ( hashbin->hb_type & HB_LOCK ) { - spin_lock_irqsave(&hashbin->hb_spinlock, flags); + spin_lock_irqsave_nested(&hashbin->hb_spinlock, flags, + hashbin_lock_depth++); } /* @@ -419,6 +423,9 @@ int hashbin_delete( hashbin_t* hashbin, FREE_FUNC free_func) /* Release lock */ if ( hashbin->hb_type & HB_LOCK) { spin_unlock_irqrestore(&hashbin->hb_spinlock, flags); +#ifdef CONFIG_LOCKDEP + hashbin_lock_depth--; +#endif } /* - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |