patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH v7 1/7] vhost: fix missing memory table NUMA realloc
       [not found] <20210629161133.79472-1-maxime.coquelin@redhat.com>
@ 2021-06-29 16:11 ` Maxime Coquelin
  2021-06-29 16:11 ` [dpdk-stable] [PATCH v7 2/7] vhost: fix missing guest pages " Maxime Coquelin
  2021-06-29 16:11 ` [dpdk-stable] [PATCH v7 4/7] vhost: fix NUMA reallocation with multiqueue Maxime Coquelin
  2 siblings, 0 replies; 6+ messages in thread
From: Maxime Coquelin @ 2021-06-29 16:11 UTC (permalink / raw)
  To: dev, chenbo.xia, david.marchand; +Cc: Maxime Coquelin, stable

When the guest allocates virtqueues on a different NUMA node
than the one the Vhost metadata are allocated, both the Vhost
device struct and the virtqueues struct are reallocated.

However, reallocating the Vhost memory table was missing, which
likely causes at least one cross-NUMA accesses for every burst
of packets.

This patch reallocates this table on the same NUMA node as the
other metadata.

Fixes: 552e8fd3d2b4 ("vhost: simplify memory regions handling")
Cc: stable@dpdk.org

Reported-by: David Marchand <david.marchand@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
---
 lib/vhost/vhost_user.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
index 8f0eba6412..b5a84f3dcd 100644
--- a/lib/vhost/vhost_user.c
+++ b/lib/vhost/vhost_user.c
@@ -473,8 +473,8 @@ vhost_user_set_vring_num(struct virtio_net **pdev,
 }
 
 /*
- * Reallocate virtio_dev and vhost_virtqueue data structure to make them on the
- * same numa node as the memory of vring descriptor.
+ * Reallocate virtio_dev, vhost_virtqueue and related data structures to
+ * make them on the same numa node as the memory of vring descriptor.
  */
 #ifdef RTE_LIBRTE_VHOST_NUMA
 static struct virtio_net*
@@ -557,6 +557,9 @@ numa_realloc(struct virtio_net *dev, int index)
 		goto out;
 	}
 	if (oldnode != newnode) {
+		struct rte_vhost_memory *old_mem;
+		ssize_t mem_size;
+
 		VHOST_LOG_CONFIG(INFO,
 			"reallocate dev from %d to %d node\n",
 			oldnode, newnode);
@@ -568,6 +571,18 @@ numa_realloc(struct virtio_net *dev, int index)
 
 		memcpy(dev, old_dev, sizeof(*dev));
 		rte_free(old_dev);
+
+		mem_size = sizeof(struct rte_vhost_memory) +
+			sizeof(struct rte_vhost_mem_region) * dev->mem->nregions;
+		old_mem = dev->mem;
+		dev->mem = rte_malloc_socket(NULL, mem_size, 0, newnode);
+		if (!dev->mem) {
+			dev->mem = old_mem;
+			goto out;
+		}
+
+		memcpy(dev->mem, old_mem, mem_size);
+		rte_free(old_mem);
 	}
 
 out:
-- 
2.31.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [dpdk-stable] [PATCH v7 2/7] vhost: fix missing guest pages table NUMA realloc
       [not found] <20210629161133.79472-1-maxime.coquelin@redhat.com>
  2021-06-29 16:11 ` [dpdk-stable] [PATCH v7 1/7] vhost: fix missing memory table NUMA realloc Maxime Coquelin
@ 2021-06-29 16:11 ` Maxime Coquelin
  2021-06-29 16:11 ` [dpdk-stable] [PATCH v7 4/7] vhost: fix NUMA reallocation with multiqueue Maxime Coquelin
  2 siblings, 0 replies; 6+ messages in thread
From: Maxime Coquelin @ 2021-06-29 16:11 UTC (permalink / raw)
  To: dev, chenbo.xia, david.marchand; +Cc: Maxime Coquelin, stable

When the guest allocates virtqueues on a different NUMA node
than the one the Vhost metadata are allocated, both the Vhost
device struct and the virtqueues struct are reallocated.

However, reallocating the guest pages table was missing, which
likely causes at least one cross-NUMA accesses for every burst
of packets.

This patch reallocates this table on the same NUMA node as the
other metadata.

Fixes: e246896178e6 ("vhost: get guest/host physical address mappings")
Cc: stable@dpdk.org

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
---
 lib/vhost/vhost_user.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
