From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:52906) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1URbyf-0003IX-Ik for qemu-devel@nongnu.org; Mon, 15 Apr 2013 01:29:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1URbyd-0004MC-SV for qemu-devel@nongnu.org; Mon, 15 Apr 2013 01:29:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40872) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1URbyd-0004M1-KV for qemu-devel@nongnu.org; Mon, 15 Apr 2013 01:29:15 -0400 Message-ID: <516B901D.7030708@redhat.com> Date: Mon, 15 Apr 2013 13:29:01 +0800 From: Jason Wang MIME-Version: 1.0 References: <1359110143-42984-1-git-send-email-jasowang@redhat.com> <1359110143-42984-19-git-send-email-jasowang@redhat.com> <20130413131715.GA17977@hall.aurel32.net> In-Reply-To: <20130413131715.GA17977@hall.aurel32.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH V2 18/20] virtio-net: multiqueue support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aurelien Jarno Cc: qemu-devel@nongnu.org On 04/13/2013 09:17 PM, Aurelien Jarno wrote: > On Fri, Jan 25, 2013 at 06:35:41PM +0800, Jason Wang wrote: >> This patch implements both userspace and vhost support for multiple queue >> virtio-net (VIRTIO_NET_F_MQ). This is done by introducing an array of >> VirtIONetQueue to VirtIONet. >> >> Signed-off-by: Jason Wang >> --- >> hw/virtio-net.c | 317 +++++++++++++++++++++++++++++++++++++++++++------------ >> hw/virtio-net.h | 28 +++++- >> 2 files changed, 275 insertions(+), 70 deletions(-) > This patch breaks virtio-net in Minix, even with multiqueue disable. I > don't know virtio enough to know if it is a Minix or a QEMU problem. > However I have been able to identify the part of the commit causing the > failure: Hi Aurelien: Thanks for the work. > >> diff --git a/hw/virtio-net.c b/hw/virtio-net.c >> index ef522d5..cec91a7 100644 >> --- a/hw/virtio-net.c >> +++ b/hw/virtio-net.c > ... > >> +static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int ctrl) >> +{ >> + VirtIODevice *vdev = &n->vdev; >> + int i, max = multiqueue ? n->max_queues : 1; >> + >> + n->multiqueue = multiqueue; >> + >> + for (i = 2; i <= n->max_queues * 2 + 1; i++) { >> + virtio_del_queue(vdev, i); >> + } >> + > The for loop above is something which is new, even with multiqueue > disabled. Even with max_queues=1 it calls virtio_del_queue with i = 2 > and i = 3. Disabling this loop makes the code to work as before. Looks like a bug here, need to change n->max_queues * 2 + 1 to n->max_queues * 2. The reason we need to del queue 2 each time because vq 2 has different meaning is multiqueue and single queue. In single queue, vq 2 maybe ctrl vq, but in multiqueue mode it was rx1. Let's see whether this small change works. > > On the Minix side it triggers the following assertion: > > | virtio.c:370: assert "q->vaddr != NULL" failed, function "free_phys_queue" > | virtio_net(73141): panic: assert failed > > This correspond to this function in lib/libvirtio/virtio.c: > > | static void > | free_phys_queue(struct virtio_queue *q) > | { > | assert(q != NULL); > | assert(q->vaddr != NULL); > | > | free_contig(q->vaddr, q->ring_size); > | q->vaddr = NULL; > | q->paddr = 0; > | q->num = 0; > | free_contig(q->data, sizeof(q->data[0])); > | q->data = NULL; > | } > > Do you have an idea if the problem is on the Minix side or on the QEMU > side? > Haven't figured out the relationship between virtqueue dynamic del/add and q->vaddr here. If the above changes does not work, I guess this problem happens only for ctrl vq (vq2)? And when does this happen? Rebooting? Thanks a lot