From mboxrd@z Thu Jan  1 00:00:00 1970
From: Tiwei Bie <tiwei.bie@intel.com>
Subject: Re: [PATCH 03/21] vhost: protect virtio_net device struct
Date: Tue, 5 Sep 2017 12:45:17 +0800
Message-ID: <20170905044516.GC31895@debian-ZGViaWFuCg>
References: <20170831095023.21037-1-maxime.coquelin@redhat.com>
 <20170831095023.21037-4-maxime.coquelin@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Cc: dev@dpdk.org, yliu@fridaylinux.org, jfreiman@redhat.com, mst@redhat.com,
 vkaplans@redhat.com, jasowang@redhat.com
To: Maxime Coquelin <maxime.coquelin@redhat.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mga07.intel.com (mga07.intel.com [134.134.136.100])
 by dpdk.org (Postfix) with ESMTP id 3D36D3254
 for <dev@dpdk.org>; Tue,  5 Sep 2017 06:44:52 +0200 (CEST)
Content-Disposition: inline
In-Reply-To: <20170831095023.21037-4-maxime.coquelin@redhat.com>
List-Id: DPDK patches and discussions <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 Thu, Aug 31, 2017 at 11:50:05AM +0200, Maxime Coquelin wrote:
> virtio_net device might be accessed while being reallocated
> in case of NUMA awareness. This case might be theoretical,
> but it will be needed anyway to protect vrings pages against
> invalidation.
> 
> The virtio_net devs are now protected with a readers/writers
> lock, so that before reallocating the device, it is ensured
> that it is not being referenced by the processing threads.
> 
[...]
>  
> +struct virtio_net *
> +get_device(int vid)
> +{
> +	struct virtio_net *dev;
> +
> +	rte_rwlock_read_lock(&vhost_devices[vid].lock);
> +
> +	dev = __get_device(vid);
> +	if (unlikely(!dev))
> +		rte_rwlock_read_unlock(&vhost_devices[vid].lock);
> +
> +	return dev;
> +}
> +
> +void
> +put_device(int vid)
> +{
> +	rte_rwlock_read_unlock(&vhost_devices[vid].lock);
> +}
> +

This patch introduced a per-device rwlock which needs to be acquired
unconditionally in the data path. So for each vhost device, the IO
threads of different queues will need to acquire/release this lock
during each enqueue and dequeue operation, which will cause cache
contention when multiple queues are enabled and handled by different
cores. With this patch alone, I saw ~7% performance drop when enabling
6 queues to do 64bytes iofwd loopback test. Is there any way to avoid
introducing this lock to the data path?

Best regards,
Tiwei Bie