On 5 September 2018 at 05:28, Tiwei Bie <tiwei....@intel.com> 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;
> }
> --
> 2.18.0
>
Tested-by: Seán Harte <sea...@gmail.com>
Reviewed-by: Seán Harte <sea...@gmail.com>
