From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christian Borntraeger Subject: Re: virtio_net and SMP guests Date: Thu, 10 Jan 2008 16:51:58 +0100 Message-ID: <200801101651.58908.borntraeger@de.ibm.com> References: <200712141312.05562.borntraeger@de.ibm.com> <200801101337.40433.borntraeger@de.ibm.com> <200801101639.15995.borntraeger@de.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Cc: kvm-devel , Anthony Liguori , Dor Laor , netdev@vger.kernel.org To: virtualization@lists.linux-foundation.org Return-path: In-Reply-To: <200801101639.15995.borntraeger@de.ibm.com> Content-Disposition: inline Sender: netdev-owner@vger.kernel.org List-Id: kvm.vger.kernel.org Am Donnerstag, 10. Januar 2008 schrieb Christian Borntraeger: > Am Donnerstag, 10. Januar 2008 schrieb Christian Borntraeger: > > Am Dienstag, 18. Dezember 2007 schrieb Rusty Russell: > > > To me this points to doing interrupt suppression a different way. If we > > > have a ->disable_cb() virtio function, and call it before we call > > > netif_rx_schedule, does that fix it? > > > > The fix looks good and I agree with it. > > > > There is one problem that I try to find for some days, but the following > > BUG_ON triggers: > > > > static void vring_disable_cb(struct virtqueue *_vq) > > { > > struct vring_virtqueue *vq = to_vvq(_vq); > > > > START_USE(vq); > > ----> BUG_ON(vq->vring.avail->flags & VRING_AVAIL_F_NO_INTERRUPT); > > vq->vring.avail->flags |= VRING_AVAIL_F_NO_INTERRUPT; > > END_USE(vq); > > } > > Ok, I found it: > > static int virtnet_open(struct net_device *dev) > { > struct virtnet_info *vi = netdev_priv(dev); > try_fill_recv(vi); > /* If we didn't even get one input buffer, we're useless. */ > if (vi->num == 0) > return -ENOMEM; > ---> int for new packet > static void skb_recv_done(struct virtqueue *rvq) > { > struct virtnet_info *vi = rvq->vdev->priv; > /* Suppress further interrupts. */ > rvq->vq_ops->disable_cb(rvq); > netif_rx_schedule(vi->dev, &vi->napi); > } > - poll is not yet possible, no softirq > - return from interrupt > napi_enable(&vi->napi); > vi->rvq->vq_ops->disable_cb(vi->rvq); > ---> BUG: its already disabled Btw. this problem also happens on single processor guests. What about the following patch: --- drivers/net/virtio_net.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) Index: kvm/drivers/net/virtio_net.c =================================================================== --- kvm.orig/drivers/net/virtio_net.c +++ kvm/drivers/net/virtio_net.c @@ -179,9 +179,12 @@ static void try_fill_recv(struct virtnet static void skb_recv_done(struct virtqueue *rvq) { struct virtnet_info *vi = rvq->vdev->priv; - /* Suppress further interrupts. */ - rvq->vq_ops->disable_cb(rvq); - netif_rx_schedule(vi->dev, &vi->napi); + /* Schedule NAPI, Suppress further interrupts if successful. */ + + if (netif_rx_schedule_prep(vi->dev, &vi->napi)) { + rvq->vq_ops->disable_cb(rvq); + __netif_rx_schedule(vi->dev, &vi->napi); + } } static int virtnet_poll(struct napi_struct *napi, int budget)