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;
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
