patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH v16.11 LTS] vhost-user: fix deadlock in case of NUMA realloc
@ 2018-03-28 19:46 Maxime Coquelin
  0 siblings, 0 replies; 3+ messages in thread
From: Maxime Coquelin @ 2018-03-28 19:46 UTC (permalink / raw)
  To: stable, bluca, jianfeng.tan, ktraynor; +Cc: Maxime Coquelin

Virtqueue's access lock was recently introduced to protect
the device against async changes.

One problem with the v16.11 backport is that in case of NUMA
reallocation, the device gets stuck because the old access_lock
gets unlocked instead of its reallocated copy. On the next
vhost-user message received, the thread keeps spinning on the
lock, as it will never be unlocked.

Fixes: ce3b23dc9296 ("vhost: protect active rings from async ring changes")

Cc: stable@dpdk.org

Tested-by: Kevin Traynor <ktraynor@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost_user.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 80348dbf6..eb75e01cb 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -249,7 +249,7 @@ numa_realloc(struct virtio_net *dev, int index)
 			"Unable to get vq numa information.\n");
 		return dev;
 	}
-	if (oldnode != newnode) {
+	if (oldnode == newnode) {
 		RTE_LOG(INFO, VHOST_CONFIG,
 			"reallocate vq from %d to %d node\n", oldnode, newnode);
 		vq = rte_malloc_socket(NULL, sizeof(*vq) * VIRTIO_QNUM, 0,
@@ -269,7 +269,7 @@ numa_realloc(struct virtio_net *dev, int index)
 			"Unable to get dev numa information.\n");
 		goto out;
 	}
-	if (oldnode != newnode) {
+	if (oldnode == newnode) {
 		RTE_LOG(INFO, VHOST_CONFIG,
 			"reallocate dev from %d to %d node\n",
 			oldnode, newnode);
@@ -327,9 +327,11 @@ qva_to_vva(struct virtio_net *dev, uint64_t qva)
  * This function then converts these to our address space.
  */
 static int
-vhost_user_set_vring_addr(struct virtio_net *dev, struct vhost_vring_addr *addr)
+vhost_user_set_vring_addr(struct virtio_net **pdev,
+						  struct vhost_vring_addr *addr)
 {
 	struct vhost_virtqueue *vq;
+	struct virtio_net *dev = *pdev;
 
 	if (dev->mem == NULL)
 		return -1;
@@ -348,6 +350,8 @@ vhost_user_set_vring_addr(struct virtio_net *dev, struct vhost_vring_addr *addr)
 	}
 
 	dev = numa_realloc(dev, addr->index);
+	*pdev = dev;
+
 	vq = dev->virtqueue[addr->index];
 
 	vq->avail = (struct vring_avail *)(uintptr_t)qva_to_vva(dev,
@@ -1092,7 +1096,7 @@ vhost_user_msg_handler(int vid, int fd)
 		vhost_user_set_vring_num(dev, &msg.payload.state);
 		break;
 	case VHOST_USER_SET_VRING_ADDR:
-		vhost_user_set_vring_addr(dev, &msg.payload.addr);
+		vhost_user_set_vring_addr(&dev, &msg.payload.addr);
 		break;
 	case VHOST_USER_SET_VRING_BASE:
 		vhost_user_set_vring_base(dev, &msg.payload.state);
-- 
2.14.3

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

* Re: [dpdk-stable] [PATCH v16.11 LTS] vhost-user: fix deadlock in case of NUMA realloc
  2018-03-28 19:45 Maxime Coquelin
@ 2018-03-28 19:46 ` Maxime Coquelin
  0 siblings, 0 replies; 3+ messages in thread
From: Maxime Coquelin @ 2018-03-28 19:46 UTC (permalink / raw)
  To: stable, bluca, jianfeng.tan

Sorry, resent with Kevins's valid address

On 03/28/2018 09:45 PM, Maxime Coquelin wrote:
> Virtqueue's access lock was recently introduced to protect
> the device against async changes.
> 
> One problem with the v16.11 backport is that in case of NUMA
> reallocation, the device gets stuck because the old access_lock
> gets unlocked instead of its reallocated copy. On the next
> vhost-user message received, the thread keeps spinning on the
> lock, as it will never be unlocked.
> 
> Fixes: ce3b23dc9296 ("vhost: protect active rings from async ring changes")
> 
> Cc: stable@dpdk.org
> 
> Tested-by: Kevin Traynor <ktraynor@redhat.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>   lib/librte_vhost/vhost_user.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> index 80348dbf6..eb75e01cb 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -249,7 +249,7 @@ numa_realloc(struct virtio_net *dev, int index)
>   			"Unable to get vq numa information.\n");
>   		return dev;
>   	}
> -	if (oldnode != newnode) {
> +	if (oldnode == newnode) {
>   		RTE_LOG(INFO, VHOST_CONFIG,
>   			"reallocate vq from %d to %d node\n", oldnode, newnode);
>   		vq = rte_malloc_socket(NULL, sizeof(*vq) * VIRTIO_QNUM, 0,
> @@ -269,7 +269,7 @@ numa_realloc(struct virtio_net *dev, int index)
>   			"Unable to get dev numa information.\n");
>   		goto out;
>   	}
> -	if (oldnode != newnode) {
> +	if (oldnode == newnode) {
>   		RTE_LOG(INFO, VHOST_CONFIG,
>   			"reallocate dev from %d to %d node\n",
>   			oldnode, newnode);
> @@ -327,9 +327,11 @@ qva_to_vva(struct virtio_net *dev, uint64_t qva)
>    * This function then converts these to our address space.
>    */
>   static int
> -vhost_user_set_vring_addr(struct virtio_net *dev, struct vhost_vring_addr *addr)
> +vhost_user_set_vring_addr(struct virtio_net **pdev,
> +						  struct vhost_vring_addr *addr)
>   {
>   	struct vhost_virtqueue *vq;
> +	struct virtio_net *dev = *pdev;
>   
>   	if (dev->mem == NULL)
>   		return -1;
> @@ -348,6 +350,8 @@ vhost_user_set_vring_addr(struct virtio_net *dev, struct vhost_vring_addr *addr)
>   	}
>   
>   	dev = numa_realloc(dev, addr->index);
> +	*pdev = dev;
> +
>   	vq = dev->virtqueue[addr->index];
>   
>   	vq->avail = (struct vring_avail *)(uintptr_t)qva_to_vva(dev,
> @@ -1092,7 +1096,7 @@ vhost_user_msg_handler(int vid, int fd)
>   		vhost_user_set_vring_num(dev, &msg.payload.state);
>   		break;
>   	case VHOST_USER_SET_VRING_ADDR:
> -		vhost_user_set_vring_addr(dev, &msg.payload.addr);
> +		vhost_user_set_vring_addr(&dev, &msg.payload.addr);
>   		break;
>   	case VHOST_USER_SET_VRING_BASE:
>   		vhost_user_set_vring_base(dev, &msg.payload.state);
> 

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

* [dpdk-stable] [PATCH v16.11 LTS] vhost-user: fix deadlock in case of NUMA realloc
@ 2018-03-28 19:45 Maxime Coquelin
  2018-03-28 19:46 ` Maxime Coquelin
  0 siblings, 1 reply; 3+ messages in thread
From: Maxime Coquelin @ 2018-03-28 19:45 UTC (permalink / raw)
  To: stable, bluca, jianfeng.tan, ktraynor; +Cc: Maxime Coquelin

Virtqueue's access lock was recently introduced to protect
the device against async changes.

One problem with the v16.11 backport is that in case of NUMA
reallocation, the device gets stuck because the old access_lock
gets unlocked instead of its reallocated copy. On the next
vhost-user message received, the thread keeps spinning on the
lock, as it will never be unlocked.

Fixes: ce3b23dc9296 ("vhost: protect active rings from async ring changes")

Cc: stable@dpdk.org

Tested-by: Kevin Traynor <ktraynor@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost_user.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 80348dbf6..eb75e01cb 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -249,7 +249,7 @@ numa_realloc(struct virtio_net *dev, int index)
 			"Unable to get vq numa information.\n");
 		return dev;
 	}
-	if (oldnode != newnode) {
+	if (oldnode == newnode) {
 		RTE_LOG(INFO, VHOST_CONFIG,
 			"reallocate vq from %d to %d node\n", oldnode, newnode);
 		vq = rte_malloc_socket(NULL, sizeof(*vq) * VIRTIO_QNUM, 0,
@@ -269,7 +269,7 @@ numa_realloc(struct virtio_net *dev, int index)
 			"Unable to get dev numa information.\n");
 		goto out;
 	}
-	if (oldnode != newnode) {
+	if (oldnode == newnode) {
 		RTE_LOG(INFO, VHOST_CONFIG,
 			"reallocate dev from %d to %d node\n",
 			oldnode, newnode);
@@ -327,9 +327,11 @@ qva_to_vva(struct virtio_net *dev, uint64_t qva)
  * This function then converts these to our address space.
  */
 static int
-vhost_user_set_vring_addr(struct virtio_net *dev, struct vhost_vring_addr *addr)
+vhost_user_set_vring_addr(struct virtio_net **pdev,
+						  struct vhost_vring_addr *addr)
 {
 	struct vhost_virtqueue *vq;
+	struct virtio_net *dev = *pdev;
 
 	if (dev->mem == NULL)
 		return -1;
@@ -348,6 +350,8 @@ vhost_user_set_vring_addr(struct virtio_net *dev, struct vhost_vring_addr *addr)
 	}
 
 	dev = numa_realloc(dev, addr->index);
+	*pdev = dev;
+
 	vq = dev->virtqueue[addr->index];
 
 	vq->avail = (struct vring_avail *)(uintptr_t)qva_to_vva(dev,
@@ -1092,7 +1096,7 @@ vhost_user_msg_handler(int vid, int fd)
 		vhost_user_set_vring_num(dev, &msg.payload.state);
 		break;
 	case VHOST_USER_SET_VRING_ADDR:
-		vhost_user_set_vring_addr(dev, &msg.payload.addr);
+		vhost_user_set_vring_addr(&dev, &msg.payload.addr);
 		break;
 	case VHOST_USER_SET_VRING_BASE:
 		vhost_user_set_vring_base(dev, &msg.payload.state);
-- 
2.14.3

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

end of thread, other threads:[~2018-03-28 19:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-28 19:46 [dpdk-stable] [PATCH v16.11 LTS] vhost-user: fix deadlock in case of NUMA realloc Maxime Coquelin
  -- strict thread matches above, loose matches on Subject: below --
2018-03-28 19:45 Maxime Coquelin
2018-03-28 19:46 ` Maxime Coquelin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).