* [PATCH] futex: Fix a faulty comment. @ 2021-12-04 18:14 6812skiii 2021-12-06 12:12 ` André Almeida 0 siblings, 1 reply; 4+ messages in thread From: 6812skiii @ 2021-12-04 18:14 UTC (permalink / raw) To: tglx, mingo Cc: peterz, dvhart, dave, andrealmeid, linux-kernel, Jangwoong Kim From: Jangwoong Kim <6812skiii@gmail.com> We return 1, not the index of futex woken up. Signed-off-by: Jangwoong Kim <6812skiii@gmail.com> --- kernel/futex/waitwake.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/futex/waitwake.c b/kernel/futex/waitwake.c index 4ce0923f1ce3..d148e5d4956b 100644 --- a/kernel/futex/waitwake.c +++ b/kernel/futex/waitwake.c @@ -455,8 +455,8 @@ static int futex_wait_multiple_setup(struct futex_vector *vs, int count, int *wo /* * Even if something went wrong, if we find out that a futex - * was woken, we don't return error and return this index to - * userspace + * was woken, we don't return error and make userspace aware + * of this by returning 1. */ *woken = unqueue_multiple(vs, i); if (*woken >= 0) -- 2.25.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] futex: Fix a faulty comment. 2021-12-04 18:14 [PATCH] futex: Fix a faulty comment 6812skiii @ 2021-12-06 12:12 ` André Almeida 2021-12-06 17:44 ` Jangwoong Kim 0 siblings, 1 reply; 4+ messages in thread From: André Almeida @ 2021-12-06 12:12 UTC (permalink / raw) To: 6812skiii; +Cc: peterz, mingo, tglx, dvhart, dave, linux-kernel Hi Jangwoong, Thanks for your patch! However... Às 15:14 de 04/12/21, 6812skiii@gmail.com escreveu: > From: Jangwoong Kim <6812skiii@gmail.com> > > We return 1, not the index of futex woken up. > > Signed-off-by: Jangwoong Kim <6812skiii@gmail.com> > --- > kernel/futex/waitwake.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/kernel/futex/waitwake.c b/kernel/futex/waitwake.c > index 4ce0923f1ce3..d148e5d4956b 100644 > --- a/kernel/futex/waitwake.c > +++ b/kernel/futex/waitwake.c > @@ -455,8 +455,8 @@ static int futex_wait_multiple_setup(struct futex_vector *vs, int count, int *wo > > /* > * Even if something went wrong, if we find out that a futex > - * was woken, we don't return error and return this index to > - * userspace > + * was woken, we don't return error and make userspace aware > + * of this by returning 1. We return to userspace the value at *woken, so your fix is wrong. Have a look at futex_wait_multiple(): ret = futex_wait_multiple_setup(vs, count, &hint); if (ret) { if (ret > 0) { /* A futex was woken during setup */ ret = hint; } return ret; } When we return 1 at futex_wait_multiple_setup(), we end up returning the hint/woken value to userspace. Let me know if you have questions. André ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] futex: Fix a faulty comment. 2021-12-06 12:12 ` André Almeida @ 2021-12-06 17:44 ` Jangwoong Kim 2021-12-06 20:34 ` André Almeida 0 siblings, 1 reply; 4+ messages in thread From: Jangwoong Kim @ 2021-12-06 17:44 UTC (permalink / raw) To: André Almeida; +Cc: peterz, mingo, tglx, dvhart, dave, linux-kernel Hi André. That patch was definitely wrong, I apologize. However, since futex_wait_multiple_setup() sets the last index of futex that was woken up, shouldn't the comment be modified as below? If so, I will resend a patch. /* - * Even if something went wrong, if we find out that a futex - * was woken, we don't return error and return this index to - * userspace + * Even if something went wrong, if we find out that any futex + * was woken, we don't return error and return the last index + * awoken to userspace */ *woken = unqueue_multiple(vs, i); if (*woken >= 0) I sent the patch because I thought this was important enough to be corrected. Let me know If this is not crucial enough to be patched, so I won't keep sending comment-fixing patches. Thank you. Jangwoong Kim. 2021년 12월 6일 (월) 오후 9:12, André Almeida <andrealmeid@collabora.com>님이 작성: > > Hi Jangwoong, > > Thanks for your patch! However... > > Às 15:14 de 04/12/21, 6812skiii@gmail.com escreveu: > > From: Jangwoong Kim <6812skiii@gmail.com> > > > > We return 1, not the index of futex woken up. > > > > Signed-off-by: Jangwoong Kim <6812skiii@gmail.com> > > --- > > kernel/futex/waitwake.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/kernel/futex/waitwake.c b/kernel/futex/waitwake.c > > index 4ce0923f1ce3..d148e5d4956b 100644 > > --- a/kernel/futex/waitwake.c > > +++ b/kernel/futex/waitwake.c > > @@ -455,8 +455,8 @@ static int futex_wait_multiple_setup(struct futex_vector *vs, int count, int *wo > > > > /* > > * Even if something went wrong, if we find out that a futex > > - * was woken, we don't return error and return this index to > > - * userspace > > + * was woken, we don't return error and make userspace aware > > + * of this by returning 1. > > We return to userspace the value at *woken, so your fix is wrong. Have a > look at futex_wait_multiple(): > > ret = futex_wait_multiple_setup(vs, count, &hint); > if (ret) { > if (ret > 0) { > /* A futex was woken during setup */ > ret = hint; > } > return ret; > } > > When we return 1 at futex_wait_multiple_setup(), we end up returning the > hint/woken value to userspace. > > Let me know if you have questions. > > André ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] futex: Fix a faulty comment. 2021-12-06 17:44 ` Jangwoong Kim @ 2021-12-06 20:34 ` André Almeida 0 siblings, 0 replies; 4+ messages in thread From: André Almeida @ 2021-12-06 20:34 UTC (permalink / raw) To: Jangwoong Kim; +Cc: peterz, mingo, tglx, dvhart, dave, linux-kernel Hi Jangwoong, Please don't top post when replying to emails in this list (read more here: https://www.mediawiki.org/wiki/Mailing_list_etiquette) Às 14:44 de 06/12/21, Jangwoong Kim escreveu: > Hi André. > > That patch was definitely wrong, I apologize. > > However, since futex_wait_multiple_setup() sets the last index of > futex that was woken up, > shouldn't the comment be modified as below? > > If so, I will resend a patch. > > /* > - * Even if something went wrong, if we find out that a futex > - * was woken, we don't return error and return this index to > - * userspace > + * Even if something went wrong, if we find out that any futex > + * was woken, we don't return error and return the last index > + * awoken to userspace Indeed, this is more correct. You can send a patch to clarify this comment. > */ > *woken = unqueue_multiple(vs, i); > if (*woken >= 0) > > I sent the patch because I thought this was important enough to be corrected. > > Let me know If this is not crucial enough to be patched, so I won't > keep sending comment-fixing patches. > > Thank you. > Jangwoong Kim. > > 2021년 12월 6일 (월) 오후 9:12, André Almeida <andrealmeid@collabora.com>님이 작성: >> >> Hi Jangwoong, >> >> Thanks for your patch! However... >> >> Às 15:14 de 04/12/21, 6812skiii@gmail.com escreveu: >>> From: Jangwoong Kim <6812skiii@gmail.com> >>> >>> We return 1, not the index of futex woken up. >>> >>> Signed-off-by: Jangwoong Kim <6812skiii@gmail.com> >>> --- >>> kernel/futex/waitwake.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/kernel/futex/waitwake.c b/kernel/futex/waitwake.c >>> index 4ce0923f1ce3..d148e5d4956b 100644 >>> --- a/kernel/futex/waitwake.c >>> +++ b/kernel/futex/waitwake.c >>> @@ -455,8 +455,8 @@ static int futex_wait_multiple_setup(struct futex_vector *vs, int count, int *wo >>> >>> /* >>> * Even if something went wrong, if we find out that a futex >>> - * was woken, we don't return error and return this index to >>> - * userspace >>> + * was woken, we don't return error and make userspace aware >>> + * of this by returning 1. >> >> We return to userspace the value at *woken, so your fix is wrong. Have a >> look at futex_wait_multiple(): >> >> ret = futex_wait_multiple_setup(vs, count, &hint); >> if (ret) { >> if (ret > 0) { >> /* A futex was woken during setup */ >> ret = hint; >> } >> return ret; >> } >> >> When we return 1 at futex_wait_multiple_setup(), we end up returning the >> hint/woken value to userspace. >> >> Let me know if you have questions. >> >> André ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-12-06 20:35 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-04 18:14 [PATCH] futex: Fix a faulty comment 6812skiii 2021-12-06 12:12 ` André Almeida 2021-12-06 17:44 ` Jangwoong Kim 2021-12-06 20:34 ` André Almeida
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).