netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] virtio_net: invoke softirqs after __napi_schedule
@ 2012-05-16  7:57 Michael S. Tsirkin
  2012-05-17  3:32 ` Rusty Russell
  0 siblings, 1 reply; 3+ messages in thread
From: Michael S. Tsirkin @ 2012-05-16  7:57 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, virtualization, linux-kernel, Michael S. Tsirkin

__napi_schedule might raise softirq but nothing
causes do_softirq to trigger, so it does not in fact
run. As a result,
the error message "NOHZ: local_softirq_pending 08"
sometimes occurs during boot of a KVM guest when the network service is
started and we are oom:

  ...
  Bringing up loopback interface:  [  OK  ]
  Bringing up interface eth0:
  Determining IP information for eth0...NOHZ: local_softirq_pending 08
   done.
  [  OK  ]
  ...

Further, receive queue processing might get delayed
indefinitely until some interrupt triggers:
virtio_net expected napi to be run immediately.

One way to cause do_softirq to be executed is by
invoking local_bh_enable(). As __napi_schedule is
normally called from bh or irq context, this
seems to make sense: disable bh before __napi_schedule
and enable afterwards.

Reported-by: Ulrich Obergfell <uobergfe@redhat.com>
Tested-by: Ulrich Obergfell <uobergfe@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

To test, one can hack try_fill_recv to always report oom.
I'm not sure it's not too late for 3.4, but we can try.
Rusty, could you review ASAP pls?

 drivers/net/virtio_net.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index af8acc8..cbefe67 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -492,7 +492,9 @@ static void virtnet_napi_enable(struct virtnet_info *vi)
 	 * We synchronize against interrupts via NAPI_STATE_SCHED */
 	if (napi_schedule_prep(&vi->napi)) {
 		virtqueue_disable_cb(vi->rvq);
+		local_bh_disable();
 		__napi_schedule(&vi->napi);
+		local_bh_enable();
 	}
 }
 
-- 
MST

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] virtio_net: invoke softirqs after __napi_schedule
  2012-05-16  7:57 [PATCH] virtio_net: invoke softirqs after __napi_schedule Michael S. Tsirkin
@ 2012-05-17  3:32 ` Rusty Russell
  2012-05-17  3:40   ` David Miller
  0 siblings, 1 reply; 3+ messages in thread
From: Rusty Russell @ 2012-05-17  3:32 UTC (permalink / raw)
  To: Michael S. Tsirkin, David Miller
  Cc: netdev, virtualization, linux-kernel, Michael S. Tsirkin

On Wed, 16 May 2012 10:57:13 +0300, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> __napi_schedule might raise softirq but nothing
> causes do_softirq to trigger, so it does not in fact
> run. As a result,
> the error message "NOHZ: local_softirq_pending 08"
> sometimes occurs during boot of a KVM guest when the network service is
> started and we are oom:
> 
>   ...
>   Bringing up loopback interface:  [  OK  ]
>   Bringing up interface eth0:
>   Determining IP information for eth0...NOHZ: local_softirq_pending 08
>    done.
>   [  OK  ]
>   ...
> 
> Further, receive queue processing might get delayed
> indefinitely until some interrupt triggers:
> virtio_net expected napi to be run immediately.
> 
> One way to cause do_softirq to be executed is by
> invoking local_bh_enable(). As __napi_schedule is
> normally called from bh or irq context, this
> seems to make sense: disable bh before __napi_schedule
> and enable afterwards.
> 
> Reported-by: Ulrich Obergfell <uobergfe@redhat.com>
> Tested-by: Ulrich Obergfell <uobergfe@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
> To test, one can hack try_fill_recv to always report oom.
> I'm not sure it's not too late for 3.4, but we can try.
> Rusty, could you review ASAP pls?

It's missing a big comment: it's a very complicated way of calling
do_softirq().

Indeed, this function is only used when we are not in interrupt
context.  It's not hot at all, in any ideal scenario.

Acked-by: Rusty Russell <rusty@rustcorp.com.au>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] virtio_net: invoke softirqs after __napi_schedule
  2012-05-17  3:32 ` Rusty Russell
@ 2012-05-17  3:40   ` David Miller
  0 siblings, 0 replies; 3+ messages in thread
From: David Miller @ 2012-05-17  3:40 UTC (permalink / raw)
  To: rusty; +Cc: netdev, virtualization, linux-kernel, mst

From: Rusty Russell <rusty@rustcorp.com.au>
Date: Thu, 17 May 2012 13:02:53 +0930

> On Wed, 16 May 2012 10:57:13 +0300, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> __napi_schedule might raise softirq but nothing
>> causes do_softirq to trigger, so it does not in fact
>> run. As a result,
>> the error message "NOHZ: local_softirq_pending 08"
>> sometimes occurs during boot of a KVM guest when the network service is
>> started and we are oom:
>> 
>>   ...
>>   Bringing up loopback interface:  [  OK  ]
>>   Bringing up interface eth0:
>>   Determining IP information for eth0...NOHZ: local_softirq_pending 08
>>    done.
>>   [  OK  ]
>>   ...
>> 
>> Further, receive queue processing might get delayed
>> indefinitely until some interrupt triggers:
>> virtio_net expected napi to be run immediately.
>> 
>> One way to cause do_softirq to be executed is by
>> invoking local_bh_enable(). As __napi_schedule is
>> normally called from bh or irq context, this
>> seems to make sense: disable bh before __napi_schedule
>> and enable afterwards.
>> 
>> Reported-by: Ulrich Obergfell <uobergfe@redhat.com>
>> Tested-by: Ulrich Obergfell <uobergfe@redhat.com>
>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
 ...
> Acked-by: Rusty Russell <rusty@rustcorp.com.au>

Michael, you're best to submit this directly to Linus as I just
made what I hope is my last push to him for 3.4 today.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2012-05-17  3:40 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-16  7:57 [PATCH] virtio_net: invoke softirqs after __napi_schedule Michael S. Tsirkin
2012-05-17  3:32 ` Rusty Russell
2012-05-17  3:40   ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).