Messages in this thread | | | Date | Mon, 6 Dec 2021 17:34:56 -0300 | Subject | Re: [PATCH] futex: Fix a faulty comment. | From | André Almeida <> |
| |
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é
| |