From mboxrd@z Thu Jan  1 00:00:00 1970
From: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Subject: Re: [PATCH v3 4/5] vhost: batch update used ring
Date: Thu, 25 Aug 2016 11:48:00 +0800
Message-ID: <20160825034800.GW30752@yliu-dev.sh.intel.com>
References: <1471585430-125925-1-git-send-email-zhihong.wang@intel.com>
 <1471585430-125925-5-git-send-email-zhihong.wang@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: dev@dpdk.org, maxime.coquelin@redhat.com
To: Zhihong Wang <zhihong.wang@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mga06.intel.com (mga06.intel.com [134.134.136.31])
 by dpdk.org (Postfix) with ESMTP id E1EC35954
 for <dev@dpdk.org>; Thu, 25 Aug 2016 05:37:59 +0200 (CEST)
Content-Disposition: inline
In-Reply-To: <1471585430-125925-5-git-send-email-zhihong.wang@intel.com>
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>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Fri, Aug 19, 2016 at 01:43:49AM -0400, Zhihong Wang wrote:
> This patch enables batch update of the used ring for better efficiency.
> 
> Signed-off-by: Zhihong Wang <zhihong.wang@intel.com>
...
> diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
> index 1785695..87d09fa 100644
> --- a/lib/librte_vhost/virtio-net.c
> +++ b/lib/librte_vhost/virtio-net.c
> @@ -152,10 +152,14 @@ cleanup_device(struct virtio_net *dev, int destroy)
>  static void
>  free_device(struct virtio_net *dev)
>  {
> +	struct vhost_virtqueue *vq;
>  	uint32_t i;
>  
> -	for (i = 0; i < dev->virt_qp_nb; i++)
> -		rte_free(dev->virtqueue[i * VIRTIO_QNUM]);
> +	for (i = 0; i < dev->virt_qp_nb; i++) {
> +		vq = dev->virtqueue[i * VIRTIO_QNUM];
> +		rte_free(vq->shadow_used_ring);
> +		rte_free(vq);
> +	}
>  	rte_free(dev);
>  }
> @@ -418,13 +422,18 @@ int
>  vhost_set_vring_num(int vid, struct vhost_vring_state *state)
>  {
>  	struct virtio_net *dev;
> +	struct vhost_virtqueue *vq;
>  
>  	dev = get_device(vid);
>  	if (dev == NULL)
>  		return -1;
>  
>  	/* State->index refers to the queue index. The txq is 1, rxq is 0. */
> -	dev->virtqueue[state->index]->size = state->num;
> +	vq = dev->virtqueue[state->index];
> +	vq->size = state->num;
> +	vq->shadow_used_ring = rte_malloc("",
> +			vq->size * sizeof(struct vring_used_elem),
> +			RTE_CACHE_LINE_SIZE);

Few notes here:

- I think the typical way to not specific a string type is using NULL,
  but not "".

- You should check the return value of rte_malloc: it could fail.

- Note that free_device() is invoked only when the vhost-user connection
  is broken (say the guest is halt). However, vhost_set_vring_num() could
  be invoked many times for a connection, say when you restart testpmd
  many times. This would lead to memory leak.

  The right way is to free it on get_vring_base().

	--yliu