index b5a84f3dcd..5fb055ea2e 100644
--- a/lib/vhost/vhost_user.c
+++ b/lib/vhost/vhost_user.c
@@ -558,7 +558,8 @@ numa_realloc(struct virtio_net *dev, int index)
 	}
 	if (oldnode != newnode) {
 		struct rte_vhost_memory *old_mem;
-		ssize_t mem_size;
+		struct guest_page *old_gp;
+		ssize_t mem_size, gp_size;
 
 		VHOST_LOG_CONFIG(INFO,
 			"reallocate dev from %d to %d node\n",
@@ -583,6 +584,17 @@ numa_realloc(struct virtio_net *dev, int index)
 
 		memcpy(dev->mem, old_mem, mem_size);
 		rte_free(old_mem);
+
+		gp_size = dev->max_guest_pages * sizeof(*dev->guest_pages);
+		old_gp = dev->guest_pages;
+		dev->guest_pages = rte_malloc_socket(NULL, gp_size, RTE_CACHE_LINE_SIZE, newnode);
+		if (!dev->guest_pages) {
+			dev->guest_pages = old_gp;
+			goto out;
+		}
+
+		memcpy(dev->guest_pages, old_gp, gp_size);
+		rte_free(old_gp);
 	}
 
 out:
-- 
2.31.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [dpdk-stable] [PATCH v7 4/7] vhost: fix NUMA reallocation with multiqueue
       [not found] <20210629161133.79472-1-maxime.coquelin@redhat.com>
  2021-06-29 16:11 ` [dpdk-stable] [PATCH v7 1/7] vhost: fix missing memory table NUMA realloc Maxime Coquelin
  2021-06-29 16:11 ` [dpdk-stable] [PATCH v7 2/7] vhost: fix missing guest pages " Maxime Coquelin
@ 2021-06-29 16:11 ` Maxime Coquelin
  2021-06-30  5:20   ` Xia, Chenbo
  2021-06-30  7:24   ` David Marchand
  2 siblings, 2 replies; 6+ messages in thread
From: Maxime Coquelin @ 2021-06-29 16:11 UTC (permalink / raw)
  To: dev, chenbo.xia, david.marchand; +Cc: Maxime Coquelin, stable

Since the Vhost-user device initialization has been reworked,
enabling the application to start using the device as soon as
the first queue pair is ready, NUMA reallocation no more
happened on queue pairs other than the first one since
numa_realloc() was returning early if the device was running.

This patch fixes this issue by only preventing the device
metadata to be allocated if the device is running. For the
virtqueues, a vring state change notification is sent to
notify the application of its disablement. Since the callback
is supposed to be blocking, it is safe to reallocate it
afterwards.

Fixes: d0fcc38f5fa4 ("vhost: improve device readiness notifications")
Cc: stable@dpdk.org

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/vhost/vhost_user.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
index 82adf80fe5..51b96a0716 100644
--- a/lib/vhost/vhost_user.c
+++ b/lib/vhost/vhost_user.c
@@ -488,12 +488,16 @@ numa_realloc(struct virtio_net *dev, int index)
 	struct batch_copy_elem *new_batch_copy_elems;
 	int ret;
 
-	if (dev->flags & VIRTIO_DEV_RUNNING)
-		return dev;
-
 	old_dev = dev;
 	vq = old_vq = dev->virtqueue[index];
 
+	/*
+	 * If VQ is ready, it is too late to reallocate, it certainly already
+	 * happened anyway on VHOST_USER_SET_VRING_ADRR.
+	 */
+	if (vq->ready)
+		return dev;
+
 	ret = get_mempolicy(&newnode, NULL, 0, old_vq->desc,
 			    MPOL_F_NODE | MPOL_F_ADDR);
 
@@ -558,6 +562,9 @@ numa_realloc(struct virtio_net *dev, int index)
 		rte_free(old_vq);
 	}
 
+	if (dev->flags & VIRTIO_DEV_RUNNING)
+		goto out;
+
 	/* check if we need to reallocate dev */
 	ret = get_mempolicy(&oldnode, NULL, 0, old_dev,
 			    MPOL_F_NODE | MPOL_F_ADDR);
-- 
2.31.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-stable] [PATCH v7 4/7] vhost: fix NUMA reallocation with multiqueue
  2021-06-29 16:11 ` [dpdk-stable] [PATCH v7 4/7] vhost: fix NUMA reallocation with multiqueue Maxime Coquelin
@ 2021-06-30  5:20   ` Xia, Chenbo
  2021-06-30  7:24   ` David Marchand
  1 sibling, 0 replies; 6+ messages in thread
From: Xia, Chenbo @ 2021-06-30  5:20 UTC (permalink / raw)
  To: Maxime Coquelin, dev, david.marchand; +Cc: stable

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Wednesday, June 30, 2021 12:12 AM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>;
> david.marchand@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>; stable@dpdk.org
> Subject: [PATCH v7 4/7] vhost: fix NUMA reallocation with multiqueue
> 
> Since the Vhost-user device initialization has been reworked,
> enabling the application to start using the device as soon as
> the first queue pair is ready, NUMA reallocation no more
> happened on queue pairs other than the first one since
> numa_realloc() was returning early if the device was running.
> 
> This patch fixes this issue by only preventing the device
> metadata to be allocated if the device is running. For the
> virtqueues, a vring state change notification is sent to
> notify the application of its disablement. Since the callback
> is supposed to be blocking, it is safe to reallocate it
> afterwards.
> 
> Fixes: d0fcc38f5fa4 ("vhost: improve device readiness notifications")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  lib/vhost/vhost_user.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
> index 82adf80fe5..51b96a0716 100644
> --- a/lib/vhost/vhost_user.c
> +++ b/lib/vhost/vhost_user.c
> @@ -488,12 +488,16 @@ numa_realloc(struct virtio_net *dev, int index)
>  	struct batch_copy_elem *new_batch_copy_elems;
>  	int ret;
> 
> -	if (dev->flags & VIRTIO_DEV_RUNNING)
> -		return dev;
> -
>  	old_dev = dev;
>  	vq = old_vq = dev->virtqueue[index];
> 
> +	/*
> +	 * If VQ is ready, it is too late to reallocate, it certainly already
> +	 * happened anyway on VHOST_USER_SET_VRING_ADRR.
> +	 */
> +	if (vq->ready)
> +		return dev;
> +
>  	ret = get_mempolicy(&newnode, NULL, 0, old_vq->desc,
>  			    MPOL_F_NODE | MPOL_F_ADDR);
> 
> @@ -558,6 +562,9 @@ numa_realloc(struct virtio_net *dev, int index)
>  		rte_free(old_vq);
>  	}
> 
> +	if (dev->flags & VIRTIO_DEV_RUNNING)
> +		goto out;
> +
>  	/* check if we need to reallocate dev */
>  	ret = get_mempolicy(&oldnode, NULL, 0, old_dev,
>  			    MPOL_F_NODE | MPOL_F_ADDR);
> --
> 2.31.1

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-stable] [PATCH v7 4/7] vhost: fix NUMA reallocation with multiqueue
  2021-06-29 16:11 ` [dpdk-stable] [PATCH v7 4/7] vhost: fix NUMA reallocation with multiqueue Maxime Coquelin
  2021-06-30  5:20   ` Xia, Chenbo
