From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by dpdk.org (Postfix) with ESMTP id EE0A51B94E for ; Fri, 14 Dec 2018 10:33:03 +0100 (CET) Received: by mail-wm1-f65.google.com with SMTP id f81so5069397wmd.4 for ; Fri, 14 Dec 2018 01:33:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=outscale-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Ak6JauJ/zV7kSxwHrjLBDYLA3+lnoXVgHGdn932u+Qw=; b=Pd+PWb68eeWGbhhLwn5m+XqsHfVpBTBRNxJmwE3mm8sFY+peRA2ubQKANVyxiSneXy DVZsHbxYSYRbzbK93fJWLRL2QIUyPblklv6y9IT4jQkk4C0Ls8ZyjXNhxOOFrY9l6+dy w8Mb7pXRDzWOGQBaZcDL//8HkPr4h8igTLLMPNU6AAgAmIb1YbCw6xj1fscMHsR6VK4p OykJ7S9pl1dBkHcz+yPtNtlkZGbPgZ2C2MwaRDyuWd+66rl2Vh12EbSuAeKwfJ9PpQBy WtQeRxxNyH7TyWqK+RW2pa4BzHHrBto5GEZRx3Yke2cAvSMwlyRswiMqSDYIyBDQOlu2 NUxA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Ak6JauJ/zV7kSxwHrjLBDYLA3+lnoXVgHGdn932u+Qw=; b=mxpv5ngzqJ9s/59rwCKaG02EUKDDES3rQAN4D6S8DQj8ET/eVumUIqNjUnZQqYn+YE Ir7mu/nU2+rS8nUmJ5YLvCA5+b+W10Y9pEvH/C9179bre1VE5mL0eLpfSIVZyLaqlCj4 fdzWjm5aglAD+zNTQ8x7Yr7sH3bw6VOvHOUXQBLC4pBHlqxicTC39mwzzdo/1gtdLUez eQfoNwNN9IDRgTi0ajltnoaMWNCfunF2cK9pTzJsPEmCezcZXYKXMi5ZIU99R4hsNQ+5 FytEEi82uiiZ/ETZtQUKKXlIKavJaI4eoGKP9OhMi4trZJkTyAOV/NqmcKytTywWr3h8 GWcg== X-Gm-Message-State: AA+aEWax2aU/6FIMa/obdB51VFIwJk70gWtOem3E8/4Wi0saGxShbGsq BY4NVNpbEcbzzEOTKG1nOHHPYzFp9CwnMtYyZH1hGg== X-Google-Smtp-Source: AFSGD/VIFs5fy7rOIXHL/9/5jX9SOXHpgfkdmVgSo1uuXsKCaHDvEWsddcZ2vkY8MIs60/e9/tsXq1/Z+og+Oc5BjWs= X-Received: by 2002:a1c:8104:: with SMTP id c4mr2289011wmd.133.1544779983557; Fri, 14 Dec 2018 01:33:03 -0800 (PST) MIME-Version: 1.0 References: <1544112007-23177-1-git-send-email-matthias.gatto@outscale.com> In-Reply-To: From: Matthias Gatto Date: Fri, 14 Dec 2018 10:32:51 +0100 Message-ID: To: maxime.coquelin@redhat.com Cc: dev@dpdk.org, tiwei.bie@intel.com, zhihong.wang@intel.com Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH] vhost: fix race condition in fdset_add X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 14 Dec 2018 09:33:04 -0000 On Tue, Dec 11, 2018 at 7:11 PM Maxime Coquelin wrote: > > 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 > > --- > > 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? yes, using the existing fd_mutex would block fdset_add during the polling in fdset_event_dispatch. here fd_pooling_mutex block only fdset_shrink_nolock inside fdset_add which happen only in very rare occasions. > > 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, > >