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: <9b6522bc-314e-d663-a035-c4614b21b756@kernel.dk>
Date:   Tue, 25 Jul 2023 15:24:30 -0600
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH 06/10] io_uring: add support for futex wake and wait
Content-Language: en-US
From:   Jens Axboe <axboe@kernel.dk>
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     io-uring@vger.kernel.org, linux-kernel@vger.kernel.org,
        andres@anarazel.de
References: <20230720221858.135240-1-axboe@kernel.dk>
 <20230720221858.135240-7-axboe@kernel.dk>
 <20230721113031.GG3630545@hirez.programming.kicks-ass.net>
 <20230721113718.GA3638458@hirez.programming.kicks-ass.net>
 <d95bfb98-8d76-f0fd-6283-efc01d0cc015@kernel.dk>
 <94b8fcc4-12b5-8d8c-3eb3-fe1e73a25456@kernel.dk>
 <20230725130015.GI3765278@hirez.programming.kicks-ass.net>
 <28a42d23-6d70-bc4c-5abc-0b3cc5d7338d@kernel.dk>
 <9a197037-4732-c524-2eb9-250ef7175a82@kernel.dk>
 <20230725151909.GT4253@hirez.programming.kicks-ass.net>
 <24a8a74a-e218-6105-ee97-02f60b1523bb@kernel.dk>
In-Reply-To: <24a8a74a-e218-6105-ee97-02f60b1523bb@kernel.dk>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 7/25/23 2:42?PM, Jens Axboe wrote:
> On 7/25/23 9:19?AM, Peter Zijlstra wrote:
>> On Tue, Jul 25, 2023 at 07:57:28AM -0600, Jens Axboe wrote:
>>
>>> Something like the below - totally untested, but just to show what I
>>> mean. Will need to get split and folded into the two separate patches.
>>> Will test and fold them later today.
>>>
>>>
>>> diff --git a/io_uring/futex.c b/io_uring/futex.c
>>> index 4c9f2c841b98..b0f90154d974 100644
>>> --- a/io_uring/futex.c
>>> +++ b/io_uring/futex.c
>>> @@ -168,7 +168,7 @@ bool io_futex_remove_all(struct io_ring_ctx *ctx, struct task_struct *task,
>>>  	return found;
>>>  }
>>>  
>>> -int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>> +static int __io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>>  {
>>>  	struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
>>>  	u32 flags;
>>> @@ -179,9 +179,6 @@ int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>>  	iof->uaddr = u64_to_user_ptr(READ_ONCE(sqe->addr));
>>>  	iof->futex_val = READ_ONCE(sqe->addr2);
>>>  	iof->futex_mask = READ_ONCE(sqe->addr3);
>>> -	iof->futex_nr = READ_ONCE(sqe->len);
>>> -	if (iof->futex_nr && req->opcode != IORING_OP_FUTEX_WAITV)
>>> -		return -EINVAL;
>>>  
>>>  	flags = READ_ONCE(sqe->futex_flags);
>>>  	if (flags & ~FUTEX2_MASK)
>>> @@ -191,14 +188,36 @@ int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>>  	if (!futex_flags_valid(iof->futex_flags))
>>>  		return -EINVAL;
>>>  
>>> -	if (!futex_validate_input(iof->futex_flags, iof->futex_val) ||
>>> -	    !futex_validate_input(iof->futex_flags, iof->futex_mask))
>>> +	if (!futex_validate_input(iof->futex_flags, iof->futex_mask))
>>>  		return -EINVAL;
>>>  
>>> -	iof->futexv_owned = 0;
>>>  	return 0;
>>>  }
>>
>> I think you can/should split more into io_futex_prep(), specifically
>> waitv should also have zero @val and @mask.
> 
> Yep, I'll include that. Updating them now...

It ends up just being this incremental for the very last patch, moving
all the waitv related prep to the wait prep and not relying on the
non-vectored one at all.


diff --git a/io_uring/futex.c b/io_uring/futex.c
index 4c9f2c841b98..e885aac12df8 100644
--- a/io_uring/futex.c
+++ b/io_uring/futex.c
@@ -179,9 +179,6 @@ int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	iof->uaddr = u64_to_user_ptr(READ_ONCE(sqe->addr));
 	iof->futex_val = READ_ONCE(sqe->addr2);
 	iof->futex_mask = READ_ONCE(sqe->addr3);
-	iof->futex_nr = READ_ONCE(sqe->len);
-	if (iof->futex_nr && req->opcode != IORING_OP_FUTEX_WAITV)
-		return -EINVAL;
 
 	flags = READ_ONCE(sqe->futex_flags);
 	if (flags & ~FUTEX2_MASK)
@@ -195,7 +192,6 @@ int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	    !futex_validate_input(iof->futex_flags, iof->futex_mask))
 		return -EINVAL;
 
-	iof->futexv_owned = 0;
 	return 0;
 }
 
@@ -220,10 +216,13 @@ int io_futexv_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	struct futex_vector *futexv;
 	int ret;
 
-	ret = io_futex_prep(req, sqe);
-	if (ret)
-		return ret;
+	/* No flags or mask supported for waitv */
+	if (unlikely(sqe->fd || sqe->buf_index || sqe->file_index ||
+		     sqe->addr2 || sqe->addr3))
+		return -EINVAL;
 
+	iof->uaddr = u64_to_user_ptr(READ_ONCE(sqe->addr));
+	iof->futex_nr = READ_ONCE(sqe->len);
 	if (!iof->futex_nr || iof->futex_nr > FUTEX_WAITV_MAX)
 		return -EINVAL;
 
@@ -238,6 +237,7 @@ int io_futexv_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 		return ret;
 	}
 
+	iof->futexv_owned = 0;
 	req->flags |= REQ_F_ASYNC_DATA;
 	req->async_data = futexv;
 	return 0;

-- 
Jens Axboe