ipoib lockdep warning

ipoib lockdep warning

[Zach Brown]

I get an awfully verbose lockdep warning when I bring up ipoib devices
and wanted to make sure it didn't go unreported.  I've put it up at the
following URL:

  http://oss.oracle.com/~zab/ipoib-multicast-lockdep-warning.txt

I looked into it a bit and it seems to be on to something.  There might
be a AB, BC, CA ordering deadlock.

AB:

priv->lock is held while acquiring query_idr.lock

ipoib_mcast_send()
  spin_lock(&priv->lock);
  ipoib_mcast_sendonly_join()
    ib_sa_mcmember_rec_query()
      send_mad()
        idr_pre_get(&query_idr)
          spin_lock(&idp->lock);

BC:

query_idr.lock is taken with interrupts enabled and so is implicitly
ordered before dev->_xmit_lock which is taken in interrupt context.

ipoib_mcast_join_task()
  ipoib_mcast_join()
    ib_sa_mcmember_rec_query()
      send_mad()
        idr_pre_get(&query_idr)
          spin_lock(&idp->lock)

CA:

dev->_xmit_lock is held while acquiring priv->lock.  This triggers the
lockdep warning that adding the dep between dev->_xmit_lock and
priv->lock connects a soft-irq-safe lock to a soft-irq-unsafe one.

ipoib_mcast_restart_task()
  local_irq_save(flags);
  netif_tx_lock(dev)
    spin_lock(&dev->_xmit_lock);
  spin_lock(&priv->lock);

I can imagine all sorts of potential fixes (block ints when calling idr?
 reorder acquiry in ipoib_mcast_restart_task()?) but I'm operating on a
partial view of the paths here so I wasn't comfortable suggesting a fix.
 I wouldn't be surprised to hear that there are circumstances that both
lockdep and I don't know about that stop this from being a problem :).

In any case, it'd be fantastic if someone who knows this code could sit
down with lockdep and some ipoib regression suite to shake out lockdep's
complaints.

- z

Re: ipoib lockdep warning

[Michael S. Tsirkin]

Quoting r. Zach Brown <zach.brown@oracle.com>:
> BC:
> 
> query_idr.lock is taken with interrupts enabled and so is implicitly
> ordered before dev->_xmit_lock which is taken in interrupt context.
> 
> ipoib_mcast_join_task()
>   ipoib_mcast_join()
>     ib_sa_mcmember_rec_query()
>       send_mad()
>         idr_pre_get(&query_idr)
>           spin_lock(&idp->lock)

Got to check, but if that's true we have a simple deadlock here:
ib_sa_mcmember_rec_query might get called from interrupt
context as well, deadlocking on idp->lock?

Sean?

> I can imagine all sorts of potential fixes (block ints when calling idr?
>  reorder acquiry in ipoib_mcast_restart_task()?) but I'm operating on a
> partial view of the paths here so I wasn't comfortable suggesting a fix.
>  I wouldn't be surprised to hear that there are circumstances that both
> lockdep and I don't know about that stop this from being a problem :).

Awesome, thanks for the analysis! Your help is very much appreciated.

-- 
MST

RE: ipoib lockdep warning

[Sean Hefty]

>Quoting r. Zach Brown <zach.brown@oracle.com>:
>> BC:
>>
>> query_idr.lock is taken with interrupts enabled and so is implicitly
>> ordered before dev->_xmit_lock which is taken in interrupt context.
>>
>> ipoib_mcast_join_task()
>>   ipoib_mcast_join()
>>     ib_sa_mcmember_rec_query()
>>       send_mad()
>>         idr_pre_get(&query_idr)
>>           spin_lock(&idp->lock)
>
>Got to check, but if that's true we have a simple deadlock here:
>ib_sa_mcmember_rec_query might get called from interrupt
>context as well, deadlocking on idp->lock?
>
>Sean?

As a side note, I believe that this is the upstream code and does not include
the latest multicast changes.

I'm not sure if anything calls into the sa_query interfaces from interrupt
context, but I doubt it.  From my brief look at the initially reported issue, I
can't determine if there's an actual problem without studying the ipoib code
more.

- Sean

Re: [openib-general] ipoib lockdep warning

[Zach Brown]

> As a side note, I believe that this is the upstream code and does not include
> the latest multicast changes.

Indeed, I should have mentioned in my report that I was running 2.6.17-mm6.

- z

Re: [openib-general] ipoib lockdep warning

[Roland Dreier]

