nets: fix problem of using lock
Subsystems (2)
PATCH 1/1 nets: fix problem of using lock
net/core/skbuff.c: void skb_queue_purge(struct sk_buff_head *list)
This function should takes the the list lock, because the operation to this list shoule be atomic. And __skb_queue_purge() (in include/linux/skbuff.c) real delete the buffers in the list.
--- net/core/skbuff.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-)
net/core/skbuff.c +5 -3
@@ -1834,9 +1834,11 @@ struct sk_buff *skb_dequeue_tail(struct sk_buff_head *list)*/ void skb_queue_purge (struct sk_buff_head *list) { -struct sk_buff *skb; -while ((skb =skb_dequeue (list)) !=NULL ) -kfree_skb (skb); +unsigned long flags; + +spin_lock_irqsave (&list->lock, flags); +__skb_queue_purge (list); +spin_unlock_irqrestore (&list->lock, flags); }/**
Signature · 1 line
1.5.6.3
David Miller
maintainerFrom: Jianjun Kong <[email protected]>
Date: Thu, 6 Nov 2008 18:41:03 +0800
Quoted 7 lines
net/core/skbuff.c: voidskb_queue_purge(struct sk_buff_head *list)This function should takes the the list lock, because the operation to this list shoule be atomic. And
__skb_queue_purge()(ininclude/linux/skbuff.c) real delete the buffers in the list.Signed-off-by Jianjun Kong <[email protected]>
No, this function is fine. skb_dequeue() takes the lock so there cannot be any list corruption.
And this function is called in contexts where the caller knows that no new packets can be added to the list (closing a socket, shutting down a device, etc.) And even if new packets could appear, taking the lock over the entire function would not help that problem.
In fact, I suspect that many if not all skb_queue_purge() callers can be converted to use __skb_queue_purge().
Quoted 14 lines · David Miller
From: Jianjun Kong <[email protected]>
Date: Thu, 6 Nov 2008 18:41:03 +0800Quoted 7 lines
net/core/skbuff.c: voidskb_queue_purge(struct sk_buff_head *list)This function should takes the the list lock, because the operation to this list shoule be atomic. And
__skb_queue_purge()(ininclude/linux/skbuff.c) real delete the buffers in the list.Signed-off-by Jianjun Kong <[email protected]>No, this function is fine.
skb_dequeue()takes the lock so there cannot be any list corruption.
Quoted 8 lines
And this function is called in contexts where the caller knows that no new packets can be added to the list (closing a socket, shutting down a device, etc.) And even if new packets could appear, taking the lock over the entire function would not help that problem.
In fact, I suspect that many if not all
skb_queue_purge()callers can be converted to use__skb_queue_purge().
Thanks, I've known :-)
Signature · 3 lines
Jianjun Kong @_@ Happy Hacking Homepage: http://kongove.cn/ Gtalk: [email protected]