On Mon, 15 May 2023 13:18:44 +0200
David Marchand <david.march...@redhat.com> wrote:
> Covscan reported:
>
> 2. dpdk-21.11/lib/vhost/socket.c:852: lock_acquire: Calling function
> "pthread_mutex_lock" acquires lock "vhost_user.mutex".
> 23. dpdk-21.11/lib/vhost/socket.c:955: sleep: Call to
> "vhost_user_reconnect_init" might sleep while holding lock
> "vhost_user.mutex".
> # 953| vsocket->reconnect =
> !(flags & RTE_VHOST_USER_NO_RECONNECT);
> # 954| if (vsocket->reconnect && reconn_tid == 0) {
> # 955|-> if (vhost_user_reconnect_init() != 0)
> # 956| goto out_mutex;
> # 957| }
>
> The reason for this warning is that vhost_user_reconnect_init() creates a
> ctrl thread and calls nanosleep waiting for this thread to be ready,
> while vhost_user.mutex is taken.
>
> Move the call to vhost_user_reconnect_init() out of this mutex.
>
> While at it, a pthread_t value should be considered opaque.
> Instead of relying reconn_tid == 0, use an internal flag in
> vhost_user_reconnect_init().
>
> Coverity issue: 373686
> Bugzilla ID: 981
> Fixes: e623e0c6d8a5 ("vhost: add reconnect ability")
> Cc: sta...@dpdk.org
>
> Signed-off-by: David Marchand <david.march...@redhat.com>
> ---
> Changes since v1:
> - moved reconn_tid in vhost_user_reconnect_init as this
> variable is not used anywhere else,
>
> ---
> lib/vhost/socket.c | 19 +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/lib/vhost/socket.c b/lib/vhost/socket.c
> index 669c322e12..00a912c59e 100644
> --- a/lib/vhost/socket.c
> +++ b/lib/vhost/socket.c
> @@ -427,7 +427,6 @@ struct vhost_user_reconnect_list {
> };
>
> static struct vhost_user_reconnect_list reconn_list;
> -static pthread_t reconn_tid;
>
> static int
> vhost_user_connect_nonblock(char *path, int fd, struct sockaddr *un, size_t
> sz)
> @@ -498,8 +497,13 @@ vhost_user_client_reconnect(void *arg __rte_unused)
> static int
> vhost_user_reconnect_init(void)
> {
> + static bool reconn_init_done;
> + static pthread_t reconn_tid;
This would not be thread safe.
Perhaps use a pthread condition variable instead of spinning here.
