Re: [dpdk-stable] [PATCH v16.11 LTS] vhost-user: fix deadlock in case of NUMA realloc

[Maxime Coquelin <maxime.coquelin@redhat.com>]

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);
>