No time to really look at this in detail, but I think the issue is a
slightly bogus conversion to netif_tx_lock().  Can you try this patch
and see if things are better?

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
index ab40488..ddd1946 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
@@ -820,9 +820,8 @@ void ipoib_mcast_restart_task(void *dev_
 
 	ipoib_mcast_stop_thread(dev, 0);
 
-	local_irq_save(flags);
 	netif_tx_lock(dev);
-	spin_lock(&priv->lock);
+	spin_lock_irqsave(&priv->lock, flags);
 
 	/*
 	 * Unfortunately, the networking core only gives us a list of all of
@@ -893,9 +892,8 @@ void ipoib_mcast_restart_task(void *dev_
 		}
 	}
 
-	spin_unlock(&priv->lock);
+	spin_unlock_irqrestore(&priv->lock, flags);
 	netif_tx_unlock(dev);
-	local_irq_restore(flags);
 
 	/* We have to cancel outside of the spinlock */
 	list_for_each_entry_safe(mcast, tmcast, &remove_list, list) {

Re: [openib-general] ipoib lockdep warning

[Zach Brown]

Roland Dreier wrote:
> No time to really look at this in detail, but I think the issue is a
> slightly bogus conversion to netif_tx_lock().  Can you try this patch
> and see if things are better?

> -	local_irq_save(flags);
>  	netif_tx_lock(dev);
> -	spin_lock(&priv->lock);
> +	spin_lock_irqsave(&priv->lock, flags);

Hmm, won't that hold dev->_xmit_lock with interrupts enabled?  That
seems like it'd deadlock with dev_watchdog grabbing it from a softirq?

But setting that aside, I don't think this will help as it doesn't
change the order that the locks are acquired in this path.  This can
still be racing with other cpus that are each in the other two paths
(queue_idr.lock -> softirq -> dev->_xmit_lock, priv->lock ->
queue_idr.lock).  The local state of interrupt masking on this cpu
doesn't stop the other cpus from each grabbing the first lock in their
path and then trying to grab the second.  Imagine that they all race to
grab the first (A, B, C) and succeed and then all get stuck spinning on
their second lock (B, C, A).

Maybe you could get the priv->lock here before dev->_xmit_lock.  Then
we'd have AB, BC, AC, and that's OK.  I'm going to guess that this won't
work because other paths have dev->_xmit_lock -> priv->lock ordering.

Another possibility is masking interrupts when getting queue_idr.lock.
That drops the implicit dependency between queue_idr and _xmit_lock and
gives us AB, B, CA -- which is fine.  That means blocking ints while in
idr_pre_get() which allocs which leads to GFP_ATOMIC and more likely
allocation failure.

That's my reading, anyway.

- z

Re: [openib-general] ipoib lockdep warning

[Roland Dreier]

Hmm, good point.

It sort of seems to me like the idr interfaces are broken by design.
Internally, lib/idr.c uses bare spin_lock(&idp->lock) with no
interrupt disabling or anything in both the idr_pre_get() and
idr_get_new() code paths.  But idr_pre_get() is supposed to be called
in a context that can sleep, while idr_get_new() is supposed to be
called with locks held to serialize things (at least according to
http://lwn.net/Articles/103209/).

So, ugh... maybe the best thing to do is change lib/idr.c to use
spin_lock_irqsave() internally?

 - R.

Re: [openib-general] ipoib lockdep warning

[Zach Brown]

Roland Dreier wrote:
> Hmm, good point.
> 
> It sort of seems to me like the idr interfaces are broken by design.
> Internally, lib/idr.c uses bare spin_lock(&idp->lock) with no
> interrupt disabling or anything in both the idr_pre_get() and
> idr_get_new() code paths.

I wasn't thrilled to see that either.  We seem to have a fair precedent
(list.h, rbtree, etc) for leaving serialization to callers.

> So, ugh... maybe the best thing to do is change lib/idr.c to use
> spin_lock_irqsave() internally?

I dunno, it seems to have had _irq() locking in the past?  From the
comment at the top:

 * Modified by George Anzinger to reuse immediately and to use
 * find bit instructions.  Also removed _irq on spinlocks.

- z

Re: [openib-general] ipoib lockdep warning

[Roland Dreier]

 > > So, ugh... maybe the best thing to do is change lib/idr.c to use
 > > spin_lock_irqsave() internally?
 > 
 > I dunno, it seems to have had _irq() locking in the past?  From the
 > comment at the top:
 > 
 >  * Modified by George Anzinger to reuse immediately and to use
 >  * find bit instructions.  Also removed _irq on spinlocks.

Well, _irq would be no good, because we might want to call idr stuff
with interrupts disabled.  But making idr internally _irqsave seems
like the right fix to me.

I think the real issue here is that the sa_query.c stuff wants to use
the idr mechanism to assign "query ids", and other modules want to be
able to start queries from any context.  So if idr uses bare spin_lock
internally, then sa_query.c has no choice but to wrap all idr calls
inside spin_lock_irqsave and do all allocation with GFP_ATOMIC, which
doesn't seem very nice.

 - R.

Re: [openib-general] ipoib lockdep warning

[Ingo Molnar]

* Roland Dreier <rdreier@cisco.com> wrote:

> Hmm, good point.
> 
> It sort of seems to me like the idr interfaces are broken by design.
[...]

> So, ugh... maybe the best thing to do is change lib/idr.c to use 
> spin_lock_irqsave() internally?

i agree that the IDR subsystem should be irq-safe if GFP_ATOMIC is 
passed in. So the _irqsave()/_irqrestore() fix should be done.

But i also think that you should avoid using GFP_ATOMIC for any sort of 
reliable IO path and push as much work into process context as possible. 
Is it acceptable for your infiniband IO model to fail with -ENOMEM if 
GFP_ATOMIC happens to fail, and is the IO retried transparently?

	Ingo

Re: ipoib lockdep warning

[Michael S. Tsirkin]

Quoting r. Ingo Molnar <mingo@elte.hu>:
> But i also think that you should avoid using GFP_ATOMIC for any sort of 
> reliable IO path and push as much work into process context as possible. 
> Is it acceptable for your infiniband IO model to fail with -ENOMEM if 
> GFP_ATOMIC happens to fail, and is the IO retried transparently?

Yes, this is true for users that pass GFP_ATOMIC to sa_query, at least.  But
might not be so for other users:  send_mad in sa_query actually gets gfp_flags
parameter, but for some reason it does not pass it to idr_pre_get, which means
even sa query done with GFP_KERNEL flag is likely to fail.

Sean, it seems we need something like the following - what do you think?

--

Avoid bogus out out memory errors: fix sa_query to actually pass gfp_mask
supplied by the user to idr_pre_get.

Signed-off-by: Michael S. Tsirkin <mst@mellanox.co.il>

diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c
index e911c99..aeda484 100644
--- a/drivers/infiniband/core/sa_query.c
+++ b/drivers/infiniband/core/sa_query.c
@@ -488,13 +488,13 @@ static void init_mad(struct ib_sa_mad *m
 	spin_unlock_irqrestore(&tid_lock, flags);
 }
 
-static int send_mad(struct ib_sa_query *query, int timeout_ms)
+static int send_mad(struct ib_sa_query *query, int timeout_ms, gfp_t gfp_mask)
 {
 	unsigned long flags;
 	int ret, id;
 
 retry:
-	if (!idr_pre_get(&query_idr, GFP_ATOMIC))
+	if (!idr_pre_get(&query_idr, gfp_mask))
 		return -ENOMEM;
 	spin_lock_irqsave(&idr_lock, flags);
 	ret = idr_get_new(&query_idr, query, &id);
@@ -630,7 +630,7 @@ int ib_sa_path_rec_get(struct ib_device 
 
 	*sa_query = &query->sa_query;
 
-	ret = send_mad(&query->sa_query, timeout_ms);
+	ret = send_mad(&query->sa_query, timeout_ms, gfp_mask);
 	if (ret < 0)
 		goto err2;
 
@@ -752,7 +752,7 @@ int ib_sa_service_rec_query(struct ib_de
 
 	*sa_query = &query->sa_query;
 
-	ret = send_mad(&query->sa_query, timeout_ms);
+	ret = send_mad(&query->sa_query, timeout_ms, gfp_mask);
 	if (ret < 0)
 		goto err2;
 
@@ -844,7 +844,7 @@ int ib_sa_mcmember_rec_query(struct ib_d
 
 	*sa_query = &query->sa_query;
 
-	ret = send_mad(&query->sa_query, timeout_ms);
+	ret = send_mad(&query->sa_query, timeout_ms, gfp_mask);
 	if (ret < 0)
 		goto err2;
 


-- 
MST

Re: [openib-general] ipoib lockdep warning

[Sean Hefty]

Michael S. Tsirkin wrote:
> Yes, this is true for users that pass GFP_ATOMIC to sa_query, at least.  But
> might not be so for other users:  send_mad in sa_query actually gets gfp_flags
> parameter, but for some reason it does not pass it to idr_pre_get, which means
> even sa query done with GFP_KERNEL flag is likely to fail.
> 
> Sean, it seems we need something like the following - what do you think?

I noticed this same thing looking at the code yesterday.  I can't think of any 
reason why your patch wouldn't work.

- Sean

Re: ipoib lockdep warning

[Roland Dreier]

 > Avoid bogus out out memory errors: fix sa_query to actually pass gfp_mask
 > supplied by the user to idr_pre_get.

Yes, this looks right to me.

 - R.

[PATCH] IB/core: use correct gfp_mask in sa_query

[Michael S. Tsirkin]

Andrew, could you please drop the following in -mm and on to Linus?

--

Avoid bogus out of memory errors: fix sa_query to actually pass gfp_mask
supplied by the user to idr_pre_get.

Signed-off-by: Michael S. Tsirkin <mst@mellanox.co.il>
Acked-by: "Sean Hefty" <mshefty@ichips.intel.com>
Acked-by: "Roland Dreier" <rdreier@cisco.com>

diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c
index e911c99..aeda484 100644
--- a/drivers/infiniband/core/sa_query.c
+++ b/drivers/infiniband/core/sa_query.c
@@ -488,13 +488,13 @@ static void init_mad(struct ib_sa_mad *m
 	spin_unlock_irqrestore(&tid_lock, flags);
 }
 
-static int send_mad(struct ib_sa_query *query, int timeout_ms)
+static int send_mad(struct ib_sa_query *query, int timeout_ms, gfp_t gfp_mask)
 {
 	unsigned long flags;
 	int ret, id;
 
 retry:
-	if (!idr_pre_get(&query_idr, GFP_ATOMIC))
+	if (!idr_pre_get(&query_idr, gfp_mask))
 		return -ENOMEM;
 	spin_lock_irqsave(&idr_lock, flags);
 	ret = idr_get_new(&query_idr, query, &id);
@@ -630,7 +630,7 @@ int ib_sa_path_rec_get(struct ib_device 
 
 	*sa_query = &query->sa_query;
 
-	ret = send_mad(&query->sa_query, timeout_ms);
+	ret = send_mad(&query->sa_query, timeout_ms, gfp_mask);
 	if (ret < 0)
 		goto err2;
 
@@ -752,7 +752,7 @@ int ib_sa_service_rec_query(struct ib_de
 
 	*sa_query = &query->sa_query;
 
-	ret = send_mad(&query->sa_query, timeout_ms);
+	ret = send_mad(&query->sa_query, timeout_ms, gfp_mask);
 	if (ret < 0)
 		goto err2;
 
@@ -844,7 +844,7 @@ int ib_sa_mcmember_rec_query(struct ib_d
 
 	*sa_query = &query->sa_query;
 
-	ret = send_mad(&query->sa_query, timeout_ms);
+	ret = send_mad(&query->sa_query, timeout_ms, gfp_mask);
 	if (ret < 0)
 		goto err2;
 
-- 
MST

Re: [openib-general] ipoib lockdep warning

[Roland Dreier]

 > i agree that the IDR subsystem should be irq-safe if GFP_ATOMIC is 
 > passed in. So the _irqsave()/_irqrestore() fix should be done.

OK, I will send the idr change to Andrew.

 > But i also think that you should avoid using GFP_ATOMIC for any sort of 
 > reliable IO path and push as much work into process context as possible. 
 > Is it acceptable for your infiniband IO model to fail with -ENOMEM if 
 > GFP_ATOMIC happens to fail, and is the IO retried transparently?

Yes, I think it's OK.  This idr use is in an inherently unreliable
path.  With that said, as Michael pointed out, we can change things to
use GFP_ATOMIC less.

Thanks,
  Roland

[PATCH] Convert idr's internal locking to _irqsave variant

[Roland Dreier]

Currently, the code in lib/idr.c uses a bare spin_lock(&idp->lock) to
do internal locking.  This is a nasty trap for code that might call
idr functions from different contexts; for example, it seems perfectly
reasonable to call idr_get_new() from process context and idr_remove()
from interrupt context -- but with the current locking this would lead
to a potential deadlock.

The simplest fix for this is to just convert the idr locking to use
spin_lock_irqsave().

In particular, this fixes a very complicated locking issue detected by
lockdep, involving the ib_ipoib driver's priv->lock and dev->_xmit_lock,
which get involved with the ib_sa module's query_idr.lock.

Cc: Arjan van de Ven <arjan@infradead.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Zach Brown <zach.brown@oracle.com>,
Signed-off-by: Roland Dreier <rolandd@cisco.com>

---

diff --git a/lib/idr.c b/lib/idr.c
index 4d09681..16d2143 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -38,14 +38,15 @@ static kmem_cache_t *idr_layer_cache;
 static struct idr_layer *alloc_layer(struct idr *idp)
 {
 	struct idr_layer *p;
+	unsigned long flags;
 
-	spin_lock(&idp->lock);
+	spin_lock_irqsave(&idp->lock, flags);
 	if ((p = idp->id_free)) {
 		idp->id_free = p->ary[0];
 		idp->id_free_cnt--;
 		p->ary[0] = NULL;
 	}
-	spin_unlock(&idp->lock);
+	spin_unlock_irqrestore(&idp->lock, flags);
 	return(p);
 }
 
@@ -59,12 +60,14 @@ static void __free_layer(struct idr *idp
 
 static void free_layer(struct idr *idp, struct idr_layer *p)
 {
+	unsigned long flags;
+
 	/*
 	 * Depends on the return element being zeroed.
 	 */
-	spin_lock(&idp->lock);
+	spin_lock_irqsave(&idp->lock, flags);
 	__free_layer(idp, p);
-	spin_unlock(&idp->lock);
+	spin_unlock_irqrestore(&idp->lock, flags);
 }
 
 /**
@@ -168,6 +171,7 @@ static int idr_get_new_above_int(struct 
 {
 	struct idr_layer *p, *new;
 	int layers, v, id;
+	unsigned long flags;
 
 	id = starting_id;
 build_up:
@@ -191,14 +195,14 @@ build_up:
 			 * The allocation failed.  If we built part of
 			 * the structure tear it down.
 			 */
-			spin_lock(&idp->lock);
+			spin_lock_irqsave(&idp->lock, flags);
 			for (new = p; p && p != idp->top; new = p) {
 				p = p->ary[0];
 				new->ary[0] = NULL;
 				new->bitmap = new->count = 0;
 				__free_layer(idp, new);
 			}
-			spin_unlock(&idp->lock);
+			spin_unlock_irqrestore(&idp->lock, flags);
 			return -1;
 		}
 		new->ary[0] = p;

Re: [PATCH] Convert idr's internal locking to _irqsave variant

[Ingo Molnar]

* Roland Dreier <rdreier@cisco.com> wrote:

> Currently, the code in lib/idr.c uses a bare spin_lock(&idp->lock) to 
> do internal locking.  This is a nasty trap for code that might call 
> idr functions from different contexts; for example, it seems perfectly 
> reasonable to call idr_get_new() from process context and idr_remove() 
> from interrupt context -- but with the current locking this would lead 
> to a potential deadlock.
> 
> The simplest fix for this is to just convert the idr locking to use 
> spin_lock_irqsave().
> 
> In particular, this fixes a very complicated locking issue detected by 
> lockdep, involving the ib_ipoib driver's priv->lock and 
> dev->_xmit_lock, which get involved with the ib_sa module's 
> query_idr.lock.
> 
> Cc: Arjan van de Ven <arjan@infradead.org>
> Cc: Ingo Molnar <mingo@elte.hu>

Acked-by: Ingo Molnar <mingo@elte.hu>

	Ingo

Re: [PATCH] Convert idr's internal locking to _irqsave variant

[Andrew Morton]

On Wed, 12 Jul 2006 13:45:12 -0700
Roland Dreier <rdreier@cisco.com> wrote:

> Currently, the code in lib/idr.c uses a bare spin_lock(&idp->lock) to
> do internal locking.  This is a nasty trap for code that might call
> idr functions from different contexts; for example, it seems perfectly
> reasonable to call idr_get_new() from process context and idr_remove()
> from interrupt context -- but with the current locking this would lead
> to a potential deadlock.
> 
> The simplest fix for this is to just convert the idr locking to use
> spin_lock_irqsave().
> 
> In particular, this fixes a very complicated locking issue detected by
> lockdep, involving the ib_ipoib driver's priv->lock and dev->_xmit_lock,
> which get involved with the ib_sa module's query_idr.lock.

Sigh.  It was always a mistake (of the kernel programming 101 type) to put
any locking at all in the idr code.  At some stage we need to weed it all
out and move it to callers.

Your fix is yet more fallout from that mistake.

Re: [PATCH] Convert idr's internal locking to _irqsave variant

[Roland Dreier]

 > Sigh.  It was always a mistake (of the kernel programming 101 type) to put
 > any locking at all in the idr code.  At some stage we need to weed it all
 > out and move it to callers.
 > 
 > Your fix is yet more fallout from that mistake.

Agreed.  Consider me on the hook to fix this up in a better way once
my life is a little saner.  Maybe I'll try to cook something up on the
plane ride to Ottawa.

Anyway you can punch me in the stomach if I don't have something in
time for 2.6.19.

 - R.

Re: [PATCH] Convert idr's internal locking to _irqsave variant

[Andrew Morton]

On Thu, 13 Jul 2006 08:42:47 -0700
Roland Dreier <rdreier@cisco.com> wrote:

>  > Sigh.  It was always a mistake (of the kernel programming 101 type) to put
>  > any locking at all in the idr code.  At some stage we need to weed it all
>  > out and move it to callers.
>  > 
>  > Your fix is yet more fallout from that mistake.
> 
> Agreed.  Consider me on the hook to fix this up in a better way once
> my life is a little saner.  Maybe I'll try to cook something up on the
> plane ride to Ottawa.
> 

I suspect it'll get really ugly.  It's a container library which needs to
allocate memory when items are added, like the radix-tree.  Either it needs
to assume GFP_ATOMIC, which is bad and can easily fail or it does weird
things like radix_tree_preload().

The basic problem is:

	idr_pre_get(GFP_KERNEL);
	spin_lock(my_lock);
	idr_get_new(..);

which is racy, because some other CPU could have got in there and consumed
some of the pool which was precharged by idr_pre_get().

It's wildly improbable that it'll actually fail.  It requires all of:

a) that the race occur

b) that the racing thread consume an appreciable amount of the pool

c) that this thread also consume an appreciable amount (such that the
   total of both exceeds the pool size).

d) that a (needs to be added) GFP_ATOMIC attempt to replenish the pool
   inside idr_get_new() fails.

Re: [PATCH] Convert idr's internal locking to _irqsave variant

[Roland Dreier]

 > I suspect it'll get really ugly.  It's a container library which needs to
 > allocate memory when items are added, like the radix-tree.  Either it needs
 > to assume GFP_ATOMIC, which is bad and can easily fail or it does weird
 > things like radix_tree_preload().

Actually I don't think it has to be too bad.  We could tweak the
interface a little bit so that consumers do something like:

	struct idr_layer *layer = NULL;	/* opaque */

retry:
        spin_lock(&my_idr_lock);
	ret = idr_get_new(&my_idr, ptr, &id, layer);
        spin_unlock(&my_idr_lock);

        if (ret == -EAGAIN) {
		layer = idr_alloc_layer(&my_idr, GFP_KERNEL);
		if (!IS_ERR(layer))
			goto retry;
	}

in other words make the consumer responsible for passing in new memory
that can be used for a new entry (or freed if other entries have
become free in the meantime).

 - R.

Re: [PATCH] Convert idr's internal locking to _irqsave variant

[Arjan van de Ven]

On Thu, 2006-07-13 at 14:03 -0700, Roland Dreier wrote:
>  > I suspect it'll get really ugly.  It's a container library which needs to
>  > allocate memory when items are added, like the radix-tree.  Either it needs
>  > to assume GFP_ATOMIC, which is bad and can easily fail or it does weird
>  > things like radix_tree_preload().
> 
> Actually I don't think it has to be too bad.  We could tweak the
> interface a little bit so that consumers do something like:
> 
> 	

it does get harder if this is needed for your IB device to do 
more work, so that your swap device on your IB can take more IO's
to free up ram..

Re: [PATCH] Convert idr's internal locking to _irqsave variant

[Roland Dreier]

    Arjan> it does get harder if this is needed for your IB device to
    Arjan> do more work, so that your swap device on your IB can take
    Arjan> more IO's to free up ram..

That's the classic problem, but it's more a matter of the consumer
using GFP_NOIO in the right places.

 - R.

Re: [PATCH] Convert idr's internal locking to _irqsave variant

[Arjan van de Ven]

On Thu, 2006-07-13 at 17:18 -0700, Roland Dreier wrote:
>     Arjan> it does get harder if this is needed for your IB device to
>     Arjan> do more work, so that your swap device on your IB can take
>     Arjan> more IO's to free up ram..
> 
> That's the classic problem, but it's more a matter of the consumer
> using GFP_NOIO in the right places.

GFP_NOIO isn't going to save you in the cases where the memory really is
running low and you need the memory to do more IO...

Re: [PATCH] Convert idr's internal locking to _irqsave variant

[Andrew Morton]

On Thu, 13 Jul 2006 14:03:21 -0700
Roland Dreier <rdreier@cisco.com> wrote:

>  > I suspect it'll get really ugly.  It's a container library which needs to
>  > allocate memory when items are added, like the radix-tree.  Either it needs
>  > to assume GFP_ATOMIC, which is bad and can easily fail or it does weird
>  > things like radix_tree_preload().
> 
> Actually I don't think it has to be too bad.  We could tweak the
> interface a little bit so that consumers do something like:
> 
> 	struct idr_layer *layer = NULL;	/* opaque */
> 
> retry:
>         spin_lock(&my_idr_lock);
> 	ret = idr_get_new(&my_idr, ptr, &id, layer);
>         spin_unlock(&my_idr_lock);
> 
>         if (ret == -EAGAIN) {
> 		layer = idr_alloc_layer(&my_idr, GFP_KERNEL);
> 		if (!IS_ERR(layer))
> 			goto retry;
> 	}
> 
> in other words make the consumer responsible for passing in new memory
> that can be used for a new entry (or freed if other entries have
> become free in the meantime).
> 

Good point, a try-again loop would work.  Do we really need the caller to
maintain a cache?  I suspect something like

drat:
	if (idr_pre_get(GFP_KERNEL) == ENOMEM)
		give_up();
	spin_lock();
	ret = idr_get_new();
	spin_unlock();
	if (ret == ENOMEM)
		goto drat;

would do it.

Re: [PATCH] Convert idr's internal locking to _irqsave variant

[Roland Dreier]

 > Good point, a try-again loop would work.  Do we really need the caller to
 > maintain a cache?  I suspect something like
 > 
 > drat:
 > 	if (idr_pre_get(GFP_KERNEL) == ENOMEM)
 > 		give_up();
 > 	spin_lock();
 > 	ret = idr_get_new();
 > 	spin_unlock();
 > 	if (ret == ENOMEM)
 > 		goto drat;
 > 
 > would do it.

The problem (for my tiny brain at least) is that I don't know where
idr_pre_get() can put the memory it allocates if there's no lock in
the idr structure -- how do you maintain internal consistency if no
locks are held when filling the cache?

Having the caller hold a chunk of memory in a stack variable was the
trick I came up with to get around that.

 - R.

Re: [PATCH] Convert idr's internal locking to _irqsave variant

[Andrew Morton]

On Thu, 13 Jul 2006 18:08:17 -0700
Roland Dreier <rdreier@cisco.com> wrote:

>  > Good point, a try-again loop would work.  Do we really need the caller to
>  > maintain a cache?  I suspect something like
>  > 
>  > drat:
>  > 	if (idr_pre_get(GFP_KERNEL) == ENOMEM)
>  > 		give_up();
>  > 	spin_lock();
>  > 	ret = idr_get_new();
>  > 	spin_unlock();
>  > 	if (ret == ENOMEM)
>  > 		goto drat;
>  > 
>  > would do it.
> 
> The problem (for my tiny brain at least) is that I don't know where
> idr_pre_get() can put the memory it allocates if there's no lock in
> the idr structure -- how do you maintain internal consistency if no
> locks are held when filling the cache?

argh.  Aren't you supposed to be on vacation or something?

> Having the caller hold a chunk of memory in a stack variable was the
> trick I came up with to get around that.

Yes, that certainly works.

Re: [PATCH] Convert idr's internal locking to _irqsave variant

[Andrew Morton]

On Thu, 13 Jul 2006 18:18:35 -0700
Andrew Morton <akpm@osdl.org> wrote:

> > Having the caller hold a chunk of memory in a stack variable was the
> > trick I came up with to get around that.
> 
> Yes, that certainly works.


Problem is, I think, you'll need to preallocate IDR_FREE_MAX items.  And
then free them all again when none of them were consumed (usual).

Yes, storing the preallocated nodes in the idr itself requires locking. 
But that locking is 100% private to the IDR implementation.  It locks only
the preload list and not the user's stuff.

radix_tree_preload() effectively does this.  Except the preload list is
kernel-wide.  It's split across CPUs and uses
local_irq_disable/preempt_disable locking tricks as a performance
optimisation.  But conceptually it's the same.

Simply copying that would give something which is known to work...  It
seems like a large amount of fuss, but when you think about it the problem
isn't simple.

Re: [PATCH] Convert idr's internal locking to _irqsave variant

[Roland Dreier]

 > Problem is, I think, you'll need to preallocate IDR_FREE_MAX items.  And
 > then free them all again when none of them were consumed (usual).

Actually I think it's OK if we just pass in no more than one extra
layer for each try to add something with idr_get_new().  In the worst
case, this leads to a lot of extra calls to idr_get_new(), but in the
usual case it's fine.

I'm including a lightly tested big patch with all my idr changes for
comments -- I'll split it up into a form more suitable for merging.
(Some of the changes are unrelated and obviously good, eg using
kmem_cache_zalloc() instead of a slab cache with a constructor that
does memset(0)).

I'm not sure I'm thrilled with this approach, but it does seem to be a
net win.  With an allyesconfig with debugging options turned off (so
spinlocks shrink back down to 8 bytes), I get the following:

   text		   data	    bss	    dec		    hex	filename
24347759	5971210	2322176	32641145	1f21079	vmlinux.old
24347370	5970474	2320704	32638548	1f20654	vmlinux.new

Most of the savings comes from ocfs2, which has a static array of
255 structures that each contain an idr -- so removing the lock from
struct idr saves 255 * 8 = 2040 bytes.  However, even without
factoring that in, this does seem to be a net win:

add/remove: 2/4 grow/shrink: 23/51 up/down: 719/-3215 (-2496)
function                                     old     new   delta
idr_get_new_above                             38     554    +516
dm_create                                    957    1000     +43
ipath_init_one                              3294    3329     +35
get_layer                                      -      32     +32
idr_alloc_layer                                -      16     +16
sd_probe                                     871     881     +10
rtc_device_register                          493     503     +10
proc_register                                277     286      +9
mmc_add_host_sysfs                           126     135      +9
idr_add_uobj                                  80      85      +5
cma_alloc_port                               224     229      +5
sys_timer_create                             876     880      +4
set_anon_super                               173     177      +4
sctp_process_init                           1312    1316      +4
ib_ucm_ctx_alloc                             197     201      +4
ib_create_cm_id                              287     290      +3
proc_mkdir_mode                               95      97      +2
vfs_kern_mount                               279     280      +1
send_mad                                     325     326      +1
proc_symlink                                 141     142      +1
proc_file_write                               40      41      +1
kill_block_super                              56      57      +1
get_sb_single                                175     176      +1
free_proc_entry                              108     109      +1
deactivate_super                             126     127      +1
proc_readdir                                 353     352      -1
proc_getattr                                  40      39      -1
get_sb_nodev                                 150     149      -1
o2net_send_message_vec                      2032    2030      -2
hwmon_device_register                        198     196      -2
create_proc_entry                            170     168      -2
__put_super_and_need_restart                  54      52      -2
v9fs_read_work                              1424    1421      -3
v9fs_mux_init                               1333    1330      -3
v9fs_mux_flush_cb                            303     300      -3
v9fs_mux_destroy                             369     366      -3
v9fs_mux_cancel                              382     379      -3
o2net_init                                   441     438      -3
inotify_add_watch                            285     280      -5
v9fs_session_init                           1490    1484      -6
unnamed_dev_idr                               32      24      -8
unit_table                                    32      24      -8
tcp_ps                                        32      24      -8
sdp_ps                                        32      24      -8
sd_index_idr                                  32      24      -8
sctp_assocs_id                                32      24      -8
rtc_idr                                       32      24      -8
query_idr                                     32      24      -8
proc_inum_idr                                 32      24      -8
posix_timers_id                               32      24      -8
mmc_host_idr                                  32      24      -8
lpfc_hba_index                                32      24      -8
ib_uverbs_srq_idr                             32      24      -8
ib_uverbs_qp_idr                              32      24      -8
ib_uverbs_pd_idr                              32      24      -8
ib_uverbs_mw_idr                              32      24      -8
ib_uverbs_mr_idr                              32      24      -8
ib_uverbs_cq_idr                              32      24      -8
ib_uverbs_ah_idr                              32      24      -8
i2c_adapter_idr                               32      24      -8
hwmon_idr                                     32      24      -8
ctx_id_table                                  32      24      -8
cm                                           112     104      -8
allocated_ptys                                32      24      -8
_minor_idr                                    32      24      -8
i2c_add_adapter                              494     484     -10
idr_cache_ctor                                12       -     -12
v9fs_get_idpool                              149     134     -15
ib_cm_init                                   237     220     -17
free_layer                                    55      34     -21
idr_init                                      88      62     -26
idr_get_new                                   40      13     -27
idr_remove                                   357     328     -29
infinipath_init                              227     197     -30
lpfc_pci_probe_one                          2752    2710     -42
ptmx_open                                    427     379     -48
idr_pre_get                                   59       -     -59
alloc_layer                                   66       -     -66
idr_get_new_above_int                        533       -    -533
o2net_nodes                                99960   97920   -2040

---

diff --git a/arch/powerpc/mm/mmu_context_64.c b/arch/powerpc/mm/mmu_context_64.c
index 90a06ac..85d6a03 100644
--- a/arch/powerpc/mm/mmu_context_64.c
+++ b/arch/powerpc/mm/mmu_context_64.c
@@ -26,20 +26,21 @@ static DEFINE_IDR(mmu_context_idr);
 
 int init_new_context(struct task_struct *tsk, struct mm_struct *mm)
 {
+	struct idr_layer *layer = NULL;
 	int index;
 	int err;
 
 again:
-	if (!idr_pre_get(&mmu_context_idr, GFP_KERNEL))
-		return -ENOMEM;
-
 	spin_lock(&mmu_context_lock);
-	err = idr_get_new_above(&mmu_context_idr, NULL, 1, &index);
+	err = idr_get_new_above(&mmu_context_idr, NULL, 1, &index, layer);
 	spin_unlock(&mmu_context_lock);
 
-	if (err == -EAGAIN)
+	if (err == -EAGAIN) {
+		layer = idr_alloc_layer(GFP_KERNEL);
+		if (!layer)
+			return -ENOMEM;
 		goto again;
-	else if (err)
+	} else if (err)
 		return err;
 
 	if (index > MAX_CONTEXT) {
diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index bfdb902..0ae099e 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -2135,6 +2135,7 @@ #ifdef CONFIG_UNIX98_PTYS
 static int ptmx_open(struct inode * inode, struct file * filp)
 {
 	struct tty_struct *tty;
+	struct idr_layer *layer = NULL;
 	int retval;
 	int index;
 	int idr_ret;
@@ -2143,24 +2144,27 @@ static int ptmx_open(struct inode * inod
 
 	/* find a device that is not in use. */
 	down(&allocated_ptys_lock);
-	if (!idr_pre_get(&allocated_ptys, GFP_KERNEL)) {
-		up(&allocated_ptys_lock);
-		return -ENOMEM;
-	}
-	idr_ret = idr_get_new(&allocated_ptys, NULL, &index);
-	if (idr_ret < 0) {
-		up(&allocated_ptys_lock);
-		if (idr_ret == -EAGAIN)
-			return -ENOMEM;
-		return -EIO;
-	}
-	if (index >= pty_limit) {
-		idr_remove(&allocated_ptys, index);
-		up(&allocated_ptys_lock);
-		return -EIO;
-	}
+	do {
+		idr_ret = idr_get_new(&allocated_ptys, NULL, &index, layer);
+		if (idr_ret == -EAGAIN) {
+			layer = idr_alloc_layer(GFP_KERNEL);
+			if (!layer) {
+				idr_ret = -ENOMEM;
+				break;
+			}
+			continue;
+		}
+
+		if (index >= pty_limit) {
+			idr_remove(&allocated_ptys, index);
+			idr_ret = -EIO;
+		}
+	} while (idr_ret == -EAGAIN);
 	up(&allocated_ptys_lock);
 
+	if (idr_ret)
+		return idr_ret == -EAGAIN ? -ENOMEM : -EIO;
+
 	mutex_lock(&tty_mutex);
 	retval = init_dev(ptm_driver, index, &tty);
 	mutex_unlock(&tty_mutex);
diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
index 106fa01..82d6d04 100644
--- a/drivers/hwmon/hwmon.c
+++ b/drivers/hwmon/hwmon.c
@@ -38,20 +38,21 @@ static DEFINE_SPINLOCK(idr_lock);
  */
 struct class_device *hwmon_device_register(struct device *dev)
 {
+	struct idr_layer *layer = NULL;
 	struct class_device *cdev;
 	int id, err;
 
 again:
-	if (unlikely(idr_pre_get(&hwmon_idr, GFP_KERNEL) == 0))
-		return ERR_PTR(-ENOMEM);
-
 	spin_lock(&idr_lock);
-	err = idr_get_new(&hwmon_idr, NULL, &id);
+	err = idr_get_new(&hwmon_idr, NULL, &id, layer);
 	spin_unlock(&idr_lock);
 
-	if (unlikely(err == -EAGAIN))
+	if (unlikely(err == -EAGAIN)) {
+		layer = idr_alloc_layer(GFP_KERNEL);
+		if (!layer)
+			return ERR_PTR(-ENOMEM);
 		goto again;
-	else if (unlikely(err))
+	} else if (unlikely(err))
 		return ERR_PTR(err);
 
 	id = id & MAX_ID_MASK;
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 9cb277d..f8aa8ea 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -151,22 +151,24 @@ static struct device_attribute dev_attr_
 int i2c_add_adapter(struct i2c_adapter *adap)
 {
 	int id, res = 0;
+	struct idr_layer   *layer = NULL;
 	struct list_head   *item;
 	struct i2c_driver  *driver;
 
 	mutex_lock(&core_lists);
 
-	if (idr_pre_get(&i2c_adapter_idr, GFP_KERNEL) == 0) {
-		res = -ENOMEM;
-		goto out_unlock;
-	}
-
-	res = idr_get_new(&i2c_adapter_idr, adap, &id);
-	if (res < 0) {
-		if (res == -EAGAIN)
+again:
+	res = idr_get_new(&i2c_adapter_idr, adap, &id, layer);
+	if (res == -EAGAIN) {
+		layer = idr_alloc_layer(GFP_KERNEL);
+		if (!layer) {
 			res = -ENOMEM;
-		goto out_unlock;
+			goto out_unlock;
+		}
+		goto again;
 	}
+	if (res < 0)
+		goto out_unlock;
 
 	adap->nr =  id & MAX_ID_MASK;
 	mutex_init(&adap->bus_lock);
diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index f85c97f..cf14d01 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -298,6 +298,7 @@ static int cm_init_av_by_path(struct ib_
 
 static int cm_alloc_id(struct cm_id_private *cm_id_priv)
 {
+	struct idr_layer *layer = NULL;
 	unsigned long flags;
 	int ret;
 	static int next_id;
@@ -305,9 +306,15 @@ static int cm_alloc_id(struct cm_id_priv
 	do {
 		spin_lock_irqsave(&cm.lock, flags);
 		ret = idr_get_new_above(&cm.local_id_table, cm_id_priv, next_id++,
-					(__force int *) &cm_id_priv->id.local_id);
+					(__force int *) &cm_id_priv->id.local_id,
+					layer);
 		spin_unlock_irqrestore(&cm.lock, flags);
-	} while( (ret == -EAGAIN) && idr_pre_get(&cm.local_id_table, GFP_KERNEL) );
+		if (ret == -EAGAIN) {
+			layer = idr_alloc_layer(GFP_KERNEL);
+			if (!layer)
+				ret = -ENOMEM;
+		}
+	} while (ret == -EAGAIN);
 	return ret;
 }
 
@@ -3347,7 +3354,6 @@ static int __init ib_cm_init(void)
 	cm.remote_qp_table = RB_ROOT;
 	cm.remote_sidr_table = RB_ROOT;
 	idr_init(&cm.local_id_table);
-	idr_pre_get(&cm.local_id_table, GFP_KERNEL);
 
 	cm.wq = create_workqueue("ib_cm");
 	if (!cm.wq)
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index d6f99d5..314b150 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -1433,6 +1433,7 @@ static void cma_bind_port(struct rdma_bi
 static int cma_alloc_port(struct idr *ps, struct rdma_id_private *id_priv,
 			  unsigned short snum)
 {
+	struct idr_layer *layer = NULL;
 	struct rdma_bind_list *bind_list;
 	int port, start, ret;
 
@@ -1443,8 +1444,13 @@ static int cma_alloc_port(struct idr *ps
 	start = snum ? snum : sysctl_local_port_range[0];
 
 	do {
-		ret = idr_get_new_above(ps, bind_list, start, &port);
-	} while ((ret == -EAGAIN) && idr_pre_get(ps, GFP_KERNEL));
+		ret = idr_get_new_above(ps, bind_list, start, &port, layer);
+		if (ret == -EAGAIN) {
+			layer = idr_alloc_layer(GFP_KERNEL);
+			if (!layer)
+				ret = -ENOMEM;
+		}
+	} while (ret == -EAGAIN);
 
 	if (ret)
 		goto err;
diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c
index aeda484..824b652 100644
--- a/drivers/infiniband/core/sa_query.c
+++ b/drivers/infiniband/core/sa_query.c
@@ -490,17 +490,20 @@ static void init_mad(struct ib_sa_mad *m
 
 static int send_mad(struct ib_sa_query *query, int timeout_ms, gfp_t gfp_mask)
 {
+	struct idr_layer *layer = NULL;
 	unsigned long flags;
 	int ret, id;
 
 retry:
-	if (!idr_pre_get(&query_idr, gfp_mask))
-		return -ENOMEM;
 	spin_lock_irqsave(&idr_lock, flags);
-	ret = idr_get_new(&query_idr, query, &id);
+	ret = idr_get_new(&query_idr, query, &id, layer);
 	spin_unlock_irqrestore(&idr_lock, flags);
-	if (ret == -EAGAIN)
-		goto retry;
+	if (ret == -EAGAIN) {
+		layer = idr_alloc_layer(gfp_mask);
+		if (layer)
+			goto retry;
+		ret = -ENOMEM;
+	}
 	if (ret)
 		return ret;
 
diff --git a/drivers/infiniband/core/ucm.c b/drivers/infiniband/core/ucm.c
index c1c6fda..22d5e24 100644
--- a/drivers/infiniband/core/ucm.c
+++ b/drivers/infiniband/core/ucm.c
@@ -173,6 +173,7 @@ static void ib_ucm_cleanup_events(struct
 
 static struct ib_ucm_context *ib_ucm_ctx_alloc(struct ib_ucm_file *file)
 {
+	struct idr_layer *layer = NULL;
 	struct ib_ucm_context *ctx;
 	int result;
 
@@ -186,13 +187,15 @@ static struct ib_ucm_context *ib_ucm_ctx
 	INIT_LIST_HEAD(&ctx->events);
 
 	do {
-		result = idr_pre_get(&ctx_id_table, GFP_KERNEL);
-		if (!result)
-			goto error;
-
 		mutex_lock(&ctx_id_mutex);
-		result = idr_get_new(&ctx_id_table, ctx, &ctx->id);
+		result = idr_get_new(&ctx_id_table, ctx, &ctx->id, layer);
 		mutex_unlock(&ctx_id_mutex);
+
+		if (result == -EAGAIN) {
+			layer = idr_alloc_layer(GFP_KERNEL);
+			if (!layer)
+				result = -ENOMEM;
+		}
 	} while (result == -EAGAIN);
 
 	if (result)
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index bdf5d50..71dea88 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -109,18 +109,20 @@ static void put_uobj_write(struct ib_uob
 
 static int idr_add_uobj(struct idr *idr, struct ib_uobject *uobj)
 {
+	struct idr_layer *layer = NULL;
 	int ret;
 
-retry:
-	if (!idr_pre_get(idr, GFP_KERNEL))
-		return -ENOMEM;
+	do {
+		spin_lock(&ib_uverbs_idr_lock);
+		ret = idr_get_new(idr, uobj, &uobj->id, layer);
+		spin_unlock(&ib_uverbs_idr_lock);
 
-	spin_lock(&ib_uverbs_idr_lock);
-	ret = idr_get_new(idr, uobj, &uobj->id);
-	spin_unlock(&ib_uverbs_idr_lock);
-
-	if (ret == -EAGAIN)
-		goto retry;
+		if (ret == -EAGAIN) {
+			layer = idr_alloc_layer(GFP_KERNEL);
+			if (!layer)
+				ret = -ENOMEM;
+		}
+	} while (ret == -EAGAIN);
 
 	return ret;
 }
diff --git a/drivers/infiniband/hw/ipath/ipath_driver.c b/drivers/infiniband/hw/ipath/ipath_driver.c
index 823131d..6f2a711 100644
--- a/drivers/infiniband/hw/ipath/ipath_driver.c
+++ b/drivers/infiniband/hw/ipath/ipath_driver.c
@@ -168,15 +168,11 @@ static void ipath_free_devdata(struct pc
 
 static struct ipath_devdata *ipath_alloc_devdata(struct pci_dev *pdev)
 {
+	struct idr_layer *layer = NULL;
 	unsigned long flags;
 	struct ipath_devdata *dd;
 	int ret;
 
-	if (!idr_pre_get(&unit_table, GFP_KERNEL)) {
-		dd = ERR_PTR(-ENOMEM);
-		goto bail;
-	}
-
 	dd = vmalloc(sizeof(*dd));
 	if (!dd) {
 		dd = ERR_PTR(-ENOMEM);
@@ -187,7 +183,19 @@ static struct ipath_devdata *ipath_alloc
 
 	spin_lock_irqsave(&ipath_devs_lock, flags);
 
-	ret = idr_get_new(&unit_table, dd, &dd->ipath_unit);
+	do {
+		ret = idr_get_new(&unit_table, dd, &dd->ipath_unit, layer);
+		if (ret == -EAGAIN) {
+			spin_unlock_irqrestore(&ipath_devs_lock, flags);
+			layer = idr_alloc_layer(GFP_KERNEL);
+			if (!layer) {
+				dd = ERR_PTR(-ENOMEM);
+				goto bail;
+			}
+			spin_lock_irqsave(&ipath_devs_lock, flags);
+		}
+	} while (ret == -EAGAIN);
+
 	if (ret < 0) {
 		printk(KERN_ERR IPATH_DRV_NAME
 		       ": Could not allocate unit ID: error %d\n", -ret);
@@ -1754,10 +1762,6 @@ static int __init infinipath_init(void)
 	 * the PCI subsystem.
 	 */
 	idr_init(&unit_table);
-	if (!idr_pre_get(&unit_table, GFP_KERNEL)) {
-		ret = -ENOMEM;
-		goto bail;
-	}
 
 	ret = pci_register_driver(&ipath_driver);
 	if (ret < 0) {
@@ -1780,7 +1784,7 @@ static int __init infinipath_init(void)
 		goto bail_group;
 	}
 
-	goto bail;
+	return ret;
 
 bail_group:
 	ipath_driver_remove_group(&ipath_driver.driver);
@@ -1791,7 +1795,6 @@ bail_pci:
 bail_unit:
 	idr_destroy(&unit_table);
 
-bail:
 	return ret;
 }
 
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index c99bf9f..e2dd20a 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -807,23 +807,30 @@ static void free_minor(int minor)
  */
 static int specific_minor(struct mapped_device *md, int minor)
 {
+	struct idr_layer *layer = NULL;
 	int r, m;
 
 	if (minor >= (1 << MINORBITS))
 		return -EINVAL;
 
-	r = idr_pre_get(&_minor_idr, GFP_KERNEL);
-	if (!r)
-		return -ENOMEM;
-
 	spin_lock(&_minor_lock);
 
+again:
 	if (idr_find(&_minor_idr, minor)) {
 		r = -EBUSY;
 		goto out;
 	}
 
-	r = idr_get_new_above(&_minor_idr, MINOR_ALLOCED, minor, &m);
+	r = idr_get_new_above(&_minor_idr, MINOR_ALLOCED, minor, &m, layer);
+	if (r == -EAGAIN) {
+		spin_unlock(&_minor_lock);
+		layer = idr_alloc_layer(GFP_KERNEL);
+		if (!layer)
+			return -ENOMEM;
+
+		spin_lock(&_minor_lock);
+		goto again;
+	}
 	if (r)
 		goto out;
 
@@ -840,18 +847,21 @@ out:
 
 static int next_free_minor(struct mapped_device *md, int *minor)
 {
+	struct idr_layer *layer = NULL;
 	int r, m;
 
-	r = idr_pre_get(&_minor_idr, GFP_KERNEL);
-	if (!r)
-		return -ENOMEM;
-
+again:
 	spin_lock(&_minor_lock);
-
-	r = idr_get_new(&_minor_idr, MINOR_ALLOCED, &m);
-	if (r) {
-		goto out;
+	r = idr_get_new(&_minor_idr, MINOR_ALLOCED, &m, layer);
+	if (r == -EAGAIN) {
+		spin_unlock(&_minor_lock);
+		layer = idr_alloc_layer(GFP_KERNEL);
+		if (!layer)
+			return -ENOMEM;
+		goto again;
 	}
+	if (r)
+		goto out;
 
 	if (m >= (1 << MINORBITS)) {
 		idr_remove(&_minor_idr, m);
diff --git a/drivers/mmc/mmc_sysfs.c b/drivers/mmc/mmc_sysfs.c
index a2a35fd..bf61ff4 100644
--- a/drivers/mmc/mmc_sysfs.c
+++ b/drivers/mmc/mmc_sysfs.c
@@ -280,14 +280,19 @@ struct mmc_host *mmc_alloc_host_sysfs(in
  */
 int mmc_add_host_sysfs(struct mmc_host *host)
 {
+	struct idr_layer *layer = NULL;
 	int err;
 
-	if (!idr_pre_get(&mmc_host_idr, GFP_KERNEL))
-		return -ENOMEM;
-
+again:
 	spin_lock(&mmc_host_lock);
-	err = idr_get_new(&mmc_host_idr, host, &host->index);
+	err = idr_get_new(&mmc_host_idr, host, &host->index, layer);
 	spin_unlock(&mmc_host_lock);
+	if (err == -EAGAIN) {
+		layer = idr_alloc_layer(GFP_KERNEL);
+		if (!layer)
+			return -ENOMEM;
+		goto again;
+	}
 	if (err)
 		return err;
 
diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
index 1cb61a7..3388059 100644
--- a/drivers/rtc/class.c
+++ b/drivers/rtc/class.c
@@ -42,19 +42,21 @@ struct rtc_device *rtc_device_register(c
 					struct rtc_class_ops *ops,
 					struct module *owner)
 {
+	struct idr_layer *layer = NULL;
 	struct rtc_device *rtc;
 	int id, err;
 
-	if (idr_pre_get(&rtc_idr, GFP_KERNEL) == 0) {
-		err = -ENOMEM;
-		goto exit;
-	}
-
-
+again:
 	mutex_lock(&idr_lock);
-	err = idr_get_new(&rtc_idr, NULL, &id);
+	err = idr_get_new(&rtc_idr, NULL, &id, layer);
 	mutex_unlock(&idr_lock);
 
+	if (err == -EAGAIN) {
+		layer = idr_alloc_layer(GFP_KERNEL);
+		if (!layer)
+			return ERR_PTR(-ENOMEM);
+		goto again;
+	}
 	if (err < 0)
 		goto exit;
 
diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index 81755a3..401608b 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -1421,6 +1421,7 @@ lpfc_pci_probe_one(struct pci_dev *pdev,
 	struct lpfc_hba  *phba;
 	struct lpfc_sli  *psli;
 	struct lpfc_iocbq *iocbq_entry = NULL, *iocbq_next = NULL;
+	struct idr_layer *layer = NULL;
 	unsigned long bar0map_len, bar2map_len;
 	int error = -ENODEV, retval;
 	int i;
@@ -1443,10 +1444,16 @@ lpfc_pci_probe_one(struct pci_dev *pdev,
 	phba->pcidev = pdev;
 
 	/* Assign an unused board number */
-	if (!idr_pre_get(&lpfc_hba_index, GFP_KERNEL))
-		goto out_put_host;
-
-	error = idr_get_new(&lpfc_hba_index, NULL, &phba->brd_no);
+again:
+	error = idr_get_new(&lpfc_hba_index, NULL, &phba->brd_no, layer);
+	if (error == -EAGAIN) {
+		layer = idr_alloc_layer(GFP_KERNEL);
+		if (!layer) {
+			error = -ENOMEM;
+			goto out_put_host;
+		}
+		goto again;
+	}
 	if (error)
 		goto out_put_host;
 
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 3225d31..fe504be 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1617,6 +1617,7 @@ static int sd_probe(struct device *dev)
 	struct scsi_device *sdp = to_scsi_device(dev);
 	struct scsi_disk *sdkp;
 	struct gendisk *gd;
+	struct idr_layer *layer = NULL;
 	u32 index;
 	int error;
 
@@ -1636,13 +1637,19 @@ static int sd_probe(struct device *dev)
 	if (!gd)
 		goto out_free;
 
-	if (!idr_pre_get(&sd_index_idr, GFP_KERNEL))
-		goto out_put;
-
+again:
 	spin_lock(&sd_index_lock);
-	error = idr_get_new(&sd_index_idr, NULL, &index);
+	error = idr_get_new(&sd_index_idr, NULL, &index, layer);
 	spin_unlock(&sd_index_lock);
 
+	if (error == -EAGAIN) {
+		layer = idr_alloc_layer(GFP_KERNEL);
+		if (!layer) {
+			error = -ENOMEM;
+			goto out_put;
+		}
+		goto again;
+	}
 	if (index >= SD_MAX_DISKS)
 		error = -EBUSY;
 	if (error)
diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
index 22f7ccd..d03c3ba 100644
--- a/fs/9p/v9fs.c
+++ b/fs/9p/v9fs.c
@@ -198,25 +198,26 @@ struct v9fs_session_info *v9fs_inode2v9s
 
 int v9fs_get_idpool(struct v9fs_idpool *p)
 {
+	struct idr_layer *layer = NULL;
 	int i = 0;
 	int error;
 
 retry:
-	if (idr_pre_get(&p->pool, GFP_KERNEL) == 0)
-		return 0;
-
 	if (down_interruptible(&p->lock) == -EINTR) {
 		eprintk(KERN_WARNING, "Interrupted while locking\n");
 		return -1;
 	}
 
 	/* no need to store exactly p, we just need something non-null */
-	error = idr_get_new(&p->pool, p, &i);
+	error = idr_get_new(&p->pool, p, &i, layer);
 	up(&p->lock);
 
-	if (error == -EAGAIN)
+	if (error == -EAGAIN) {
+		layer = idr_alloc_layer(GFP_KERNEL);
+		if (!layer)
+			return -1;
 		goto retry;
-	else if (error)
+	} else if (error)
 		return -1;
 
 	return i;
diff --git a/fs/inotify.c b/fs/inotify.c
index 723836a..7e12bed 100644
--- a/fs/inotify.c
+++ b/fs/inotify.c
@@ -131,12 +131,16 @@ EXPORT_SYMBOL_GPL(put_inotify_watch);
 static int inotify_handle_get_wd(struct inotify_handle *ih,
 				 struct inotify_watch *watch)
 {
+	struct idr_layer *layer = NULL;
 	int ret;
 
 	do {
-		if (unlikely(!idr_pre_get(&ih->idr, GFP_KERNEL)))
-			return -ENOSPC;
-		ret = idr_get_new_above(&ih->idr, watch, ih->last_wd+1, &watch->wd);
+		ret = idr_get_new_above(&ih->idr, watch, ih->last_wd+1, &watch->wd, layer);
+		if (ret == -EAGAIN) {
+			layer = idr_alloc_layer(GFP_KERNEL);
+			if (!layer)
+				ret = -ENOSPC;
+		}
 	} while (ret == -EAGAIN);
 
 	if (likely(!ret))
diff --git a/fs/ocfs2/cluster/tcp.c b/fs/ocfs2/cluster/tcp.c
index b650efa..f9d1b04 100644
--- a/fs/ocfs2/cluster/tcp.c
+++ b/fs/ocfs2/cluster/tcp.c
@@ -175,19 +175,21 @@ static u8 o2net_num_from_nn(struct o2net
 
 static int o2net_prep_nsw(struct o2net_node *nn, struct o2net_status_wait *nsw)
 {
+	struct idr_layer *layer = NULL;
 	int ret = 0;
 
 	do {
-		if (!idr_pre_get(&nn->nn_status_idr, GFP_ATOMIC)) {
-			ret = -EAGAIN;
-			break;
-		}
 		spin_lock(&nn->nn_lock);
-		ret = idr_get_new(&nn->nn_status_idr, nsw, &nsw->ns_id);
+		ret = idr_get_new(&nn->nn_status_idr, nsw, &nsw->ns_id, layer);
 		if (ret == 0)
 			list_add_tail(&nsw->ns_node_item,
 				      &nn->nn_status_list);
 		spin_unlock(&nn->nn_lock);
+		if (ret == -EAGAIN) {
+			layer = idr_alloc_layer(GFP_ATOMIC);
+			if (!layer)
+				ret = -ENOMEM;
+		}
 	} while (ret == -EAGAIN);
 
 	if (ret == 0)  {
diff --git a/fs/proc/generic.c b/fs/proc/generic.c
index 4ba0300..60bdd42 100644
--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -318,19 +318,20 @@ #define PROC_DYNAMIC_FIRST 0xF0000000UL
  */
 static unsigned int get_inode_number(void)
 {
+	struct idr_layer *layer = NULL;
 	int i, inum = 0;
 	int error;
 
 retry:
-	if (idr_pre_get(&proc_inum_idr, GFP_KERNEL) == 0)
-		return 0;
-
 	spin_lock(&proc_inum_lock);
-	error = idr_get_new(&proc_inum_idr, NULL, &i);
+	error = idr_get_new(&proc_inum_idr, NULL, &i, layer);
 	spin_unlock(&proc_inum_lock);
-	if (error == -EAGAIN)
+	if (error == -EAGAIN) {
+		layer = idr_alloc_layer(GFP_KERNEL);
+		if (!layer)
+			return 0;
 		goto retry;
-	else if (error)
+	} else if (error)
 		return 0;
 
 	inum = (i & MAX_ID_MASK) + PROC_DYNAMIC_FIRST;
diff --git a/fs/super.c b/fs/super.c
index 6d4e817..67455f5 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -607,19 +607,20 @@ static DEFINE_SPINLOCK(unnamed_dev_lock)
 
 int set_anon_super(struct super_block *s, void *data)
 {
+	struct idr_layer *layer = NULL;
 	int dev;
 	int error;
 
  retry:
-	if (idr_pre_get(&unnamed_dev_idr, GFP_ATOMIC) == 0)
-		return -ENOMEM;
 	spin_lock(&unnamed_dev_lock);
-	error = idr_get_new(&unnamed_dev_idr, NULL, &dev);
+	error = idr_get_new(&unnamed_dev_idr, NULL, &dev, layer);
 	spin_unlock(&unnamed_dev_lock);
-	if (error == -EAGAIN)
-		/* We raced and lost with another CPU. */
+	if (error == -EAGAIN) {
+		layer = idr_alloc_layer(GFP_ATOMIC);
+		if (!layer)
+			return -ENOMEM;
 		goto retry;
-	else if (error)
+	} else if (error)
 		return -EAGAIN;
 
 	if ((dev & MAX_ID_MASK) == (1 << MINORBITS)) {
diff --git a/include/linux/idr.h b/include/linux/idr.h
index 8268034..de34c4e 100644
--- a/include/linux/idr.h
+++ b/include/linux/idr.h
@@ -44,7 +44,7 @@ #define MAX_ID_MASK (MAX_ID_BIT - 1)
 #define MAX_LEVEL (MAX_ID_SHIFT + IDR_BITS - 1) / IDR_BITS
 
 /* Number of id_layer structs to leave in free list */
-#define IDR_FREE_MAX MAX_LEVEL + MAX_LEVEL
+#define IDR_FREE_MAX (MAX_LEVEL + MAX_LEVEL)
 
 struct idr_layer {
 	unsigned long		 bitmap; /* A zero bit means "space here" */
@@ -57,7 +57,6 @@ struct idr {
 	struct idr_layer *id_free;
 	int		  layers;
 	int		  id_free_cnt;
-	spinlock_t	  lock;
 };
 
 #define IDR_INIT(name)						\
@@ -66,7 +65,6 @@ #define IDR_INIT(name)						\
 	.id_free	= NULL,					\
 	.layers 	= 0,					\
 	.id_free_cnt	= 0,					\
-	.lock		= __SPIN_LOCK_UNLOCKED(name.lock),	\
 }
 #define DEFINE_IDR(name)	struct idr name = IDR_INIT(name)
 
@@ -75,9 +73,10 @@ #define DEFINE_IDR(name)	struct idr name
  */
 
 void *idr_find(struct idr *idp, int id);
-int idr_pre_get(struct idr *idp, gfp_t gfp_mask);
-int idr_get_new(struct idr *idp, void *ptr, int *id);
-int idr_get_new_above(struct idr *idp, void *ptr, int starting_id, int *id);
+struct idr_layer *idr_alloc_layer(gfp_t gfp_mask);
+int idr_get_new(struct idr *idp, void *ptr, int *id, struct idr_layer *layer);
+int idr_get_new_above(struct idr *idp, void *ptr, int starting_id, int *id,
+		      struct idr_layer *layer);
 void *idr_replace(struct idr *idp, void *ptr, int id);
 void idr_remove(struct idr *idp, int id);
 void idr_destroy(struct idr *idp);
diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
index ac6dc87..18891b2 100644
--- a/kernel/posix-timers.c
+++ b/kernel/posix-timers.c
@@ -434,6 +434,7 @@ sys_timer_create(const clockid_t which_c
 		 struct sigevent __user *timer_event_spec,
 		 timer_t __user * created_timer_id)
 {
+	struct idr_layer *layer = NULL;
 	int error = 0;
 	struct k_itimer *new_timer = NULL;
 	int new_timer_id;
@@ -451,17 +452,16 @@ sys_timer_create(const clockid_t which_c
 
 	spin_lock_init(&new_timer->it_lock);
  retry:
-	if (unlikely(!idr_pre_get(&posix_timers_id, GFP_KERNEL))) {
-		error = -EAGAIN;
-		goto out;
-	}
 	spin_lock_irq(&idr_lock);
 	error = idr_get_new(&posix_timers_id, (void *) new_timer,
-			    &new_timer_id);
+			    &new_timer_id, layer);
 	spin_unlock_irq(&idr_lock);
-	if (error == -EAGAIN)
-		goto retry;
-	else if (error) {
+	if (error == -EAGAIN) {
+		layer = idr_alloc_layer(GFP_KERNEL);
+		if (layer)
+			goto retry;
+	}
+	if (error) {
 		/*
 		 * Wierd looking, but we return EAGAIN if the IDR is
 		 * full (proper POSIX return value for this)
diff --git a/lib/idr.c b/lib/idr.c
index 16d2143..187ce4d 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -35,65 +35,39 @@ #include <linux/idr.h>
 
 static kmem_cache_t *idr_layer_cache;
 
-static struct idr_layer *alloc_layer(struct idr *idp)
+static struct idr_layer *get_layer(struct idr *idp)
 {
 	struct idr_layer *p;
-	unsigned long flags;
 
-	spin_lock_irqsave(&idp->lock, flags);
-	if ((p = idp->id_free)) {
+	p = idp->id_free;
+	if (p) {
 		idp->id_free = p->ary[0];
 		idp->id_free_cnt--;
 		p->ary[0] = NULL;
 	}
-	spin_unlock_irqrestore(&idp->lock, flags);
-	return(p);
-}
 
-/* only called when idp->lock is held */
-static void __free_layer(struct idr *idp, struct idr_layer *p)
-{
-	p->ary[0] = idp->id_free;
-	idp->id_free = p;
-	idp->id_free_cnt++;
+	return p;
 }
 
 static void free_layer(struct idr *idp, struct idr_layer *p)
 {
-	unsigned long flags;
-
-	/*
-	 * Depends on the return element being zeroed.
-	 */
-	spin_lock_irqsave(&idp->lock, flags);
-	__free_layer(idp, p);
-	spin_unlock_irqrestore(&idp->lock, flags);
+	if (idp->id_free_cnt < IDR_FREE_MAX) {
+		p->ary[0] = idp->id_free;
+		idp->id_free = p;
+		idp->id_free_cnt++;
+	} else
+		kmem_cache_free(idr_layer_cache, p);
 }
 
 /**
- * idr_pre_get - reserver resources for idr allocation
- * @idp:	idr handle
+ * idr_alloc_layer - reserve resources for idr allocation
  * @gfp_mask:	memory allocation flags
- *
- * This function should be called prior to locking and calling the
- * following function.  It preallocates enough memory to satisfy
- * the worst possible allocation.
- *
- * If the system is REALLY out of memory this function returns 0,
- * otherwise 1.
  */
-int idr_pre_get(struct idr *idp, gfp_t gfp_mask)
+struct idr_layer *idr_alloc_layer(gfp_t gfp_mask)
 {
-	while (idp->id_free_cnt < IDR_FREE_MAX) {
-		struct idr_layer *new;
-		new = kmem_cache_alloc(idr_layer_cache, gfp_mask);
-		if (new == NULL)
-			return (0);
-		free_layer(idp, new);
-	}
-	return 1;
+	return kmem_cache_zalloc(idr_layer_cache, gfp_mask);
 }
-EXPORT_SYMBOL(idr_pre_get);
+EXPORT_SYMBOL(idr_alloc_layer);
 
 static int sub_alloc(struct idr *idp, void *ptr, int *starting_id)
 {
@@ -136,7 +110,7 @@ static int sub_alloc(struct idr *idp, vo
 		 * Create the layer below if it is missing.
 		 */
 		if (!p->ary[m]) {
-			if (!(new = alloc_layer(idp)))
+			if (!(new = get_layer(idp)))
 				return -1;
 			p->ary[m] = new;
 			p->count++;
@@ -171,14 +145,13 @@ static int idr_get_new_above_int(struct 
 {
 	struct idr_layer *p, *new;
 	int layers, v, id;
-	unsigned long flags;
 
 	id = starting_id;
 build_up:
 	p = idp->top;
 	layers = idp->layers;
 	if (unlikely(!p)) {
-		if (!(p = alloc_layer(idp)))
+		if (!(p = get_layer(idp)))
 			return -1;
 		layers = 1;
 	}
@@ -190,19 +163,17 @@ build_up:
 		layers++;
 		if (!p->count)
 			continue;
-		if (!(new = alloc_layer(idp))) {
+		if (!(new = get_layer(idp))) {
 			/*
 			 * The allocation failed.  If we built part of
 			 * the structure tear it down.
 			 */
-			spin_lock_irqsave(&idp->lock, flags);
 			for (new = p; p && p != idp->top; new = p) {
 				p = p->ary[0];
 				new->ary[0] = NULL;
 				new->bitmap = new->count = 0;
-				__free_layer(idp, new);
+				free_layer(idp, new);
 			}
-			spin_unlock_irqrestore(&idp->lock, flags);
 			return -1;
 		}
 		new->ary[0] = p;
@@ -216,7 +187,7 @@ build_up:
 	v = sub_alloc(idp, ptr, &id);
 	if (v == -2)
 		goto build_up;
-	return(v);
+	return v;
 }
 
 /**
@@ -225,20 +196,25 @@ build_up:
  * @ptr: pointer you want associated with the ide
  * @start_id: id to start search at
  * @id: pointer to the allocated handle
+ * @layer: pointer to extra storage
  *
  * This is the allocate id function.  It should be called with any
  * required locks.
  *
  * If memory is required, it will return -EAGAIN, you should unlock
- * and go back to the idr_pre_get() call.  If the idr is full, it will
- * return -ENOSPC.
+ * and call idr_alloc_layer() to get a new layer to pass in as the
+ * @layer parameter.  If the idr is full, it will return -ENOSPC.
  *
  * @id returns a value in the range 0 ... 0x7fffffff
  */
-int idr_get_new_above(struct idr *idp, void *ptr, int starting_id, int *id)
+int idr_get_new_above(struct idr *idp, void *ptr, int starting_id, int *id,
+		      struct idr_layer *layer)
 {
 	int rv;
 
+	if (layer)
+		free_layer(idp, layer);
+
 	rv = idr_get_new_above_int(idp, ptr, starting_id);
 	/*
 	 * This is a cheap hack until the IDR code can be fixed to
@@ -260,33 +236,20 @@ EXPORT_SYMBOL(idr_get_new_above);
  * @idp: idr handle
  * @ptr: pointer you want associated with the ide
  * @id: pointer to the allocated handle
+ * @layer: pointer to extra storage
  *
  * This is the allocate id function.  It should be called with any
  * required locks.
  *
  * If memory is required, it will return -EAGAIN, you should unlock
- * and go back to the idr_pre_get() call.  If the idr is full, it will
- * return -ENOSPC.
+ * and call idr_alloc_layer() to get a new layer to pass in as the
+ * @layer parameter.  If the idr is full, it will return -ENOSPC.
  *
  * @id returns a value in the range 0 ... 0x7fffffff
  */
-int idr_get_new(struct idr *idp, void *ptr, int *id)
+int idr_get_new(struct idr *idp, void *ptr, int *id, struct idr_layer *layer)
 {
-	int rv;
-
-	rv = idr_get_new_above_int(idp, ptr, 0);
-	/*
-	 * This is a cheap hack until the IDR code can be fixed to
-	 * return proper error values.
-	 */
-	if (rv < 0) {
-		if (rv == -1)
-			return -EAGAIN;
-		else /* Will be -3 */
-			return -ENOSPC;
-	}
-	*id = rv;
-	return 0;
+	return idr_get_new_above(idp, ptr, 0, id, layer);
 }
 EXPORT_SYMBOL(idr_get_new);
 
@@ -349,11 +312,6 @@ void idr_remove(struct idr *idp, int id)
 		idp->top = p;
 		--idp->layers;
 	}
-	while (idp->id_free_cnt >= IDR_FREE_MAX) {
-		p = alloc_layer(idp);
-		kmem_cache_free(idr_layer_cache, p);
-		return;
-	}
 }
 EXPORT_SYMBOL(idr_remove);
 
@@ -364,7 +322,7 @@ EXPORT_SYMBOL(idr_remove);
 void idr_destroy(struct idr *idp)
 {
 	while (idp->id_free_cnt) {
-		struct idr_layer *p = alloc_layer(idp);
+		struct idr_layer *p = get_layer(idp);
 		kmem_cache_free(idr_layer_cache, p);
 	}
 }
@@ -445,17 +403,11 @@ void *idr_replace(struct idr *idp, void 
 }
 EXPORT_SYMBOL(idr_replace);
 
-static void idr_cache_ctor(void * idr_layer, kmem_cache_t *idr_layer_cache,
-		unsigned long flags)
-{
-	memset(idr_layer, 0, sizeof(struct idr_layer));
-}
-
 static  int init_id_cache(void)
 {
 	if (!idr_layer_cache)
 		idr_layer_cache = kmem_cache_create("idr_layer_cache",
-			sizeof(struct idr_layer), 0, 0, idr_cache_ctor, NULL);
+			sizeof(struct idr_layer), 0, 0, NULL, NULL);
 	return 0;
 }
 
@@ -470,6 +422,5 @@ void idr_init(struct idr *idp)
 {
 	init_id_cache();
 	memset(idp, 0, sizeof(struct idr));
-	spin_lock_init(&idp->lock);
 }
 EXPORT_SYMBOL(idr_init);
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 2a87736..fc712c4 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -1928,6 +1928,7 @@ int sctp_process_init(struct sctp_associ
  	 * association.
 	 */
 	if (!asoc->temp) {
+		struct idr_layer *layer = NULL;
 		int assoc_id;
 		int error;
 
@@ -1937,15 +1938,16 @@ int sctp_process_init(struct sctp_associ
 			goto clean_up;
 
 	retry:
-		if (unlikely(!idr_pre_get(&sctp_assocs_id, gfp)))
-			goto clean_up;
 		spin_lock_bh(&sctp_assocs_id_lock);
 		error = idr_get_new_above(&sctp_assocs_id, (void *)asoc, 1,
-					  &assoc_id);
+					  &assoc_id, layer);
 		spin_unlock_bh(&sctp_assocs_id_lock);
-		if (error == -EAGAIN)
-			goto retry;
-		else if (error)
+		if (error == -EAGAIN) {
+			layer = idr_alloc_layer(gfp);
+			if (layer)
+				goto retry;
+		}
+		if (error)
 			goto clean_up;
 
 		asoc->assoc_id = (sctp_assoc_t) assoc_id;

Re: [openib-general] ipoib lockdep warning

[Roland Dreier]

OK, the patch to lib/idr.c below is at least one way to fix this.
However, with that applied I get the lockdep warning below, which
seems to be a false positive -- I'm not sure what the right fix is,
but the blame really seems to fall on udp_ioctl for poking into the
sk_buff_head lock itself.

Ingo and/or Arjan, any thoughts on the idr.c change or the
sk_buff_head warning?

Thanks,
  Roland


Here's the idr.c change:

diff --git a/lib/idr.c b/lib/idr.c
index 4d09681..16d2143 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -38,14 +38,15 @@ static kmem_cache_t *idr_layer_cache;
 static struct idr_layer *alloc_layer(struct idr *idp)
 {
 	struct idr_layer *p;
+	unsigned long flags;
 
-	spin_lock(&idp->lock);
+	spin_lock_irqsave(&idp->lock, flags);
 	if ((p = idp->id_free)) {
 		idp->id_free = p->ary[0];
 		idp->id_free_cnt--;
 		p->ary[0] = NULL;
 	}
-	spin_unlock(&idp->lock);
+	spin_unlock_irqrestore(&idp->lock, flags);
 	return(p);
 }
 
@@ -59,12 +60,14 @@ static void __free_layer(struct idr *idp
 
 static void free_layer(struct idr *idp, struct idr_layer *p)
 {
+	unsigned long flags;
+
 	/*
 	 * Depends on the return element being zeroed.
 	 */
-	spin_lock(&idp->lock);
+	spin_lock_irqsave(&idp->lock, flags);
 	__free_layer(idp, p);
-	spin_unlock(&idp->lock);
+	spin_unlock_irqrestore(&idp->lock, flags);
 }
 
 /**
@@ -168,6 +171,7 @@ static int idr_get_new_above_int(struct 
 {
 	struct idr_layer *p, *new;
 	int layers, v, id;
+	unsigned long flags;
 
 	id = starting_id;
 build_up:
@@ -191,14 +195,14 @@ build_up:
 			 * The allocation failed.  If we built part of
 			 * the structure tear it down.
 			 */
-			spin_lock(&idp->lock);
+			spin_lock_irqsave(&idp->lock, flags);
 			for (new = p; p && p != idp->top; new = p) {
 				p = p->ary[0];
 				new->ary[0] = NULL;
 				new->bitmap = new->count = 0;
 				__free_layer(idp, new);
 			}
-			spin_unlock(&idp->lock);
+			spin_unlock_irqrestore(&idp->lock, flags);
 			return -1;
 		}
 		new->ary[0] = p;


And here's the warning I get, which appears to be a false positive:

======================================================
[ INFO: hard-safe -> hard-unsafe lock order detected ]
------------------------------------------------------
swapper/0 [HC0[0]:SC1[2]:HE0:SE0] is trying to acquire:
 (&skb_queue_lock_key){-+..}, at: [<ffffffff8038683e>] skb_queue_tail+0x1d/0x47

and this task is already holding:
 (&priv->lock){.+..}, at: [<ffffffff881efd5b>] ipoib_mcast_send+0x29/0x413 [ib_ipoib]
which would create a new lock dependency:
 (&priv->lock){.+..} -> (&skb_queue_lock_key){-+..}

but this new dependency connects a hard-irq-safe lock:
 (&priv->tx_lock){+...}
... which became hard-irq-safe at:
  [<ffffffff8024172a>] lock_acquire+0x4a/0x69
  [<ffffffff803eb7da>] _spin_lock_irqsave+0x2b/0x3c
  [<ffffffff881ee615>] ipoib_ib_completion+0x340/0x3ff [ib_ipoib]
  [<ffffffff881ca8a5>] mthca_cq_completion+0x65/0x6b [ib_mthca]
  [<ffffffff881ca1bc>] mthca_eq_int+0x70/0x3d2 [ib_mthca]
  [<ffffffff881ca644>] mthca_arbel_interrupt+0x3b/0x98 [ib_mthca]
  [<ffffffff802537de>] handle_IRQ_event+0x28/0x64
  [<ffffffff802538c6>] __do_IRQ+0xac/0x117
  [<ffffffff8020becf>] do_IRQ+0xf7/0x108
  [<ffffffff80209bd8>] common_interrupt+0x64/0x65

to a hard-irq-unsafe lock:
 (&skb_queue_lock_key){-+..}
... which became hard-irq-unsafe at:
...  [<ffffffff8024172a>] lock_acquire+0x4a/0x69
  [<ffffffff803eb349>] _spin_lock_bh+0x26/0x33
  [<ffffffff803c6de8>] udp_ioctl+0x46/0x87
  [<ffffffff803cd1ab>] inet_ioctl+0x8c/0x8f
  [<ffffffff8038203c>] sock_ioctl+0x1c0/0x1ea
  [<ffffffff8028591e>] do_ioctl+0x26/0x74
  [<ffffffff80285bb6>] vfs_ioctl+0x24a/0x264
  [<ffffffff80285c11>] sys_ioctl+0x41/0x68
  [<ffffffff802096a1>] system_call+0x7d/0x83

other info that might help us debug this:

2 locks held by swapper/0:
 #0:  (&priv->tx_lock){+...}, at: [<ffffffff881ed6ef>] ipoib_start_xmit+0x42/0x66d [ib_ipoib]
 #1:  (&priv->lock){.+..}, at: [<ffffffff881efd5b>] ipoib_mcast_send+0x29/0x413 [ib_ipoib]

the hard-irq-safe lock's dependencies:
-> (&priv->tx_lock){+...} ops: 0 {
   initial-use  at:
                        [<ffffffff8024172a>] lock_acquire+0x4a/0x69
                        [<ffffffff803eb644>] _spin_lock_irq+0x28/0x35
                        [<ffffffff881ef705>] ipoib_mcast_join_finish+0x340/0x379 [ib_ipoib]
                        [<ffffffff881ef7e0>] ipoib_mcast_join_complete+0xa2/0x294 [ib_ipoib]
                        [<ffffffff881e6389>] ib_sa_mcmember_rec_callback+0x4b/0x57 [ib_sa]
                        [<ffffffff881e648d>] recv_handler+0x3d/0x4a [ib_sa]
                        [<ffffffff881b9c20>] ib_mad_completion_handler+0x3df/0x5e4 [ib_mad]
                        [<ffffffff80238422>] run_workqueue+0xa0/0xf7
                        [<ffffffff8023861d>] worker_thread+0xee/0x122
                        [<ffffffff8023b661>] kthread+0xd0/0xfb
                        [<ffffffff8020a5f1>] child_rip+0x7/0x12
   in-hardirq-W at:
                        [<ffffffff8024172a>] lock_acquire+0x4a/0x69
                        [<ffffffff803eb7da>] _spin_lock_irqsave+0x2b/0x3c
                        [<ffffffff881ee615>] ipoib_ib_completion+0x340/0x3ff [ib_ipoib]
                        [<ffffffff881ca8a5>] mthca_cq_completion+0x65/0x6b [ib_mthca]
                        [<ffffffff881ca1bc>] mthca_eq_int+0x70/0x3d2 [ib_mthca]
                        [<ffffffff881ca644>] mthca_arbel_interrupt+0x3b/0x98 [ib_mthca]
                        [<ffffffff802537de>] handle_IRQ_event+0x28/0x64
                        [<ffffffff802538c6>] __do_IRQ+0xac/0x117
                        [<ffffffff8020becf>] do_IRQ+0xf7/0x108
                        [<ffffffff80209bd8>] common_interrupt+0x64/0x65
 }
 ... key      at: [<ffffffff881f9a28>] __key.23652+0x0/0xffffffffffff804f [ib_ipoib]
 -> (&priv->lock){.+..} ops: 0 {
    initial-use  at:
                          [<ffffffff8024172a>] lock_acquire+0x4a/0x69
                          [<ffffffff803eb644>] _spin_lock_irq+0x28/0x35
                          [<ffffffff881ef2c6>] ipoib_mcast_start_thread+0x6e/0x87 [ib_ipoib]
                          [<ffffffff881eee7d>] ipoib_ib_dev_up+0x52/0x58 [ib_ipoib]
                          [<ffffffff881ec23f>] ipoib_open+0x65/0x102 [ib_ipoib]
                          [<ffffffff8038c74c>] dev_open+0x3a/0x80
                          [<ffffffff8038ba90>] dev_change_flags+0x65/0x139
                          [<ffffffff803ccb56>] devinet_ioctl+0x240/0x5e2
                          [<ffffffff803cd18f>] inet_ioctl+0x70/0x8f
                          [<ffffffff8038203c>] sock_ioctl+0x1c0/0x1ea
                          [<ffffffff8028591e>] do_ioctl+0x26/0x74
                          [<ffffffff80285bb6>] vfs_ioctl+0x24a/0x264
                          [<ffffffff80285c11>] sys_ioctl+0x41/0x68
                          [<ffffffff802096a1>] system_call+0x7d/0x83
    in-softirq-W at:
                          [<ffffffff8024172a>] lock_acquire+0x4a/0x69
                          [<ffffffff803eb316>] _spin_lock+0x21/0x2e
                          [<ffffffff881efd5a>] ipoib_mcast_send+0x28/0x413 [ib_ipoib]
                          [<ffffffff881eda0e>] ipoib_start_xmit+0x361/0x66d [ib_ipoib]
                          [<ffffffff8038dfc8>] dev_hard_start_xmit+0x1ab/0x221
                          [<ffffffff8039a024>] __qdisc_run+0xfa/0x1cd
                          [<ffffffff8038e174>] dev_queue_xmit+0x136/0x263
                          [<ffffffff80390209>] neigh_connected_output+0xae/0xc7
                          [<ffffffff8813f4de>] ip6_output2+0x254/0x28c [ipv6]
                          [<ffffffff8813fcf8>] ip6_output+0x7e2/0x7f8 [ipv6]
                          [<ffffffff8814dbf7>] ndisc_send_ns+0x38f/0x4c1 [ipv6]
                          [<ffffffff88145048>] addrconf_dad_timer+0xfb/0x11e [ipv6]
                          [<ffffffff80231a8e>] run_timer_softirq+0x150/0x1dc
                          [<ffffffff8022e345>] __do_softirq+0x6b/0xf7
                          [<ffffffff8020a94d>] call_softirq+0x1d/0x28
                          [<ffffffff8022e0bc>] irq_exit+0x56/0x59
                          [<ffffffff802146ee>] smp_apic_timer_interrupt+0x59/0x5f
                          [<ffffffff8020a2c1>] apic_timer_interrupt+0x69/0x70
  }
  ... key      at: [<ffffffff881f9a30>] __key.23651+0x0/0xffffffffffff8047 [ib_ipoib]
  -> (&idr_lock){....} ops: 0 {
     initial-use  at:
                            [<ffffffff8024172a>] lock_acquire+0x4a/0x69
                            [<ffffffff803eb7da>] _spin_lock_irqsave+0x2b/0x3c
                            [<ffffffff881e6161>] send_mad+0x39/0x141 [ib_sa]
                            [<ffffffff881e6a20>] ib_sa_mcmember_rec_query+0x147/0x17c [ib_sa]
                            [<ffffffff881f02a4>] ipoib_mcast_join+0x15f/0x1f2 [ib_ipoib]
                            [<ffffffff881f0582>] ipoib_mcast_join_task+0x24b/0x2e7 [ib_ipoib]
                            [<ffffffff80238422>] run_workqueue+0xa0/0xf7
                            [<ffffffff8023861d>] worker_thread+0xee/0x122
                            [<ffffffff8023b661>] kthread+0xd0/0xfb
                            [<ffffffff8020a5f1>] child_rip+0x7/0x12
   }
   ... key      at: [<ffffffff881ea488>] __key.17990+0x0/0xffffffffffffc8c2 [ib_sa]
   -> (query_idr.lock){....} ops: 0 {
      initial-use  at:
                              [<ffffffff8024172a>] lock_acquire+0x4a/0x69
                              [<ffffffff803eb7da>] _spin_lock_irqsave+0x2b/0x3c
                              [<ffffffff802e4581>] free_layer+0x1c/0x40
                              [<ffffffff802e45d6>] idr_pre_get+0x31/0x42
                              [<ffffffff881e614d>] send_mad+0x25/0x141 [ib_sa]
                              [<ffffffff881e6a20>] ib_sa_mcmember_rec_query+0x147/0x17c [ib_sa]
                              [<ffffffff881f02a4>] ipoib_mcast_join+0x15f/0x1f2 [ib_ipoib]
                              [<ffffffff881f0582>] ipoib_mcast_join_task+0x24b/0x2e7 [ib_ipoib]
                              [<ffffffff80238422>] run_workqueue+0xa0/0xf7
                              [<ffffffff8023861d>] worker_thread+0xee/0x122
                              [<ffffffff8023b661>] kthread+0xd0/0xfb
                              [<ffffffff8020a5f1>] child_rip+0x7/0x12
    }
    ... key      at: [<ffffffff881e9f30>] query_idr+0x30/0xffffffffffffce4a [ib_sa]
   ... acquired at:
   [<ffffffff8024172a>] lock_acquire+0x4a/0x69
   [<ffffffff803eb7da>] _spin_lock_irqsave+0x2b/0x3c
   [<ffffffff802e4530>] alloc_layer+0x18/0x4d
   [<ffffffff802e48de>] idr_get_new_above_int+0x37/0x225
   [<ffffffff802e4adb>] idr_get_new+0xf/0x2f
   [<ffffffff881e6177>] send_mad+0x4f/0x141 [ib_sa]
   [<ffffffff881e6a20>] ib_sa_mcmember_rec_query+0x147/0x17c [ib_sa]
   [<ffffffff881f02a4>] ipoib_mcast_join+0x15f/0x1f2 [ib_ipoib]
   [<ffffffff881f0582>] ipoib_mcast_join_task+0x24b/0x2e7 [ib_ipoib]
   [<ffffffff80238422>] run_workqueue+0xa0/0xf7
   [<ffffffff8023861d>] worker_thread+0xee/0x122
   [<ffffffff8023b661>] kthread+0xd0/0xfb
   [<ffffffff8020a5f1>] child_rip+0x7/0x12

  ... acquired at:
   [<ffffffff8024172a>] lock_acquire+0x4a/0x69
   [<ffffffff803eb7da>] _spin_lock_irqsave+0x2b/0x3c
   [<ffffffff881e6069>] ib_sa_cancel_query+0x1b/0x73 [ib_sa]
   [<ffffffff881eefb5>] wait_for_mcast_join+0x35/0xd2 [ib_ipoib]
   [<ffffffff881ef385>] ipoib_mcast_stop_thread+0xa6/0xe6 [ib_ipoib]
   [<ffffffff881f07da>] ipoib_mcast_restart_task+0x4b/0x3d9 [ib_ipoib]
   [<ffffffff80238422>] run_workqueue+0xa0/0xf7
   [<ffffffff8023861d>] worker_thread+0xee/0x122
   [<ffffffff8023b661>] kthread+0xd0/0xfb
   [<ffffffff8020a5f1>] child_rip+0x7/0x12

  -> (&mad_agent_priv->lock){....} ops: 0 {
     initial-use  at:
                            [<ffffffff8024172a>] lock_acquire+0x4a/0x69
                            [<ffffffff803eb7da>] _spin_lock_irqsave+0x2b/0x3c
                            [<ffffffff881b936d>] ib_post_send_mad+0x3e0/0x512 [ib_mad]
                            [<ffffffff881bb251>] agent_send_response+0x11b/0x155 [ib_mad]
                            [<ffffffff881b9b2b>] ib_mad_completion_handler+0x2ea/0x5e4 [ib_mad]
                            [<ffffffff80238422>] run_workqueue+0xa0/0xf7
                            [<ffffffff8023861d>] worker_thread+0xee/0x122
                            [<ffffffff8023b661>] kthread+0xd0/0xfb
                            [<ffffffff8020a5f1>] child_rip+0x7/0x12
   }
   ... key      at: [<ffffffff881c1424>] __key.17415+0x0/0xffffffffffffaf74 [ib_mad]
   -> (base_lock_keys + cpu){++..} ops: 0 {
      initial-use  at:
                              [<ffffffff8024172a>] lock_acquire+0x4a/0x69
                              [<ffffffff803eb644>] _spin_lock_irq+0x28/0x35
                              [<ffffffff8023197f>] run_timer_softirq+0x41/0x1dc
                              [<ffffffff8022e345>] __do_softirq+0x6b/0xf7
                              [<ffffffff8020a94d>] call_softirq+0x1d/0x28
                              [<ffffffff8022e0bc>] irq_exit+0x56/0x59
                              [<ffffffff8020bed4>] do_IRQ+0xfc/0x108
                              [<ffffffff80209bd8>] common_interrupt+0x64/0x65
      in-hardirq-W at:
                              [<ffffffff8024172a>] lock_acquire+0x4a/0x69
                              [<ffffffff803eb316>] _spin_lock+0x21/0x2e
                              [<ffffffff80231bad>] __mod_timer+0x93/0xc8
                              [<ffffffff80231c0e>] mod_timer+0x2c/0x2f
                              [<ffffffff80377fb2>] i8042_interrupt+0x2a/0x222
                              [<ffffffff802537de>] handle_IRQ_event+0x28/0x64
                              [<ffffffff802538c6>] __do_IRQ+0xac/0x117
                              [<ffffffff8020becf>] do_IRQ+0xf7/0x108
                              [<ffffffff80209bd8>] common_interrupt+0x64/0x65
                              [<7ffffffffffffffd>] 0x7ffffffffffffffd
                              [<ffffffff8020a94d>] call_softirq+0x1d/0x28
                              [<ffffffff8022e0bc>] irq_exit+0x56/0x59
                              [<ffffffff802146ee>] smp_apic_timer_interrupt+0x59/0x5f
                              [<ffffffff8020a2c1>] apic_timer_interrupt+0x69/0x70
      in-softirq-W at:
                              [<ffffffff8024172a>] lock_acquire+0x4a/0x69
                              [<ffffffff803eb644>] _spin_lock_irq+0x28/0x35
                              [<ffffffff8023197f>] run_timer_softirq+0x41/0x1dc
                              [<ffffffff8022e345>] __do_softirq+0x6b/0xf7
                              [<ffffffff8020a94d>] call_softirq+0x1d/0x28
                              [<ffffffff8022e0bc>] irq_exit+0x56/0x59
                              [<ffffffff8020bed4>] do_IRQ+0xfc/0x108
                              [<ffffffff80209bd8>] common_interrupt+0x64/0x65
    }
    ... key      at: [<ffffffff805a16e0>] base_lock_keys+0x0/0x20
   ... acquired at:
   [<ffffffff8024172a>] lock_acquire+0x4a/0x69
   [<ffffffff803eb7da>] _spin_lock_irqsave+0x2b/0x3c
   [<ffffffff80231316>] lock_timer_base+0x21/0x47
   [<ffffffff80231c84>] try_to_del_timer_sync+0x14/0x62
   [<ffffffff80231ce3>] del_timer_sync+0x11/0x1a
   [<ffffffff881b8a7b>] ib_mad_complete_send_wr+0x10b/0x1f8 [ib_mad]
   [<ffffffff881b8c8d>] ib_mad_send_done_handler+0x125/0x17a [ib_mad]
   [<ffffffff881b9db2>] ib_mad_completion_handler+0x571/0x5e4 [ib_mad]
   [<ffffffff80238422>] run_workqueue+0xa0/0xf7
   [<ffffffff8023861d>] worker_thread+0xee/0x122
   [<ffffffff8023b661>] kthread+0xd0/0xfb
   [<ffffffff8020a5f1>] child_rip+0x7/0x12

   -> (base_lock_keys + cpu#4){++..} ops: 0 {
      initial-use  at:
                              [<ffffffff8024172a>] lock_acquire+0x4a/0x69
                              [<ffffffff803eb644>] _spin_lock_irq+0x28/0x35
                              [<ffffffff8023197f>] run_timer_softirq+0x41/0x1dc
                              [<ffffffff8022e345>] __do_softirq+0x6b/0xf7
                              [<ffffffff8020a94d>] call_softirq+0x1d/0x28
                              [<ffffffff8022e0bc>] irq_exit+0x56/0x59
                              [<ffffffff802146ee>] smp_apic_timer_interrupt+0x59/0x5f
                              [<ffffffff8020a2c1>] apic_timer_interrupt+0x69/0x70
      in-hardirq-W at:
                              [<ffffffff8024172a>] lock_acquire+0x4a/0x69
                              [<ffffffff803eb7da>] _spin_lock_irqsave+0x2b/0x3c
                              [<ffffffff80231316>] lock_timer_base+0x21/0x47
                              [<ffffffff80231c2f>] del_timer+0x1e/0x5f
                              [<ffffffff8034f281>] scsi_delete_timer+0x12/0x5d
                              [<ffffffff8034d064>] scsi_done+0xd/0x1e
                              [<ffffffff8035d444>] ata_scsi_qc_complete+0xb1/0xc3
                              [<ffffffff803583e9>] __ata_qc_complete+0x218/0x225
                              [<ffffffff803584c5>] ata_qc_complete+0xcf/0xd5
                              [<ffffffff80359cf0>] ata_hsm_qc_complete+0x1c6/0x1d8
                              [<ffffffff8035a31a>] ata_hsm_move+0x618/0x638
                              [<ffffffff8035a8ba>] ata_interrupt+0x161/0x1ae
                              [<ffffffff802537de>] handle_IRQ_event+0x28/0x64
                              [<ffffffff802538c6>] __do_IRQ+0xac/0x117
                              [<ffffffff8020becf>] do_IRQ+0xf7/0x108
                              [<ffffffff80209bd8>] common_interrupt+0x64/0x65
      in-softirq-W at:
                              [<ffffffff8024172a>] lock_acquire+0x4a/0x69
                              [<ffffffff803eb644>] _spin_lock_irq+0x28/0x35
                              [<ffffffff8023197f>] run_timer_softirq+0x41/0x1dc
                              [<ffffffff8022e345>] __do_softirq+0x6b/0xf7
                              [<ffffffff8020a94d>] call_softirq+0x1d/0x28
                              [<ffffffff8022e0bc>] irq_exit+0x56/0x59
                              [<ffffffff802146ee>] smp_apic_timer_interrupt+0x59/0x5f
                              [<ffffffff8020a2c1>] apic_timer_interrupt+0x69/0x70
    }
    ... key      at: [<ffffffff805a16f8>] base_lock_keys+0x18/0x20
   ... acquired at:
   [<ffffffff8024172a>] lock_acquire+0x4a/0x69
   [<ffffffff803eb316>] _spin_lock+0x21/0x2e
   [<ffffffff80231bad>] __mod_timer+0x93/0xc8
   [<ffffffff802386c6>] queue_delayed_work+0x75/0x7d
   [<ffffffff881b776a>] wait_for_response+0xdf/0xe8 [ib_mad]
   [<ffffffff881b8a31>] ib_mad_complete_send_wr+0xc1/0x1f8 [ib_mad]
   [<ffffffff881b8c8d>] ib_mad_send_done_handler+0x125/0x17a [ib_mad]
   [<ffffffff881b9db2>] ib_mad_completion_handler+0x571/0x5e4 [ib_mad]
   [<ffffffff80238422>] run_workqueue+0xa0/0xf7
   [<ffffffff8023861d>] worker_thread+0xee/0x122
   [<ffffffff8023b661>] kthread+0xd0/0xfb
   [<ffffffff8020a5f1>] child_rip+0x7/0x12

   -> (base_lock_keys + cpu#2){++..} ops: 0 {
      initial-use  at:
                              [<ffffffff8024172a>] lock_acquire+0x4a/0x69
                              [<ffffffff803eb644>] _spin_lock_irq+0x28/0x35
                              [<ffffffff8023197f>] run_timer_softirq+0x41/0x1dc
                              [<ffffffff8022e345>] __do_softirq+0x6b/0xf7
                              [<ffffffff8020a94d>] call_softirq+0x1d/0x28
                              [<ffffffff8022e0bc>] irq_exit+0x56/0x59
                              [<ffffffff802146ee>] smp_apic_timer_interrupt+0x59/0x5f
                              [<ffffffff8020a2c1>] apic_timer_interrupt+0x69/0x70
      in-hardirq-W at:
                              [<ffffffff8024172a>] lock_acquire+0x4a/0x69
                              [<ffffffff803eb316>] _spin_lock+0x21/0x2e
                              [<ffffffff80231bad>] __mod_timer+0x93/0xc8
                              [<ffffffff80231c0e>] mod_timer+0x2c/0x2f
                              [<ffffffff80377fb2>] i8042_interrupt+0x2a/0x222
                              [<ffffffff802537de>] handle_IRQ_event+0x28/0x64
                              [<ffffffff802538c6>] __do_IRQ+0xac/0x117
                              [<ffffffff8020becf>] do_IRQ+0xf7/0x108
                              [<ffffffff80209bd8>] common_interrupt+0x64/0x65
      in-softirq-W at:
                              [<ffffffff8024172a>] lock_acquire+0x4a/0x69
                              [<ffffffff803eb644>] _spin_lock_irq+0x28/0x35
                              [<ffffffff8023197f>] run_timer_softirq+0x41/0x1dc
                              [<ffffffff8022e345>] __do_softirq+0x6b/0xf7
                              [<ffffffff8020a94d>] call_softirq+0x1d/0x28
                              [<ffffffff8022e0bc>] irq_exit+0x56/0x59
                              [<ffffffff802146ee>] smp_apic_timer_interrupt+0x59/0x5f
                              [<ffffffff8020a2c1>] apic_timer_interrupt+0x69/0x70
    }
    ... key      at: [<ffffffff805a16e8>] base_lock_keys+0x8/0x20
   ... acquired at:
   [<ffffffff8024172a>] lock_acquire+0x4a/0x69
   [<ffffffff803eb316>] _spin_lock+0x21/0x2e
   [<ffffffff80231bad>] __mod_timer+0x93/0xc8
   [<ffffffff802386c6>] queue_delayed_work+0x75/0x7d
   [<ffffffff881b776a>] wait_for_response+0xdf/0xe8 [ib_mad]
   [<ffffffff881b8a31>] ib_mad_complete_send_wr+0xc1/0x1f8 [ib_mad]
   [<ffffffff881b8c8d>] ib_mad_send_done_handler+0x125/0x17a [ib_mad]
   [<ffffffff881b9db2>] ib_mad_completion_handler+0x571/0x5e4 [ib_mad]
   [<ffffffff80238422>] run_workqueue+0xa0/0xf7
   [<ffffffff8023861d>] worker_thread+0xee/0x122
   [<ffffffff8023b661>] kthread+0xd0/0xfb
   [<ffffffff8020a5f1>] child_rip+0x7/0x12

  ... acquired at:
   [<ffffffff8024172a>] lock_acquire+0x4a/0x69
   [<ffffffff803eb7da>] _spin_lock_irqsave+0x2b/0x3c
   [<ffffffff881b7a16>] ib_modify_mad+0x27/0x14c [ib_mad]
   [<ffffffff881b7b45>] ib_cancel_mad+0xa/0xd [ib_mad]
   [<ffffffff881e60b8>] ib_sa_cancel_query+0x6a/0x73 [ib_sa]
   [<ffffffff881eefb5>] wait_for_mcast_join+0x35/0xd2 [ib_ipoib]
   [<ffffffff881ef385>] ipoib_mcast_stop_thread+0xa6/0xe6 [ib_ipoib]
   [<ffffffff881f07da>] ipoib_mcast_restart_task+0x4b/0x3d9 [ib_ipoib]
   [<ffffffff80238422>] run_workqueue+0xa0/0xf7
   [<ffffffff8023861d>] worker_thread+0xee/0x122
   [<ffffffff8023b661>] kthread+0xd0/0xfb
   [<ffffffff8020a5f1>] child_rip+0x7/0x12

  -> (modlist_lock){.+..} ops: 0 {
     initial-use  at:
                            [<ffffffff8024172a>] lock_acquire+0x4a/0x69
                            [<ffffffff803eb7da>] _spin_lock_irqsave+0x2b/0x3c
                            [<ffffffff80247f0c>] module_text_address+0x15/0x3b
                            [<ffffffff803ee75b>] __register_kprobe+0x2bc/0x2da
                            [<ffffffff803ee8e3>] register_kprobe+0xc/0xf
                            [<ffffffff80806064>] arch_init_kprobes+0xf/0x12
                            [<ffffffff808094be>] init_kprobes+0x3f/0x52
                            [<ffffffff8020718b>] init+0x143/0x308
                            [<ffffffff8020a5f1>] child_rip+0x7/0x12
     in-softirq-W at:
                            [<ffffffff8024172a>] lock_acquire+0x4a/0x69
                            [<ffffffff803eb7da>] _spin_lock_irqsave+0x2b/0x3c
                            [<ffffffff80247e1e>] is_module_address+0x14/0x80
                            [<ffffffff8023ede1>] static_obj+0x6c/0x79
                            [<ffffffff8023ee8b>] lockdep_init_map+0x7d/0xc3
                            [<ffffffff802f64d2>] __spin_lock_init+0x2e/0x58
                            [<ffffffff881ef221>] ipoib_mcast_alloc+0x97/0xce [ib_ipoib]
                            [<ffffffff881efe3b>] ipoib_mcast_send+0x109/0x413 [ib_ipoib]
                            [<ffffffff881eda0e>] ipoib_start_xmit+0x361/0x66d [ib_ipoib]
                            [<ffffffff8038dfc8>] dev_hard_start_xmit+0x1ab/0x221
                            [<ffffffff8039a024>] __qdisc_run+0xfa/0x1cd
                            [<ffffffff8038e174>] dev_queue_xmit+0x136/0x263
                            [<ffffffff80390209>] neigh_connected_output+0xae/0xc7
                            [<ffffffff8813f4de>] ip6_output2+0x254/0x28c [ipv6]
                            [<ffffffff8813fcf8>] ip6_output+0x7e2/0x7f8 [ipv6]
                            [<ffffffff8814d736>] ndisc_send_rs+0x33d/0x46f [ipv6]
                            [<ffffffff881427b9>] addrconf_dad_completed+0x90/0xe2 [ipv6]
                            [<ffffffff88144fc1>] addrconf_dad_timer+0x74/0x11e [ipv6]
                            [<ffffffff80231a8e>] run_timer_softirq+0x150/0x1dc
                            [<ffffffff8022e345>] __do_softirq+0x6b/0xf7
                            [<ffffffff8020a94d>] call_softirq+0x1d/0x28
                            [<ffffffff8022e0bc>] irq_exit+0x56/0x59
                            [<ffffffff802146ee>] smp_apic_timer_interrupt+0x59/0x5f
                            [<ffffffff8020a2c1>] apic_timer_interrupt+0x69/0x70
   }
   ... key      at: [<ffffffff804cfd98>] modlist_lock+0x18/0x40
  ... acquired at:
   [<ffffffff8024172a>] lock_acquire+0x4a/0x69
   [<ffffffff803eb7da>] _spin_lock_irqsave+0x2b/0x3c
   [<ffffffff80247e1e>] is_module_address+0x14/0x80
   [<ffffffff8023ede1>] static_obj+0x6c/0x79
   [<ffffffff8023ee8b>] lockdep_init_map+0x7d/0xc3
   [<ffffffff802f64d2>] __spin_lock_init+0x2e/0x58
   [<ffffffff881ef221>] ipoib_mcast_alloc+0x97/0xce [ib_ipoib]
   [<ffffffff881f0903>] ipoib_mcast_restart_task+0x174/0x3d9 [ib_ipoib]
   [<ffffffff80238422>] run_workqueue+0xa0/0xf7
   [<ffffffff8023861d>] worker_thread+0xee/0x122
   [<ffffffff8023b661>] kthread+0xd0/0xfb
   [<ffffffff8020a5f1>] child_rip+0x7/0x12

  -> (&qp->sq.lock){.+..} ops: 0 {
     initial-use  at:
                            [<ffffffff8024172a>] lock_acquire+0x4a/0x69
                            [<ffffffff803eb644>] _spin_lock_irq+0x28/0x35
                            [<ffffffff881cdef3>] mthca_modify_qp+0x67/0xd7b [ib_mthca]
                            [<ffffffff881a68fa>] ib_modify_qp+0xc/0xf [ib_core]
                            [<ffffffff881b8394>] ib_mad_init_device+0x2db/0x55c [ib_mad]
                            [<ffffffff881a83fe>] ib_register_device+0x20d/0x300 [ib_core]
                            [<ffffffff881d2f53>] mthca_register_device+0x3e0/0x430 [ib_mthca]
                            [<ffffffff881c4f26>] mthca_init_one+0xbc2/0xcbc [ib_mthca]
                            [<ffffffff802fcab2>] pci_device_probe+0x4b/0x72
                            [<ffffffff803481b7>] driver_probe_device+0x59/0xaf
                            [<ffffffff803482d3>] __driver_attach+0x58/0x90
                            [<ffffffff80347665>] bus_for_each_dev+0x48/0x7a
                            [<ffffffff80348047>] driver_attach+0x1b/0x1e
                            [<ffffffff803479d2>] bus_add_driver+0x74/0x112
                            [<ffffffff80348705>] driver_register+0x8c/0x91
                            [<ffffffff802fc684>] __pci_register_driver+0x60/0x84
                            [<ffffffff88022016>] 0xffffffff88022016
                            [<ffffffff80247a93>] sys_init_module+0x174c/0x1884
                            [<ffffffff802096a1>] system_call+0x7d/0x83
     in-softirq-W at:
                            [<ffffffff8024172a>] lock_acquire+0x4a/0x69
                            [<ffffffff803eb7da>] _spin_lock_irqsave+0x2b/0x3c
                            [<ffffffff881cff9d>] mthca_arbel_post_send+0x2f/0x5be [ib_mthca]
                            [<ffffffff881ee043>] ipoib_send+0x117/0x1d6 [ib_ipoib]
                            [<ffffffff881f012d>] ipoib_mcast_send+0x3fb/0x413 [ib_ipoib]
                            [<ffffffff881eda0e>] ipoib_start_xmit+0x361/0x66d [ib_ipoib]
                            [<ffffffff8038dfc8>] dev_hard_start_xmit+0x1ab/0x221
                            [<ffffffff8039a024>] __qdisc_run+0xfa/0x1cd
                            [<ffffffff8038e174>] dev_queue_xmit+0x136/0x263
                            [<ffffffff80390209>] neigh_connected_output+0xae/0xc7
                            [<ffffffff8813f4de>] ip6_output2+0x254/0x28c [ipv6]
                            [<ffffffff8813fcf8>] ip6_output+0x7e2/0x7f8 [ipv6]
                            [<ffffffff8814dbf7>] ndisc_send_ns+0x38f/0x4c1 [ipv6]
                            [<ffffffff88145048>] addrconf_dad_timer+0xfb/0x11e [ipv6]
                            [<ffffffff80231a8e>] run_timer_softirq+0x150/0x1dc
                            [<ffffffff8022e345>] __do_softirq+0x6b/0xf7
                            [<ffffffff8020a94d>] call_softirq+0x1d/0x28
                            [<ffffffff8022e0bc>] irq_exit+0x56/0x59
                            [<ffffffff802146ee>] smp_apic_timer_interrupt+0x59/0x5f
                            [<ffffffff8020a2c1>] apic_timer_interrupt+0x69/0x70
   }
   ... key      at: [<ffffffff881e4060>] __key.20374+0x0/0xffffffffffff231a [ib_mthca]
   -> (&qp->rq.lock){+...} ops: 0 {
      initial-use  at:
                              [<ffffffff8024172a>] lock_acquire+0x4a/0x69
                              [<ffffffff803eb316>] _spin_lock+0x21/0x2e
                              [<ffffffff881cdf02>] mthca_modify_qp+0x76/0xd7b [ib_mthca]
                              [<ffffffff881a68fa>] ib_modify_qp+0xc/0xf [ib_core]
                              [<ffffffff881b8394>] ib_mad_init_device+0x2db/0x55c [ib_mad]
                              [<ffffffff881a83fe>] ib_register_device+0x20d/0x300 [ib_core]
                              [<ffffffff881d2f53>] mthca_register_device+0x3e0/0x430 [ib_mthca]
                              [<ffffffff881c4f26>] mthca_init_one+0xbc2/0xcbc [ib_mthca]
                              [<ffffffff802fcab2>] pci_device_probe+0x4b/0x72
                              [<ffffffff803481b7>] driver_probe_device+0x59/0xaf
                              [<ffffffff803482d3>] __driver_attach+0x58/0x90
                              [<ffffffff80347665>] bus_for_each_dev+0x48/0x7a
                              [<ffffffff80348047>] driver_attach+0x1b/0x1e
                              [<ffffffff803479d2>] bus_add_driver+0x74/0x112
                              [<ffffffff80348705>] driver_register+0x8c/0x91
                              [<ffffffff802fc684>] __pci_register_driver+0x60/0x84
                              [<ffffffff88022016>] 0xffffffff88022016
                              [<ffffffff80247a93>] sys_init_module+0x174c/0x1884
                              [<ffffffff802096a1>] system_call+0x7d/0x83
      in-hardirq-W at:
                              [<ffffffff8024172a>] lock_acquire+0x4a/0x69
                              [<ffffffff803eb7da>] _spin_lock_irqsave+0x2b/0x3c
                              [<ffffffff881cd749>] mthca_arbel_post_receive+0x35/0x23a [ib_mthca]
                              [<ffffffff881ee183>] ipoib_ib_post_receive+0x81/0xf9 [ib_ipoib]
                              [<ffffffff881ee55b>] ipoib_ib_completion+0x286/0x3ff [ib_ipoib]
                              [<ffffffff881ca8a5>] mthca_cq_completion+0x65/0x6b [ib_mthca]
                              [<ffffffff881ca1bc>] mthca_eq_int+0x70/0x3d2 [ib_mthca]
                              [<ffffffff881ca644>] mthca_arbel_interrupt+0x3b/0x98 [ib_mthca]
                              [<ffffffff802537de>] handle_IRQ_event+0x28/0x64
                              [<ffffffff802538c6>] __do_IRQ+0xac/0x117
                              [<ffffffff8020becf>] do_IRQ+0xf7/0x108
                              [<ffffffff80209bd8>] common_interrupt+0x64/0x65
    }
    ... key      at: [<ffffffff881e4058>] __key.20375+0x0/0xffffffffffff2322 [ib_mthca]
   ... acquired at:
   [<ffffffff8024172a>] lock_acquire+0x4a/0x69
   [<ffffffff803eb316>] _spin_lock+0x21/0x2e
   [<ffffffff881cdf02>] mthca_modify_qp+0x76/0xd7b [ib_mthca]
   [<ffffffff881a68fa>] ib_modify_qp+0xc/0xf [ib_core]
   [<ffffffff881b8394>] ib_mad_init_device+0x2db/0x55c [ib_mad]
   [<ffffffff881a83fe>] ib_register_device+0x20d/0x300 [ib_core]
   [<ffffffff881d2f53>] mthca_register_device+0x3e0/0x430 [ib_mthca]
   [<ffffffff881c4f26>] mthca_init_one+0xbc2/0xcbc [ib_mthca]
   [<ffffffff802fcab2>] pci_device_probe+0x4b/0x72
   [<ffffffff803481b7>] driver_probe_device+0x59/0xaf
   [<ffffffff803482d3>] __driver_attach+0x58/0x90
   [<ffffffff80347665>] bus_for_each_dev+0x48/0x7a
   [<ffffffff80348047>] driver_attach+0x1b/0x1e
   [<ffffffff803479d2>] bus_add_driver+0x74/0x112
   [<ffffffff80348705>] driver_register+0x8c/0x91
   [<ffffffff802fc684>] __pci_register_driver+0x60/0x84
   [<ffffffff88022016>] 0xffffffff88022016
   [<ffffffff80247a93>] sys_init_module+0x174c/0x1884
   [<ffffffff802096a1>] system_call+0x7d/0x83

  ... acquired at:
   [<ffffffff8024172a>] lock_acquire+0x4a/0x69
   [<ffffffff803eb7da>] _spin_lock_irqsave+0x2b/0x3c
   [<ffffffff881cff9d>] mthca_arbel_post_send+0x2f/0x5be [ib_mthca]
   [<ffffffff881ee043>] ipoib_send+0x117/0x1d6 [ib_ipoib]
   [<ffffffff881f012d>] ipoib_mcast_send+0x3fb/0x413 [ib_ipoib]
   [<ffffffff881eda0e>] ipoib_start_xmit+0x361/0x66d [ib_ipoib]
   [<ffffffff8038dfc8>] dev_hard_start_xmit+0x1ab/0x221
   [<ffffffff8039a024>] __qdisc_run+0xfa/0x1cd
   [<ffffffff8038e174>] dev_queue_xmit+0x136/0x263
   [<ffffffff80390209>] neigh_connected_output+0xae/0xc7
   [<ffffffff8813f4de>] ip6_output2+0x254/0x28c [ipv6]
   [<ffffffff8813fcf8>] ip6_output+0x7e2/0x7f8 [ipv6]
   [<ffffffff8814dbf7>] ndisc_send_ns+0x38f/0x4c1 [ipv6]
   [<ffffffff88145048>] addrconf_dad_timer+0xfb/0x11e [ipv6]
   [<ffffffff80231a8e>] run_timer_softirq+0x150/0x1dc
   [<ffffffff8022e345>] __do_softirq+0x6b/0xf7
   [<ffffffff8020a94d>] call_softirq+0x1d/0x28
   [<ffffffff8022e0bc>] irq_exit+0x56/0x59
   [<ffffffff802146ee>] smp_apic_timer_interrupt+0x59/0x5f
   [<ffffffff8020a2c1>] apic_timer_interrupt+0x69/0x70

 ... acquired at:
   [<ffffffff8024172a>] lock_acquire+0x4a/0x69
   [<ffffffff803eb316>] _spin_lock+0x21/0x2e
   [<ffffffff881efd5a>] ipoib_mcast_send+0x28/0x413 [ib_ipoib]
   [<ffffffff881eda0e>] ipoib_start_xmit+0x361/0x66d [ib_ipoib]
   [<ffffffff8038dfc8>] dev_hard_start_xmit+0x1ab/0x221
   [<ffffffff8039a024>] __qdisc_run+0xfa/0x1cd
   [<ffffffff8038e174>] dev_queue_xmit+0x136/0x263
   [<ffffffff80390209>] neigh_connected_output+0xae/0xc7
   [<ffffffff8813f4de>] ip6_output2+0x254/0x28c [ipv6]
   [<ffffffff8813fcf8>] ip6_output+0x7e2/0x7f8 [ipv6]
   [<ffffffff8814dbf7>] ndisc_send_ns+0x38f/0x4c1 [ipv6]
   [<ffffffff88145048>] addrconf_dad_timer+0xfb/0x11e [ipv6]
   [<ffffffff80231a8e>] run_timer_softirq+0x150/0x1dc
   [<ffffffff8022e345>] __do_softirq+0x6b/0xf7
   [<ffffffff8020a94d>] call_softirq+0x1d/0x28
   [<ffffffff8022e0bc>] irq_exit+0x56/0x59
   [<ffffffff802146ee>] smp_apic_timer_interrupt+0x59/0x5f
   [<ffffffff8020a2c1>] apic_timer_interrupt+0x69/0x70


the hard-irq-unsafe lock's dependencies:
-> (&skb_queue_lock_key){-+..} ops: 0 {
   initial-use  at:
                        [<ffffffff8024172a>] lock_acquire+0x4a/0x69
                        [<ffffffff803eb7da>] _spin_lock_irqsave+0x2b/0x3c
                        [<ffffffff8038683d>] skb_queue_tail+0x1c/0x47
                        [<ffffffff8039ff72>] netlink_broadcast+0x212/0x2ea
                        [<ffffffff802e57c4>] kobject_uevent+0x3c9/0x43d
                        [<ffffffff8034897f>] store_uevent+0x16/0x1e
                        [<ffffffff803488ef>] class_device_attr_store+0x1b/0x1e
                        [<ffffffff802b0c33>] sysfs_write_file+0xb7/0xe4
                        [<ffffffff8027448e>] vfs_write+0xad/0x154
                        [<ffffffff80274d8b>] sys_write+0x46/0x6f
                        [<ffffffff802096a1>] system_call+0x7d/0x83
   in-softirq-W at:
                        [<ffffffff8024172a>] lock_acquire+0x4a/0x69
                        [<ffffffff803eb7da>] _spin_lock_irqsave+0x2b/0x3c
                        [<ffffffff80386aca>] skb_dequeue+0x18/0x5c
                        [<ffffffff80387015>] skb_queue_purge+0x18/0x26
                        [<ffffffff80394261>] neigh_timer_handler+0x26f/0x346
                        [<ffffffff80231a8e>] run_timer_softirq+0x150/0x1dc
                        [<ffffffff8022e345>] __do_softirq+0x6b/0xf7
                        [<ffffffff8020a94d>] call_softirq+0x1d/0x28
                        [<ffffffff8022e0bc>] irq_exit+0x56/0x59
                        [<ffffffff802146ee>] smp_apic_timer_interrupt+0x59/0x5f
                        [<ffffffff8020a2c1>] apic_timer_interrupt+0x69/0x70
   hardirq-on-W at:
                        [<ffffffff8024172a>] lock_acquire+0x4a/0x69
                        [<ffffffff803eb349>] _spin_lock_bh+0x26/0x33
                        [<ffffffff803c6de8>] udp_ioctl+0x46/0x87
                        [<ffffffff803cd1ab>] inet_ioctl+0x8c/0x8f
                        [<ffffffff8038203c>] sock_ioctl+0x1c0/0x1ea
                        [<ffffffff8028591e>] do_ioctl+0x26/0x74
                        [<ffffffff80285bb6>] vfs_ioctl+0x24a/0x264
                        [<ffffffff80285c11>] sys_ioctl+0x41/0x68
                        [<ffffffff802096a1>] system_call+0x7d/0x83
 }
 ... key      at: [<ffffffff807ccac8>] skb_queue_lock_key+0x0/0x18

stack backtrace:

Call Trace:
 [<ffffffff8020ac99>] show_trace+0xaa/0x238
 [<ffffffff8020b037>] dump_stack+0x13/0x15
 [<ffffffff8023ff9e>] check_usage+0x282/0x293
 [<ffffffff802412b7>] __lock_acquire+0x85c/0xa29
 [<ffffffff8024172b>] lock_acquire+0x4b/0x69
 [<ffffffff803eb7db>] _spin_lock_irqsave+0x2c/0x3c
 [<ffffffff8038683e>] skb_queue_tail+0x1d/0x47
 [<ffffffff881efecc>] :ib_ipoib:ipoib_mcast_send+0x19a/0x413
 [<ffffffff881eda0f>] :ib_ipoib:ipoib_start_xmit+0x362/0x66d
 [<ffffffff8038dfc9>] dev_hard_start_xmit+0x1ac/0x221
 [<ffffffff8039a025>] __qdisc_run+0xfb/0x1cd
 [<ffffffff8038e175>] dev_queue_xmit+0x137/0x263
 [<ffffffff8039020a>] neigh_connected_output+0xaf/0xc7
 [<ffffffff8813f4df>] :ipv6:ip6_output2+0x255/0x28c
 [<ffffffff8813fcf9>] :ipv6:ip6_output+0x7e3/0x7f8
 [<ffffffff8814d737>] :ipv6:ndisc_send_rs+0x33e/0x46f
 [<ffffffff881427ba>] :ipv6:addrconf_dad_completed+0x91/0xe2
 [<ffffffff88144fc2>] :ipv6:addrconf_dad_timer+0x75/0x11e
 [<ffffffff80231a8f>] run_timer_softirq+0x151/0x1dc
 [<ffffffff8022e346>] __do_softirq+0x6c/0xf7
 [<ffffffff8020a94e>] call_softirq+0x1e/0x28
ib0: no IPv6 routers present

Re: [openib-general] ipoib lockdep warning

[Arjan van de Ven]

On Tue, 2006-07-11 at 19:33 -0700, Roland Dreier wrote:
> OK, the patch to lib/idr.c below is at least one way to fix this.
> However, with that applied I get the lockdep warning below, which
> seems to be a false positive -- I'm not sure what the right fix is,
> but the blame really seems to fall on udp_ioctl for poking into the
> sk_buff_head lock itself.

this does not have to be a false positive!
It is not legal to take ANY non-hardirq safe lock after having taken a
lock that's used in hardirq context.
(having said that the skb_queue_tail lock needs a  special treatment for
some real false positives; Linus merged that already)

> And here's the warning I get, which appears to be a false positive:
> 
> ======================================================
> [ INFO: hard-safe -> hard-unsafe lock order detected ]
> ------------------------------------------------------
> swapper/0 [HC0[0]:SC1[2]:HE0:SE0] is trying to acquire:
>  (&skb_queue_lock_key){-+..}, at: [<ffffffff8038683e>] skb_queue_tail+0x1d/0x47
> 
> and this task is already holding:
>  (&priv->lock){.+..}, at: [<ffffffff881efd5b>] ipoib_mcast_send+0x29/0x413 [ib_ipoib]
> which would create a new lock dependency:
>  (&priv->lock){.+..} -> (&skb_queue_lock_key){-+..}
> 

Lets call priv->lock lock A, and the skb_queue_lock lock B to keep this
short


CPU 0			CPU 1
soft irq context	user or softirq context

take lock B
(anywhere in net stack)
			take lock A 
			(ipoib_mcast_send)

interrupt happens

take lock A (spins)
in ISR somewhere

			take lock B (spins)
			(in skb_queue_tail() called from
			ipoib_mcast_send)




Now this assumes your queue is shared with the networking stack,
so that the first "take lock B" on CPU 0 can happen without disabling interrupts.
(as several of the SKB functions do that get used by the networking stack, but not
 the simple ones used by drivers for private queues).
If this queue used in ipoib_mcast_send() is a private queue you probably want to 
apply the patch below (already in/on its way to mainline)

Index: linux-2.6.18-rc1/include/linux/skbuff.h
===================================================================
--- linux-2.6.18-rc1.orig/include/linux/skbuff.h
+++ linux-2.6.18-rc1/include/linux/skbuff.h
@@ -606,10 +606,17 @@ static inline __u32 skb_queue_len(const 
 
 extern struct lock_class_key skb_queue_lock_key;
 
+/*
+ * This function creates a split out lock class for each invocation;
+ * this is needed for now since a whole lot of users of the skb-queue
+ * infrastructure in drivers have different locking usage (in hardirq)
+ * than the networking core (in softirq only). In the long run either the
+ * network layer or drivers should need annotation to consolidate the
+ * main types of usage into 3 classes.
+ */
 static inline void skb_queue_head_init(struct sk_buff_head *list)
 {
 	spin_lock_init(&list->lock);
-	lockdep_set_class(&list->lock, &skb_queue_lock_key);
 	list->prev = list->next = (struct sk_buff *)list;
 	list->qlen = 0;
 }

Re: [openib-general] ipoib lockdep warning

[Roland Dreier]

 > this does not have to be a false positive!
 > It is not legal to take ANY non-hardirq safe lock after having taken a
 > lock that's used in hardirq context.
 > (having said that the skb_queue_tail lock needs a  special treatment for
 > some real false positives; Linus merged that already)

 ...

 > Now this assumes your queue is shared with the networking stack,

Right, understood -- but ipoib has a private skb queue.  Which is why
I said it was a false positive.

Thanks,
  Roland