[PATCH] dst: call cond_resched() in dst_gc_task()

[Eric Dumazet <eric.dumazet@gmail.com>]

Le lundi 08 février 2010 à 15:32 +0100, Eric Dumazet a écrit :
> Le lundi 08 février 2010 à 15:16 +0100, Paweł Staszewski a écrit :
> 
> > >    
> > Some day ago after info about route cache i was have  also this info:
> 
> > Code: fe 79 4c 00 48 85 db 74 14 48 8b 74 24 10 48 89 ef ff 13 48 83 c3 08 48
> > 83 3b 00 eb ea 48 83 c4 18 5b 5d 41 5c 41 5d 41 5e 41 5f<c3>  55 48 89 f5 53 48
> > 89 fb 48 83 ec 08 48 8b 76 18 48 2b 75 10
> > Call Trace:
> >   <IRQ>   [<ffffffff8126826f>] ? e1000_put_txbuf+0x62/0x74
> >   [<ffffffff8126834a>] ? e1000_clean_tx_irq+0xc9/0x235
> >   [<ffffffff8126b71b>] ? e1000_clean+0x5c/0x21c
> >   [<ffffffff812f29a3>] ? net_rx_action+0x71/0x15d
> >   [<ffffffff81035311>] ? __do_softirq+0xd7/0x196
> >   [<ffffffff81002dac>] ? call_softirq+0x1c/0x28
> >   [<ffffffff812f768f>] ? dst_gc_task+0x0/0x1a7
> >   [<ffffffff81002dac>] ? call_softirq+0x1c/0x28
> >   <EOI>   [<ffffffff81004599>] ? do_softirq+0x31/0x63
> >   [<ffffffff81034ec1>] ? local_bh_enable_ip+0x75/0x86
> >   [<ffffffff812f768f>] ? dst_gc_task+0x0/0x1a7
> >   [<ffffffff812f775d>] ? dst_gc_task+0xce/0x1a7
> >   [<ffffffff8136b08c>] ? schedule+0x82c/0x906
> >   [<ffffffff8103c44f>] ? lock_timer_base+0x26/0x4b
> >   [<ffffffff810a41d6>] ? cache_reap+0x0/0x11d
> >   [<ffffffff81044c38>] ? worker_thread+0x14c/0x1dc
> >   [<ffffffff81047dcd>] ? autoremove_wake_function+0x0/0x2e
> >   [<ffffffff81044aec>] ? worker_thread+0x0/0x1dc
> >   [<ffffffff810479bd>] ? kthread+0x79/0x81
> >   [<ffffffff81002cb4>] ? kernel_thread_helper+0x4/0x10
> >   [<ffffffff81047944>] ? kthread+0x0/0x81
> > 
> > 
> >   [<ffffffff81002cb0>] ? kernel_thread_helper+0x0/0x10
> > 
> > 
> 
> This trace is indeed very interesting, since dst_gc_task() is run from a
> work queue, and there is no scheduling point in it.
> 
> We might need add a scheduling point in dst_gc_task() in case huge
> number of entries were flushed.
> 

David, here is the patch I sent to Pawel to solve this problem.
This probaby is a stable candidate.

Thanks

[PATCH] dst: call cond_resched() in dst_gc_task()

On some workloads, it is quite possible to get a huge dst list to
process in dst_gc_task(), and trigger soft lockup detection.

Fix is to call cond_resched(), as we run in process context.

Reported-by: Pawel Staszewski <pstaszewski@itcare.pl>
Tested-by: Pawel Staszewski <pstaszewski@itcare.pl>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---

diff --git a/net/core/dst.c b/net/core/dst.c
index 57bc4d5..cb1b348 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -17,6 +17,7 @@
 #include <linux/string.h>
 #include <linux/types.h>
 #include <net/net_namespace.h>
+#include <linux/sched.h>
 
 #include <net/dst.h>
 
@@ -79,6 +80,7 @@ loop:
 	while ((dst = next) != NULL) {
 		next = dst->next;
 		prefetch(&next->next);
+		cond_resched();
 		if (likely(atomic_read(&dst->__refcnt))) {
 			last->next = dst;
 			last = dst;