Re: [dpdk-dev] [PATCH] vhost: fix race condition in fdset_add

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

Hi Matthias,

On 12/6/18 5:00 PM, Matthias Gatto wrote:
> fdset_add can call fdset_shrink_nolock which call fdset_move
> concurrently to poll that is call in fdset_event_dispatch.
> 
> This patch add a mutex to protect poll from been call at the same time
> fdset_add call fdset_shrink_nolock.
> 
> Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com>
> ---
>   lib/librte_vhost/fd_man.c | 4 ++++
>   lib/librte_vhost/fd_man.h | 1 +
>   lib/librte_vhost/socket.c | 1 +
>   3 files changed, 6 insertions(+)
> 
> diff --git a/lib/librte_vhost/fd_man.c b/lib/librte_vhost/fd_man.c
> index 38347ab..55d4856 100644
> --- a/lib/librte_vhost/fd_man.c
> +++ b/lib/librte_vhost/fd_man.c
> @@ -129,7 +129,9 @@
>   	pthread_mutex_lock(&pfdset->fd_mutex);
>   	i = pfdset->num < MAX_FDS ? pfdset->num++ : -1;
>   	if (i == -1) {
> +		pthread_mutex_lock(&pfdset->fd_pooling_mutex);
>   		fdset_shrink_nolock(pfdset);
> +		pthread_mutex_unlock(&pfdset->fd_pooling_mutex);
>   		i = pfdset->num < MAX_FDS ? pfdset->num++ : -1;
>   		if (i == -1) {
>   			pthread_mutex_unlock(&pfdset->fd_mutex);
> @@ -246,7 +248,9 @@
>   		numfds = pfdset->num;
>   		pthread_mutex_unlock(&pfdset->fd_mutex);
>   
> +		pthread_mutex_lock(&pfdset->fd_pooling_mutex);
>   		val = poll(pfdset->rwfds, numfds, 1000 /* millisecs */);
> +		pthread_mutex_unlock(&pfdset->fd_pooling_mutex);

Any reason we cannot use the existing fd_mutex?

>   		if (val < 0)
>   			continue;
>   
> diff --git a/lib/librte_vhost/fd_man.h b/lib/librte_vhost/fd_man.h
> index 3331bcd..3ab5cfd 100644
> --- a/lib/librte_vhost/fd_man.h
> +++ b/lib/librte_vhost/fd_man.h
> @@ -24,6 +24,7 @@ struct fdset {
>   	struct pollfd rwfds[MAX_FDS];
>   	struct fdentry fd[MAX_FDS];
>   	pthread_mutex_t fd_mutex;
> +	pthread_mutex_t fd_pooling_mutex;
>   	int num;	/* current fd number of this fdset */
>   
>   	union pipefds {
> diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
> index d630317..cc4e748 100644
> --- a/lib/librte_vhost/socket.c
> +++ b/lib/librte_vhost/socket.c
> @@ -88,6 +88,7 @@ struct vhost_user {
>   	.fdset = {
>   		.fd = { [0 ... MAX_FDS - 1] = {-1, NULL, NULL, NULL, 0} },
>   		.fd_mutex = PTHREAD_MUTEX_INITIALIZER,
> +		.fd_pooling_mutex = PTHREAD_MUTEX_INITIALIZER,
>   		.num = 0
>   	},
>   	.vsocket_cnt = 0,
>