Re: [dpdk-dev] [PATCH 1/3] net/virtio-user: fix deadlock in memory events callback

[Burakov, Anatoly]

On 05-Sep-18 5:28 AM, Tiwei Bie wrote:

Deadlock can occur when allocating memory if a vhost-kernel
based virtio-user device is in use. To fix the deadlock,
we will take memory hotplug lock explicitly in virtio-user
when necessary, and always call the _thread_unsafe memory
functions.

Bugzilla ID: 81
Fixes: 12ecb2f63b12 ("net/virtio-user: support memory hotplug")
Cc: sta...@dpdk.org

Reported-by: Seán Harte <sea...@gmail.com>
Signed-off-by: Tiwei Bie <tiwei....@intel.com>
---
  drivers/net/virtio/virtio_user/vhost_kernel.c |  6 +++++-
  .../net/virtio/virtio_user/virtio_user_dev.c  | 19 +++++++++++++++++++
  2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio/virtio_user/vhost_kernel.c 
b/drivers/net/virtio/virtio_user/vhost_kernel.c
index b2444096c..897fee0af 100644
--- a/drivers/net/virtio/virtio_user/vhost_kernel.c
+++ b/drivers/net/virtio/virtio_user/vhost_kernel.c
@@ -115,7 +115,11 @@ prepare_vhost_memory_kernel(void)
        wa.region_nr = 0;
        wa.vm = vm;

- if (rte_memseg_contig_walk(add_memory_region, &wa) < 0) {

+       /*
+        * The memory lock has already been taken by memory subsystem
+        * or virtio_user_start_device().
+        */
+       if (rte_memseg_contig_walk_thread_unsafe(add_memory_region, &wa) < 0) {
                free(vm);
                return NULL;
        }
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c 
b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 7df600b02..869e96f87 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -13,6 +13,8 @@
  #include <sys/types.h>
  #include <sys/stat.h>

+#include <rte_eal_memconfig.h>

+
  #include "vhost.h"
  #include "virtio_user_dev.h"
  #include "../virtio_ethdev.h"
@@ -109,9 +111,24 @@ is_vhost_user_by_type(const char *path)
  int
  virtio_user_start_device(struct virtio_user_dev *dev)
  {
+       struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
        uint64_t features;
        int ret;

+ /*

+        * XXX workaround!
+        *
+        * We need to make sure that the locks will be
+        * taken in the correct order to avoid deadlocks.
+        *
+        * Before releasing this lock, this thread should
+        * not trigger any memory hotplug events.
+        *
+        * This is a temporary workaround, and should be
+        * replaced when we get proper supports from the
+        * memory subsystem in the future.
+        */
+       rte_rwlock_read_lock(&mcfg->memory_hotplug_lock);
        pthread_mutex_lock(&dev->mutex);

if (is_vhost_user_by_type(dev->path) && dev->vhostfd < 0)

@@ -152,10 +169,12 @@ virtio_user_start_device(struct virtio_user_dev *dev)

dev->started = true;

        pthread_mutex_unlock(&dev->mutex);
+       rte_rwlock_read_unlock(&mcfg->memory_hotplug_lock);

return 0;

  error:
        pthread_mutex_unlock(&dev->mutex);
+       rte_rwlock_read_unlock(&mcfg->memory_hotplug_lock);
        /* TODO: free resource here or caller to check */
        return -1;
  }

For the memory parts,

Reviewed-by: Anatoly Burakov <anatoly.bura...@intel.com>

--
Thanks,
Anatoly