From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Miller Subject: Re: [PATCH net-next] virtio_net: Disable interrupts if napi_complete_done rescheduled napi Date: Fri, 08 Dec 2017 13:19:23 -0500 (EST) Message-ID: <20171208.131923.1211894626989773629.davem@davemloft.net> References: <1512620115-7569-1-git-send-email-makita.toshiaki@lab.ntt.co.jp> <20171207070618-mutt-send-email-mst@kernel.org> Mime-Version: 1.0 Content-Type: Text/Plain; charset=iso-2022-jp Content-Transfer-Encoding: 7bit Cc: mst@redhat.com, makita.toshiaki@lab.ntt.co.jp, netdev@vger.kernel.org, virtualization@lists.linux-foundation.org, edumazet@google.com To: jasowang@redhat.com Return-path: Received: from shards.monkeyblade.net ([184.105.139.130]:56882 "EHLO shards.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752111AbdLHSTZ (ORCPT ); Fri, 8 Dec 2017 13:19:25 -0500 In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: From: Jason Wang Date: Thu, 7 Dec 2017 15:08:45 +0800 > > > On 2017年12月07日 13:09, Michael S. Tsirkin wrote: >> On Thu, Dec 07, 2017 at 01:15:15PM +0900, Toshiaki Makita wrote: >>> Since commit 39e6c8208d7b ("net: solve a NAPI race") napi has been >>> able >>> to be rescheduled within napi_complete_done() even in non-busypoll >>> case, >>> but virtnet_poll() always enabled interrupts before complete, and when >>> napi was rescheduled within napi_complete_done() it did not disable >>> interrupts. >>> This caused more interrupts when event idx is disabled. >>> >>> According to commit cbdadbbf0c79 ("virtio_net: fix race in RX VQ >>> processing") we cannot place virtqueue_enable_cb_prepare() after >>> NAPI_STATE_SCHED is cleared, so disable interrupts again if >>> napi_complete_done() returned false. >>> >>> Tested with vhost-user of OVS 2.7 on host, which does not have the >>> event >>> idx feature. ... >>> Signed-off-by: Toshiaki Makita >> Acked-by: Michael S. Tsirkin >> >> it might make sense in net and not -next since tx napi regressed >> performance >> in some configs, this might bring it back at least partially. >> Jason - what do you think? > > No sure, the regression I saw was tested with event idx on. And > virtqueue_disable_cb() does almost nothing for event idx (or even a > little bit slower). > > The patch looks good. > > Acked-by: Jason Wang I'm going to put this into net-next for now.