* [dpdk-dev] [PATCH] vhost: fix race condition in fdset_add
@ 2018-12-06 16:00 Matthias Gatto
2018-12-11 18:11 ` Maxime Coquelin
2018-12-18 14:01 ` Maxime Coquelin
0 siblings, 2 replies; 8+ messages in thread
From: Matthias Gatto @ 2018-12-06 16:00 UTC (permalink / raw)
To: dev; +Cc: Matthias Gatto, maxime.coquelin, tiwei.bie, zhihong.wang
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);
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,
--
1.8.3.1
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [dpdk-dev] [PATCH] vhost: fix race condition in fdset_add
2018-12-06 16:00 [dpdk-dev] [PATCH] vhost: fix race condition in fdset_add Matthias Gatto
@ 2018-12-11 18:11 ` Maxime Coquelin
2018-12-14 9:32 ` Matthias Gatto
2018-12-18 14:01 ` Maxime Coquelin
1 sibling, 1 reply; 8+ messages in thread
From: Maxime Coquelin @ 2018-12-11 18:11 UTC (permalink / raw)
To: Matthias Gatto, dev; +Cc: tiwei.bie, zhihong.wang
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,
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [dpdk-dev] [PATCH] vhost: fix race condition in fdset_add
2018-12-11 18:11 ` Maxime Coquelin
@ 2018-12-14 9:32 ` Matthias Gatto
2018-12-14 9:51 ` Maxime Coquelin
0 siblings, 1 reply; 8+ messages in thread
From: Matthias Gatto @ 2018-12-14 9:32 UTC (permalink / raw)
To: maxime.coquelin; +Cc: dev, tiwei.bie, zhihong.wang
On Tue, Dec 11, 2018 at 7:11 PM Maxime Coquelin
<maxime.coquelin@redhat.com> 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 <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?
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,
> >
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [dpdk-dev] [PATCH] vhost: fix race condition in fdset_add
2018-12-14 9:32 ` Matthias Gatto
@ 2018-12-14 9:51 ` Maxime Coquelin
2018-12-14 9:53 ` Maxime Coquelin
0 siblings, 1 reply; 8+ messages in thread
From: Maxime Coquelin @ 2018-12-14 9:51 UTC (permalink / raw)
To: Matthias Gatto; +Cc: dev, tiwei.bie, zhihong.wang
On 12/14/18 10:32 AM, Matthias Gatto wrote:
> On Tue, Dec 11, 2018 at 7:11 PM Maxime Coquelin
> <maxime.coquelin@redhat.com> 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 <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?
>
> 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.
Thanks for the clarification:
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Maxime
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [dpdk-dev] [PATCH] vhost: fix race condition in fdset_add
2018-12-14 9:51 ` Maxime Coquelin
@ 2018-12-14 9:53 ` Maxime Coquelin
2018-12-14 10:07 ` Matthias Gatto
0 siblings, 1 reply; 8+ messages in thread
From: Maxime Coquelin @ 2018-12-14 9:53 UTC (permalink / raw)
To: Matthias Gatto; +Cc: dev, tiwei.bie, zhihong.wang
On 12/14/18 10:51 AM, Maxime Coquelin wrote:
>
>
> On 12/14/18 10:32 AM, Matthias Gatto wrote:
>> On Tue, Dec 11, 2018 at 7:11 PM Maxime Coquelin
>> <maxime.coquelin@redhat.com> 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 <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?
>>
>> 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.
>
>
> Thanks for the clarification:
>
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
I guess we need to cc: stable, can you help with specifying which
commit it fixes?
Thanks in advance,
Maxime
> Maxime
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [dpdk-dev] [PATCH] vhost: fix race condition in fdset_add
2018-12-14 9:53 ` Maxime Coquelin
@ 2018-12-14 10:07 ` Matthias Gatto
2018-12-14 10:08 ` Maxime Coquelin
0 siblings, 1 reply; 8+ messages in thread
From: Matthias Gatto @ 2018-12-14 10:07 UTC (permalink / raw)
To: maxime.coquelin; +Cc: dev, tiwei.bie, zhihong.wang, stable
On Fri, Dec 14, 2018 at 10:53 AM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
>
>
> On 12/14/18 10:51 AM, Maxime Coquelin wrote:
> >
> >
> > On 12/14/18 10:32 AM, Matthias Gatto wrote:
> >> On Tue, Dec 11, 2018 at 7:11 PM Maxime Coquelin
> >> <maxime.coquelin@redhat.com> 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 <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?
> >>
> >> 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.
> >
> >
> > Thanks for the clarification:
> >
> > Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>
> I guess we need to cc: stable, can you help with specifying which
> commit it fixes?
>
> Thanks in advance,
> Maxime
>
this commit 1b815b89599cdd9b54e5aa70f5b97088225b2bcc
which was actually a commit I've made, sorry for that.
Thanks for the review,
Matthias
> > Maxime
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [dpdk-dev] [PATCH] vhost: fix race condition in fdset_add
2018-12-14 10:07 ` Matthias Gatto
@ 2018-12-14 10:08 ` Maxime Coquelin
0 siblings, 0 replies; 8+ messages in thread
From: Maxime Coquelin @ 2018-12-14 10:08 UTC (permalink / raw)
To: Matthias Gatto; +Cc: dev, tiwei.bie, zhihong.wang, stable
On 12/14/18 11:07 AM, Matthias Gatto wrote:
> On Fri, Dec 14, 2018 at 10:53 AM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
>>
>>
>>
>> On 12/14/18 10:51 AM, Maxime Coquelin wrote:
>>>
>>>
>>> On 12/14/18 10:32 AM, Matthias Gatto wrote:
>>>> On Tue, Dec 11, 2018 at 7:11 PM Maxime Coquelin
>>>> <maxime.coquelin@redhat.com> 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 <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?
>>>>
>>>> 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.
>>>
>>>
>>> Thanks for the clarification:
>>>
>>> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>
>> I guess we need to cc: stable, can you help with specifying which
>> commit it fixes?
>>
>> Thanks in advance,
>> Maxime
>>
>
> this commit 1b815b89599cdd9b54e5aa70f5b97088225b2bcc
> which was actually a commit I've made, sorry for that.
Don't be sorry, your contributions are welcome!
I'll fixup the commit message with adding Fixes line.
Thanks,
Maxime
> Thanks for the review,
>
> Matthias
>>> Maxime
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: fix race condition in fdset_add
2018-12-06 16:00 [dpdk-dev] [PATCH] vhost: fix race condition in fdset_add Matthias Gatto
2018-12-11 18:11 ` Maxime Coquelin
@ 2018-12-18 14:01 ` Maxime Coquelin
1 sibling, 0 replies; 8+ messages in thread
From: Maxime Coquelin @ 2018-12-18 14:01 UTC (permalink / raw)
To: Matthias Gatto, dev; +Cc: tiwei.bie, zhihong.wang
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(+)
Applied to dpdk-next-virtio
Thanks,
Maxime
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-12-18 14:01 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-06 16:00 [dpdk-dev] [PATCH] vhost: fix race condition in fdset_add Matthias Gatto
2018-12-11 18:11 ` Maxime Coquelin
2018-12-14 9:32 ` Matthias Gatto
2018-12-14 9:51 ` Maxime Coquelin
2018-12-14 9:53 ` Maxime Coquelin
2018-12-14 10:07 ` Matthias Gatto
2018-12-14 10:08 ` Maxime Coquelin
2018-12-18 14:01 ` Maxime Coquelin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).