@ 2021-06-30  7:24   ` David Marchand
  2021-06-30  7:47     ` Maxime Coquelin
  1 sibling, 1 reply; 6+ messages in thread
From: David Marchand @ 2021-06-30  7:24 UTC (permalink / raw)
  To: Maxime Coquelin; +Cc: dev, Xia, Chenbo, dpdk stable

On Tue, Jun 29, 2021 at 6:11 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
> Since the Vhost-user device initialization has been reworked,
> enabling the application to start using the device as soon as
> the first queue pair is ready, NUMA reallocation no more
> happened on queue pairs other than the first one since
> numa_realloc() was returning early if the device was running.
>
> This patch fixes this issue by only preventing the device
> metadata to be allocated if the device is running. For the

Hum, I understand the meaning, but I think we could make it easier to read:

This patch fixes this issue by reallocating the device metadata only
if the device is not running.

WDYT?

> virtqueues, a vring state change notification is sent to
> notify the application of its disablement. Since the callback
> is supposed to be blocking, it is safe to reallocate it
> afterwards.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-stable] [PATCH v7 4/7] vhost: fix NUMA reallocation with multiqueue
  2021-06-30  7:24   ` David Marchand
@ 2021-06-30  7:47     ` Maxime Coquelin
  0 siblings, 0 replies; 6+ messages in thread
From: Maxime Coquelin @ 2021-06-30  7:47 UTC (permalink / raw)
  To: David Marchand; +Cc: dev, Xia, Chenbo, dpdk stable



On 6/30/21 9:24 AM, David Marchand wrote:
> On Tue, Jun 29, 2021 at 6:11 PM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
>>
>> Since the Vhost-user device initialization has been reworked,
>> enabling the application to start using the device as soon as
>> the first queue pair is ready, NUMA reallocation no more
>> happened on queue pairs other than the first one since
>> numa_realloc() was returning early if the device was running.
>>
>> This patch fixes this issue by only preventing the device
>> metadata to be allocated if the device is running. For the
> 
> Hum, I understand the meaning, but I think we could make it easier to read:
> 
> This patch fixes this issue by reallocating the device metadata only
> if the device is not running.
> 
> WDYT?

It sounds better, I can be changed while applying if only issue.

>> virtqueues, a vring state change notification is sent to
>> notify the application of its disablement. Since the callback
>> is supposed to be blocking, it is safe to reallocate it
>> afterwards.
> 
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-06-30  7:47 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20210629161133.79472-1-maxime.coquelin@redhat.com>
2021-06-29 16:11 ` [dpdk-stable] [PATCH v7 1/7] vhost: fix missing memory table NUMA realloc Maxime Coquelin
2021-06-29 16:11 ` [dpdk-stable] [PATCH v7 2/7] vhost: fix missing guest pages " Maxime Coquelin
2021-06-29 16:11 ` [dpdk-stable] [PATCH v7 4/7] vhost: fix NUMA reallocation with multiqueue Maxime Coquelin
2021-06-30  5:20   ` Xia, Chenbo
2021-06-30  7:24   ` David Marchand
2021-06-30  7:47     ` Maxime Coquelin

patches for DPDK stable branches

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ https://inbox.dpdk.org/stable \
		stable@dpdk.org
	public-inbox-index stable

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.stable


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git