[v3] vhost: make iotlb cache name unique among multi processes

Message ID 20200311231918.20701-1-oda@valinux.co.jp (mailing list archive)
State Accepted, archived
Delegated to: Maxime Coquelin
Headers
Series [v3] vhost: make iotlb cache name unique among multi processes |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-testing fail Testing issues
ci/iol-mellanox-Performance success Performance Testing PASS
ci/travis-robot success Travis build: passed
ci/Intel-compilation success Compilation OK

Commit Message

Itsuro Oda March 11, 2020, 11:19 p.m. UTC
  Currently, iotlb cache name is comprised of vid and virtqueue
index. For example, "iotlb_cache_0_0". Because vid is assigned
per process, iotlb cache name is not unique among multi processes.
For example a secondary process uses a vhost
(ex. eth_vhost0,iface=/tmp/sock0) and another secondary process
uses a vhost (ex. eth_vhost1,iface=/tmp/sock1), iotlb cache
name of both vhost ("iotlb_cache_0_0") are same and as a result
iotlb cache is broken.

This patch makes iotlb cache name unique among milti processes
by adding process id to the iotlb cache name.

The prefix of the name is shortend to "iotlb_" since the maximum
length of pool name is 25 bytes (RTE_MEMPOOL_NAMESIZE is 26).
Note that it is just 25 characters in maximum at the moment.
Here,
* pid_t == int: max 10 digits.
* vid < MAX_VHOST_DECICE(1024): max 4 digits.
* vq_index < VHOST_MAX_VRING(256): max 3 digits.

Fixes: d012d1f293f4 (vhost: add IOTLB helper functions)
Cc: stable@dpdk.org

Signed-off-by: Itsuro Oda <oda@valinux.co.jp>
---
v3:
* change format.
* fix commit message.

v2:
* use the process id to make the iotlb cache name unique.

 lib/librte_vhost/iotlb.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
  

Comments

Maxime Coquelin April 9, 2020, 2:25 p.m. UTC | #1
On 3/12/20 12:19 AM, Itsuro Oda wrote:
> Currently, iotlb cache name is comprised of vid and virtqueue
> index. For example, "iotlb_cache_0_0". Because vid is assigned
> per process, iotlb cache name is not unique among multi processes.
> For example a secondary process uses a vhost
> (ex. eth_vhost0,iface=/tmp/sock0) and another secondary process
> uses a vhost (ex. eth_vhost1,iface=/tmp/sock1), iotlb cache
> name of both vhost ("iotlb_cache_0_0") are same and as a result
> iotlb cache is broken.
> 
> This patch makes iotlb cache name unique among milti processes
> by adding process id to the iotlb cache name.
> 
> The prefix of the name is shortend to "iotlb_" since the maximum
> length of pool name is 25 bytes (RTE_MEMPOOL_NAMESIZE is 26).
> Note that it is just 25 characters in maximum at the moment.
> Here,
> * pid_t == int: max 10 digits.
> * vid < MAX_VHOST_DECICE(1024): max 4 digits.
> * vq_index < VHOST_MAX_VRING(256): max 3 digits.
> 
> Fixes: d012d1f293f4 (vhost: add IOTLB helper functions)
> Cc: stable@dpdk.org
> 
> Signed-off-by: Itsuro Oda <oda@valinux.co.jp>
> ---
> v3:
> * change format.
> * fix commit message.
> 
> v2:
> * use the process id to make the iotlb cache name unique.
> 
>  lib/librte_vhost/iotlb.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 


Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime
  
Maxime Coquelin April 10, 2020, 2:45 p.m. UTC | #2
On 3/12/20 12:19 AM, Itsuro Oda wrote:
> Currently, iotlb cache name is comprised of vid and virtqueue
> index. For example, "iotlb_cache_0_0". Because vid is assigned
> per process, iotlb cache name is not unique among multi processes.
> For example a secondary process uses a vhost
> (ex. eth_vhost0,iface=/tmp/sock0) and another secondary process
> uses a vhost (ex. eth_vhost1,iface=/tmp/sock1), iotlb cache
> name of both vhost ("iotlb_cache_0_0") are same and as a result
> iotlb cache is broken.
> 
> This patch makes iotlb cache name unique among milti processes
> by adding process id to the iotlb cache name.
> 
> The prefix of the name is shortend to "iotlb_" since the maximum
> length of pool name is 25 bytes (RTE_MEMPOOL_NAMESIZE is 26).
> Note that it is just 25 characters in maximum at the moment.
> Here,
> * pid_t == int: max 10 digits.
> * vid < MAX_VHOST_DECICE(1024): max 4 digits.
> * vq_index < VHOST_MAX_VRING(256): max 3 digits.
> 
> Fixes: d012d1f293f4 (vhost: add IOTLB helper functions)
> Cc: stable@dpdk.org
> 
> Signed-off-by: Itsuro Oda <oda@valinux.co.jp>
> ---
> v3:
> * change format.
> * fix commit message.
> 
> v2:
> * use the process id to make the iotlb cache name unique.
> 
>  lib/librte_vhost/iotlb.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
Applied to dpdk-next-virtio/master.

Thanks,
Maxime
  

Patch

diff --git a/lib/librte_vhost/iotlb.c b/lib/librte_vhost/iotlb.c
index bc1758528..5b3a0c090 100644
--- a/lib/librte_vhost/iotlb.c
+++ b/lib/librte_vhost/iotlb.c
@@ -308,8 +308,9 @@  vhost_user_iotlb_init(struct virtio_net *dev, int vq_index)
 	TAILQ_INIT(&vq->iotlb_list);
 	TAILQ_INIT(&vq->iotlb_pending_list);
 
-	snprintf(pool_name, sizeof(pool_name), "iotlb_cache_%d_%d",
-			dev->vid, vq_index);
+	snprintf(pool_name, sizeof(pool_name), "iotlb_%u_%d_%d",
+			getpid(), dev->vid, vq_index);
+	VHOST_LOG_CONFIG(DEBUG, "IOTLB cache name: %s\n", pool_name);
 
 	/* If already created, free it and recreate */
 	vq->iotlb_pool = rte_mempool_lookup(pool_name);