From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 279EB959E for ; Thu, 22 Oct 2015 13:29:55 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga103.fm.intel.com with ESMTP; 22 Oct 2015 04:29:54 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,181,1444719600"; d="scan'208";a="832505087" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.66.49]) by orsmga002.jf.intel.com with ESMTP; 22 Oct 2015 04:29:52 -0700 Date: Thu, 22 Oct 2015 19:30:21 +0800 From: Yuanhan Liu To: "Xie, Huawei" Message-ID: <20151022113021.GS3115@yliu-dev.sh.intel.com> References: <1445399294-18826-1-git-send-email-yuanhan.liu@linux.intel.com> <1445399294-18826-4-git-send-email-yuanhan.liu@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Cc: "dev@dpdk.org" , "marcel@redhat.com" , "Michael S. Tsirkin" Subject: Re: [dpdk-dev] [PATCH v7 3/8] vhost: vring queue setup for multiple queue support X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Oct 2015 11:29:55 -0000 On Thu, Oct 22, 2015 at 09:49:58AM +0000, Xie, Huawei wrote: > On 10/21/2015 11:48 AM, Yuanhan Liu wrote: > > All queue pairs, including the default (the first) queue pair, > > are allocated dynamically, when a vring_call message is received > > first time for a specific queue pair. > > > > This is a refactor work for enabling vhost-user multiple queue; > > it should not break anything as it does no functional changes: > > we don't support mq set, so there is only one mq at max. > > > > This patch is based on Changchun's patch. > > > [...] > > > > void > > @@ -290,13 +298,9 @@ user_get_vring_base(struct vhost_device_ctx ctx, > > * sent and only sent in vhost_vring_stop. > > * TODO: cleanup the vring, it isn't usable since here. > > */ > > - if ((dev->virtqueue[VIRTIO_RXQ]->kickfd) >= 0) { > > - close(dev->virtqueue[VIRTIO_RXQ]->kickfd); > > - dev->virtqueue[VIRTIO_RXQ]->kickfd = -1; > > - } > > - if ((dev->virtqueue[VIRTIO_TXQ]->kickfd) >= 0) { > > - close(dev->virtqueue[VIRTIO_TXQ]->kickfd); > > - dev->virtqueue[VIRTIO_TXQ]->kickfd = -1; > > + if ((dev->virtqueue[state->index]->kickfd) >= 0) { > > + close(dev->virtqueue[state->index]->kickfd); > > + dev->virtqueue[state->index]->kickfd = -1; > > } > Since we change the behavior here, better list in the commit message as > well. I checked the code again, and found I should not change that: GET_VRING_BASE is sent per virt queue pair. BTW, it's wrong to do this kind of stuff here, we need fix it in future. > > > > > > > @@ -680,13 +704,21 @@ set_vring_call(struct vhost_device_ctx ctx, struct vhost_vring_file *file) > > { > > struct virtio_net *dev; > > struct vhost_virtqueue *vq; > > + uint32_t cur_qp_idx = file->index / VIRTIO_QNUM; > > > > dev = get_device(ctx); > > if (dev == NULL) > > return -1; > > > > + /* alloc vring queue pair if it is a new queue pair */ > > + if (cur_qp_idx + 1 > dev->virt_qp_nb) { > > + if (alloc_vring_queue_pair(dev, cur_qp_idx) < 0) > > + return -1; > > + } > > + > Here we rely on the fact that this set_vring_call message is sent in the > continuous ascending order of queue idx 0, 1, 2, ... That's true. > > > /* file->index refers to the queue index. The txq is 1, rxq is 0. */ > > vq = dev->virtqueue[file->index]; > > + assert(vq != NULL); > > > If we allocate the queue until the we receive the first vring message, > better add comment that we rely on this fact. Will do that. > Could we add the vhost-user message to tell us the queue number QEMU > allocates before vring message? We may need do that. But it's too late to make it in v2.2 --yliu > > if (vq->callfd >= 0) > > close(vq->callfd); >