From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by dpdk.org (Postfix) with ESMTP id 9849823A for ; Wed, 28 Mar 2018 21:46:49 +0200 (CEST) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1F5B34068053; Wed, 28 Mar 2018 19:46:49 +0000 (UTC) Received: from [10.36.112.44] (ovpn-112-44.ams2.redhat.com [10.36.112.44]) by smtp.corp.redhat.com (Postfix) with ESMTPS id CD0BC2026DFD; Wed, 28 Mar 2018 19:46:47 +0000 (UTC) To: stable@dpdk.org, bluca@debian.org, jianfeng.tan@intel.com References: <20180328194515.7696-1-maxime.coquelin@redhat.com> From: Maxime Coquelin Message-ID: Date: Wed, 28 Mar 2018 21:46:46 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <20180328194515.7696-1-maxime.coquelin@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Wed, 28 Mar 2018 19:46:49 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Wed, 28 Mar 2018 19:46:49 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'maxime.coquelin@redhat.com' RCPT:'' Subject: Re: [dpdk-stable] [PATCH v16.11 LTS] vhost-user: fix deadlock in case of NUMA realloc X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 28 Mar 2018 19:46:49 -0000 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 > Signed-off-by: Maxime Coquelin > --- > 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); >