Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <e2d478de-24de-dc56-ca33-e008c09163ee@kernel.dk>
Date:   Wed, 12 Jul 2023 09:06:35 -0600
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:102.0) Gecko/20100101
 Thunderbird/102.12.0
Subject: Re: [PATCH 5/7] futex: make futex_parse_waitv() available as a helper
Content-Language: en-US
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     io-uring@vger.kernel.org, linux-kernel@vger.kernel.org,
        tglx@linutronix.de, mingo@redhat.com
References: <20230712004705.316157-1-axboe@kernel.dk>
 <20230712004705.316157-6-axboe@kernel.dk>
 <20230712092514.GE3100107@hirez.programming.kicks-ass.net>
From:   Jens Axboe <axboe@kernel.dk>
In-Reply-To: <20230712092514.GE3100107@hirez.programming.kicks-ass.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 7/12/23 3:25?AM, Peter Zijlstra wrote:
> On Tue, Jul 11, 2023 at 06:47:03PM -0600, Jens Axboe wrote:
> 
>> Since we now provide a way to pass in a wake handler and data, ensure we
>> use __futex_queue() to avoid having futex_queue() overwrite our wait
>> data.
> 
>> diff --git a/kernel/futex/waitwake.c b/kernel/futex/waitwake.c
>> index 3471af87cb7d..dfd02ca5ecfa 100644
>> --- a/kernel/futex/waitwake.c
>> +++ b/kernel/futex/waitwake.c
>> @@ -446,7 +446,8 @@ static int futex_wait_multiple_setup(struct futex_vector *vs, int count, int *wo
>>  			 * next futex. Queue each futex at this moment so hb can
>>  			 * be unlocked.
>>  			 */
>> -			futex_queue(q, hb);
>> +			__futex_queue(q, hb);
>> +			spin_unlock(&hb->lock);
>>  			continue;
>>  		}
> 
> I'm not following; I even applied all your patches up to this point, but
> futex_queue() still reads:
> 
> static inline void futex_queue(struct futex_q *q, struct futex_hash_bucket *hb)
> 	__releases(&hb->lock)
> {
> 	__futex_queue(q, hb);
> 	spin_unlock(&hb->lock);
> }
> 
> How would it be different and overwrite anything ?!?

Good catch, this is a leftover from storing the task/wakeup data
separately. But I got rid of that, so it's stale comment at this point
and we can certainly use futex_queue() here again and drop this hunk.
Will make that edit.

-- 
Jens Axboe