diff mbox series

[v2] io_uring: Fix reversed nonblock flag

Message ID eecaf117de4894b595f300b9fb567825330b2d24.1570183599.git.asml.silence@gmail.com (mailing list archive)
State New, archived
Headers show
Series [v2] io_uring: Fix reversed nonblock flag | expand

Commit Message

Pavel Begunkov Oct. 4, 2019, 10:07 a.m. UTC
From: Pavel Begunkov <asml.silence@gmail.com>

io_queue_link_head() accepts @force_nonblock flag, but io_ring_submit()
passes something opposite.

v2: fix build error by test robot: Rebase from custom tree
Reported-by: kbuild test robot <lkp@intel.com>

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 fs/io_uring.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Jens Axboe Oct. 4, 2019, 1:05 p.m. UTC | #1
On 10/4/19 4:07 AM, Pavel Begunkov (Silence) wrote:
> From: Pavel Begunkov <asml.silence@gmail.com>
> 
> io_queue_link_head() accepts @force_nonblock flag, but io_ring_submit()
> passes something opposite.
> 
> v2: fix build error by test robot: Rebase from custom tree
> Reported-by: kbuild test robot <lkp@intel.com>
> 
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>   fs/io_uring.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index c934f91c51e9..c909ea2b84e9 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -2703,6 +2703,7 @@ static int io_ring_submit(struct io_ring_ctx *ctx, unsigned int to_submit,
>   	struct io_kiocb *shadow_req = NULL;
>   	bool prev_was_link = false;
>   	int i, submit = 0;
> +	bool force_nonblock = true;
>   
>   	if (to_submit > IO_PLUG_THRESHOLD) {
>   		io_submit_state_start(&state, ctx, to_submit);
> @@ -2710,9 +2711,9 @@ static int io_ring_submit(struct io_ring_ctx *ctx, unsigned int to_submit,
>   	}
>   
>   	for (i = 0; i < to_submit; i++) {
> -		bool force_nonblock = true;
>   		struct sqe_submit s;
>   
> +		force_nonblock = true;
>   		if (!io_get_sqring(ctx, &s))
>   			break;
>   
> @@ -2761,7 +2762,7 @@ static int io_ring_submit(struct io_ring_ctx *ctx, unsigned int to_submit,
>   
>   	if (link)
>   		io_queue_link_head(ctx, link, &link->submit, shadow_req,
> -					block_for_last);
> +					force_nonblock);
>   	if (statep)
>   		io_submit_state_end(statep);

Shouldn't this just be:

   		io_queue_link_head(ctx, link, &link->submit, shadow_req,
 					!block_for_last);

We're outside the loop, so by definition at the end of what we need to
do. We don't need to factor in the fiddling of force_nonblock here,
it'll be false at this point anyway. Only exception is error handling,
if the caller asked for more than what was in the ring. Not a big
deal...
Pavel Begunkov Oct. 4, 2019, 1:58 p.m. UTC | #2
On 04/10/2019 16:05, Jens Axboe wrote:
> On 10/4/19 4:07 AM, Pavel Begunkov (Silence) wrote:
>> From: Pavel Begunkov <asml.silence@gmail.com>
>>
>> io_queue_link_head() accepts @force_nonblock flag, but io_ring_submit()
>> passes something opposite.
>>
>> v2: fix build error by test robot: Rebase from custom tree
>> Reported-by: kbuild test robot <lkp@intel.com>
>>
>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>> ---
>>   fs/io_uring.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>> index c934f91c51e9..c909ea2b84e9 100644
>> --- a/fs/io_uring.c
>> +++ b/fs/io_uring.c
>> @@ -2703,6 +2703,7 @@ static int io_ring_submit(struct io_ring_ctx *ctx, unsigned int to_submit,
>>   	struct io_kiocb *shadow_req = NULL;
>>   	bool prev_was_link = false;
>>   	int i, submit = 0;
>> +	bool force_nonblock = true;
>>   
>>   	if (to_submit > IO_PLUG_THRESHOLD) {
>>   		io_submit_state_start(&state, ctx, to_submit);
>> @@ -2710,9 +2711,9 @@ static int io_ring_submit(struct io_ring_ctx *ctx, unsigned int to_submit,
>>   	}
>>   
>>   	for (i = 0; i < to_submit; i++) {
>> -		bool force_nonblock = true;
>>   		struct sqe_submit s;
>>   
>> +		force_nonblock = true;
>>   		if (!io_get_sqring(ctx, &s))
>>   			break;
>>   
>> @@ -2761,7 +2762,7 @@ static int io_ring_submit(struct io_ring_ctx *ctx, unsigned int to_submit,
>>   
>>   	if (link)
>>   		io_queue_link_head(ctx, link, &link->submit, shadow_req,
>> -					block_for_last);
>> +					force_nonblock);
>>   	if (statep)
>>   		io_submit_state_end(statep);
> 
> Shouldn't this just be:
> 
>    		io_queue_link_head(ctx, link, &link->submit, shadow_req,
>  					!block_for_last);
> 
> We're outside the loop, so by definition at the end of what we need to
> do. We don't need to factor in the fiddling of force_nonblock here,
> it'll be false at this point anyway. Only exception is error handling,
> if the caller asked for more than what was in the ring. Not a big
> deal...

Thanks for explaining this, I'll resend.
Played safe because of breaks in the loop.
diff mbox series

Patch

diff --git a/fs/io_uring.c b/fs/io_uring.c
index c934f91c51e9..c909ea2b84e9 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -2703,6 +2703,7 @@  static int io_ring_submit(struct io_ring_ctx *ctx, unsigned int to_submit,
 	struct io_kiocb *shadow_req = NULL;
 	bool prev_was_link = false;
 	int i, submit = 0;
+	bool force_nonblock = true;
 
 	if (to_submit > IO_PLUG_THRESHOLD) {
 		io_submit_state_start(&state, ctx, to_submit);
@@ -2710,9 +2711,9 @@  static int io_ring_submit(struct io_ring_ctx *ctx, unsigned int to_submit,
 	}
 
 	for (i = 0; i < to_submit; i++) {
-		bool force_nonblock = true;
 		struct sqe_submit s;
 
+		force_nonblock = true;
 		if (!io_get_sqring(ctx, &s))
 			break;
 
@@ -2761,7 +2762,7 @@  static int io_ring_submit(struct io_ring_ctx *ctx, unsigned int to_submit,
 
 	if (link)
 		io_queue_link_head(ctx, link, &link->submit, shadow_req,
-					block_for_last);
+					force_nonblock);
 	if (statep)
 		io_submit_state_end(statep);