From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <yuanhan.liu@linux.intel.com>
Received: from mga14.intel.com (mga14.intel.com [192.55.52.115])
by dpdk.org (Postfix) with ESMTP id 279EB959E
for <dev@dpdk.org>; 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 <yuanhan.liu@linux.intel.com>
To: "Xie, Huawei" <huawei.xie@intel.com>
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>
<C37D651A908B024F974696C65296B57B4B136D58@SHSMSX101.ccr.corp.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <C37D651A908B024F974696C65296B57B4B136D58@SHSMSX101.ccr.corp.intel.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: "dev@dpdk.org" <dev@dpdk.org>, "marcel@redhat.com" <marcel@redhat.com>,
"Michael S. Tsirkin" <mst@redhat.com>
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 <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=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);
>