linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] io_uring: use cpu_online() to check p->sq_thread_cpu instead of cpu_possible()
       [not found] <20190507080016.1945-1-shhuiw@foxmail.com>
@ 2019-05-07 14:40 ` Jens Axboe
  0 siblings, 0 replies; 9+ messages in thread
From: Jens Axboe @ 2019-05-07 14:40 UTC (permalink / raw)
  To: Shenghui Wang, viro, linux-block, linux-fsdevel; +Cc: jmoyer

On 5/7/19 2:00 AM, Shenghui Wang wrote:
> This issue is found by running liburing/test/io_uring_setup test.
> 
> When test run, the testcase "attempt to bind to invalid cpu" would not
> pass with messages like:
>    io_uring_setup(1, 0xbfc2f7c8), \
> flags: IORING_SETUP_SQPOLL|IORING_SETUP_SQ_AFF, \
> resv: 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000, \
> sq_thread_cpu: 2
>    expected -1, got 3
>    FAIL
> 
> On my system, there is:
>    CPU(s) possible : 0-3
>    CPU(s) online   : 0-1
>    CPU(s) offline  : 2-3
>    CPU(s) present  : 0-1
> 
> The sq_thread_cpu 2 is offline on my system, so the bind should fail.
> But cpu_possible() will pass the check. We shouldn't be able to bind
> to an offline cpu. Use cpu_online() to do the check.
> 
> After the change, the testcase run as expected: EINVAL will be returned
> for cpu offlined.

Applied, thanks.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] io_uring: use cpu_online() to check p->sq_thread_cpu instead of cpu_possible()
  2019-05-07  8:03 Shenghui Wang
@ 2019-05-07 11:22 ` Jeff Moyer
  0 siblings, 0 replies; 9+ messages in thread
From: Jeff Moyer @ 2019-05-07 11:22 UTC (permalink / raw)
  To: Shenghui Wang; +Cc: axboe, viro, linux-block, linux-fsdevel

Shenghui Wang <shhuiw@foxmail.com> writes:

> This issue is found by running liburing/test/io_uring_setup test.
>
> When test run, the testcase "attempt to bind to invalid cpu" would not
> pass with messages like:
>    io_uring_setup(1, 0xbfc2f7c8), \
> flags: IORING_SETUP_SQPOLL|IORING_SETUP_SQ_AFF, \
> resv: 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000, \
> sq_thread_cpu: 2
>    expected -1, got 3
>    FAIL
>
> On my system, there is:
>    CPU(s) possible : 0-3
>    CPU(s) online   : 0-1
>    CPU(s) offline  : 2-3
>    CPU(s) present  : 0-1
>
> The sq_thread_cpu 2 is offline on my system, so the bind should fail.
> But cpu_possible() will pass the check. We shouldn't be able to bind
> to an offline cpu. Use cpu_online() to do the check.
>
> After the change, the testcase run as expected: EINVAL will be returned
> for cpu offlined.
>
> Signed-off-by: Shenghui Wang <shhuiw@foxmail.com>
> ---
>  fs/io_uring.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index d91cbd53d3ca..718d7b873f4a 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -2472,7 +2472,7 @@ static int io_sq_offload_start(struct io_ring_ctx *ctx,
>  							nr_cpu_ids);
>  
>  			ret = -EINVAL;
> -			if (!cpu_possible(cpu))
> +			if (!cpu_online(cpu))
>  				goto err;
>  
>  			ctx->sqo_thread = kthread_create_on_cpu(io_sq_thread,

Reviewed-by: Jeff Moyer <jmoyer@redhat.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] io_uring: use cpu_online() to check p->sq_thread_cpu instead of cpu_possible()
@ 2019-05-07  8:03 Shenghui Wang
  2019-05-07 11:22 ` Jeff Moyer
  0 siblings, 1 reply; 9+ messages in thread
From: Shenghui Wang @ 2019-05-07  8:03 UTC (permalink / raw)
  To: axboe, viro, linux-block, linux-fsdevel; +Cc: jmoyer

This issue is found by running liburing/test/io_uring_setup test.

When test run, the testcase "attempt to bind to invalid cpu" would not
pass with messages like:
   io_uring_setup(1, 0xbfc2f7c8), \
flags: IORING_SETUP_SQPOLL|IORING_SETUP_SQ_AFF, \
resv: 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000, \
sq_thread_cpu: 2
   expected -1, got 3
   FAIL

On my system, there is:
   CPU(s) possible : 0-3
   CPU(s) online   : 0-1
   CPU(s) offline  : 2-3
   CPU(s) present  : 0-1

The sq_thread_cpu 2 is offline on my system, so the bind should fail.
But cpu_possible() will pass the check. We shouldn't be able to bind
to an offline cpu. Use cpu_online() to do the check.

After the change, the testcase run as expected: EINVAL will be returned
for cpu offlined.

Signed-off-by: Shenghui Wang <shhuiw@foxmail.com>
---
 fs/io_uring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index d91cbd53d3ca..718d7b873f4a 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -2472,7 +2472,7 @@ static int io_sq_offload_start(struct io_ring_ctx *ctx,
 							nr_cpu_ids);
 
 			ret = -EINVAL;
-			if (!cpu_possible(cpu))
+			if (!cpu_online(cpu))
 				goto err;
 
 			ctx->sqo_thread = kthread_create_on_cpu(io_sq_thread,
-- 
2.20.1




^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] io_uring: use cpu_online() to check p->sq_thread_cpu instead of cpu_possible()
  2019-05-01 14:39       ` Jens Axboe
@ 2019-05-01 15:40         ` Jeff Moyer
  0 siblings, 0 replies; 9+ messages in thread
From: Jeff Moyer @ 2019-05-01 15:40 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Shenghui Wang, viro, linux-block, linux-fsdevel

Jens Axboe <axboe@kernel.dk> writes:

> Agree, I've cleaned it up, it was a bit of a mess.

LGTM, thanks!

-Jeff

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] io_uring: use cpu_online() to check p->sq_thread_cpu instead of cpu_possible()
  2019-05-01 14:32     ` Jeff Moyer
@ 2019-05-01 14:39       ` Jens Axboe
  2019-05-01 15:40         ` Jeff Moyer
  0 siblings, 1 reply; 9+ messages in thread
From: Jens Axboe @ 2019-05-01 14:39 UTC (permalink / raw)
  To: Jeff Moyer; +Cc: Shenghui Wang, viro, linux-block, linux-fsdevel

On 5/1/19 8:32 AM, Jeff Moyer wrote:
> Jens Axboe <axboe@kernel.dk> writes:
> 
>> On 5/1/19 5:56 AM, Jeff Moyer wrote:
>>> Shenghui Wang <shhuiw@foxmail.com> writes:
>>>
>>>> This issue is found by running liburing/test/io_uring_setup test.
>>>>
>>>> When test run, the testcase "attempt to bind to invalid cpu" would not
>>>> pass with messages like:
>>>>    io_uring_setup(1, 0xbfc2f7c8), \
>>>> flags: IORING_SETUP_SQPOLL|IORING_SETUP_SQ_AFF, \
>>>> resv: 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000, \
>>>> sq_thread_cpu: 2
>>>>    expected -1, got 3
>>>>    FAIL
>>>>
>>>> On my system, there is:
>>>>    CPU(s) possible : 0-3
>>>>    CPU(s) online   : 0-1
>>>>    CPU(s) offline  : 2-3
>>>>    CPU(s) present  : 0-1
>>>>
>>>> The sq_thread_cpu 2 is offline on my system, so the bind should fail.
>>>> But cpu_possible() will pass the check. We shouldn't be able to bind
>>>> to an offline cpu. Use cpu_online() to do the check.
>>>>
>>>> After the change, the testcase run as expected: EINVAL will be returned
>>>> for cpu offlined.
>>>>
>>>> Signed-off-by: Shenghui Wang <shhuiw@foxmail.com>
>>>> ---
>>>>  fs/io_uring.c | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>>>> index 0e9fb2cb1984..aa3d39860a1c 100644
>>>> --- a/fs/io_uring.c
>>>> +++ b/fs/io_uring.c
>>>> @@ -2241,7 +2241,7 @@ static int io_sq_offload_start(struct io_ring_ctx *ctx,
>>>>  	ctx->sqo_mm = current->mm;
>>>>  
>>>>  	ret = -EINVAL;
>>>> -	if (!cpu_possible(p->sq_thread_cpu))
>>>> +	if (!cpu_online(p->sq_thread_cpu))
>>>>  		goto err;
>>>>  
>>>>  	if (ctx->flags & IORING_SETUP_SQPOLL) {
>>>> @@ -2258,7 +2258,7 @@ static int io_sq_offload_start(struct io_ring_ctx *ctx,
>>>>  
>>>>  			cpu = array_index_nospec(p->sq_thread_cpu, NR_CPUS);
>>>>  			ret = -EINVAL;
>>>> -			if (!cpu_possible(p->sq_thread_cpu))
>>>> +			if (!cpu_online(p->sq_thread_cpu))
>>>>  				goto err;
>>>>  
>>>>  			ctx->sqo_thread = kthread_create_on_cpu(io_sq_thread,
>>>
>>> Hmm.  Why are we doing this check twice?  Oh... Jens, I think you
>>> braino'd commit 917257daa0fea.  Have a look.  You probably wanted to get
>>> rid of the first check for cpu_possible.
>>
>> Added a fixup patch the other day:
>>
>> http://git.kernel.dk/cgit/linux-block/commit/?h=for-linus&id=362bf8670efccebca22efda1ee5a5ee831ec5efb
> 
> @@ -2333,13 +2329,14 @@ static int io_sq_offload_start(struct io_ring_ctx *ctx,
>  			ctx->sq_thread_idle = HZ;
>  
>  		if (p->flags & IORING_SETUP_SQ_AFF) {
> -			int cpu;
> +			int cpu = p->sq_thread_cpu;
>  
> -			cpu = array_index_nospec(p->sq_thread_cpu, NR_CPUS);
>  			ret = -EINVAL;
> -			if (!cpu_possible(p->sq_thread_cpu))
> +			if (cpu >= nr_cpu_ids || !cpu_possible(cpu))
>  				goto err;
>  
> +			cpu = array_index_nospec(cpu, nr_cpu_ids);
> +
> 
> Why do you do the array_index_nospec last?  Why wouldn't that be written
> as:
> 
> 	if (p->flags & IORING_SETUP_SQ_AFF) {
> 		int cpu = array_index_nospec(p->sq_thread_cpu, nr_cpu_ids);
> 
> 		ret = -EINVAL;
> 		if (!cpu_possible(cpu))
> 			goto err;
> 
> 		ctx->sqo_thread = kthread_create_on_cpu(io_sq_thread,
> 						ctx, cpu,
> 						"io_uring-sq");
> 	} else {
> ...
> 
> That would take away some head-scratching for me.

Agree, I've cleaned it up, it was a bit of a mess.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] io_uring: use cpu_online() to check p->sq_thread_cpu instead of cpu_possible()
  2019-05-01 14:15   ` Jens Axboe
@ 2019-05-01 14:32     ` Jeff Moyer
  2019-05-01 14:39       ` Jens Axboe
  0 siblings, 1 reply; 9+ messages in thread
From: Jeff Moyer @ 2019-05-01 14:32 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Shenghui Wang, viro, linux-block, linux-fsdevel

Jens Axboe <axboe@kernel.dk> writes:

> On 5/1/19 5:56 AM, Jeff Moyer wrote:
>> Shenghui Wang <shhuiw@foxmail.com> writes:
>> 
>>> This issue is found by running liburing/test/io_uring_setup test.
>>>
>>> When test run, the testcase "attempt to bind to invalid cpu" would not
>>> pass with messages like:
>>>    io_uring_setup(1, 0xbfc2f7c8), \
>>> flags: IORING_SETUP_SQPOLL|IORING_SETUP_SQ_AFF, \
>>> resv: 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000, \
>>> sq_thread_cpu: 2
>>>    expected -1, got 3
>>>    FAIL
>>>
>>> On my system, there is:
>>>    CPU(s) possible : 0-3
>>>    CPU(s) online   : 0-1
>>>    CPU(s) offline  : 2-3
>>>    CPU(s) present  : 0-1
>>>
>>> The sq_thread_cpu 2 is offline on my system, so the bind should fail.
>>> But cpu_possible() will pass the check. We shouldn't be able to bind
>>> to an offline cpu. Use cpu_online() to do the check.
>>>
>>> After the change, the testcase run as expected: EINVAL will be returned
>>> for cpu offlined.
>>>
>>> Signed-off-by: Shenghui Wang <shhuiw@foxmail.com>
>>> ---
>>>  fs/io_uring.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>>> index 0e9fb2cb1984..aa3d39860a1c 100644
>>> --- a/fs/io_uring.c
>>> +++ b/fs/io_uring.c
>>> @@ -2241,7 +2241,7 @@ static int io_sq_offload_start(struct io_ring_ctx *ctx,
>>>  	ctx->sqo_mm = current->mm;
>>>  
>>>  	ret = -EINVAL;
>>> -	if (!cpu_possible(p->sq_thread_cpu))
>>> +	if (!cpu_online(p->sq_thread_cpu))
>>>  		goto err;
>>>  
>>>  	if (ctx->flags & IORING_SETUP_SQPOLL) {
>>> @@ -2258,7 +2258,7 @@ static int io_sq_offload_start(struct io_ring_ctx *ctx,
>>>  
>>>  			cpu = array_index_nospec(p->sq_thread_cpu, NR_CPUS);
>>>  			ret = -EINVAL;
>>> -			if (!cpu_possible(p->sq_thread_cpu))
>>> +			if (!cpu_online(p->sq_thread_cpu))
>>>  				goto err;
>>>  
>>>  			ctx->sqo_thread = kthread_create_on_cpu(io_sq_thread,
>> 
>> Hmm.  Why are we doing this check twice?  Oh... Jens, I think you
>> braino'd commit 917257daa0fea.  Have a look.  You probably wanted to get
>> rid of the first check for cpu_possible.
>
> Added a fixup patch the other day:
>
> http://git.kernel.dk/cgit/linux-block/commit/?h=for-linus&id=362bf8670efccebca22efda1ee5a5ee831ec5efb

@@ -2333,13 +2329,14 @@ static int io_sq_offload_start(struct io_ring_ctx *ctx,
 			ctx->sq_thread_idle = HZ;
 
 		if (p->flags & IORING_SETUP_SQ_AFF) {
-			int cpu;
+			int cpu = p->sq_thread_cpu;
 
-			cpu = array_index_nospec(p->sq_thread_cpu, NR_CPUS);
 			ret = -EINVAL;
-			if (!cpu_possible(p->sq_thread_cpu))
+			if (cpu >= nr_cpu_ids || !cpu_possible(cpu))
 				goto err;
 
+			cpu = array_index_nospec(cpu, nr_cpu_ids);
+

Why do you do the array_index_nospec last?  Why wouldn't that be written
as:

	if (p->flags & IORING_SETUP_SQ_AFF) {
		int cpu = array_index_nospec(p->sq_thread_cpu, nr_cpu_ids);

		ret = -EINVAL;
		if (!cpu_possible(cpu))
			goto err;

		ctx->sqo_thread = kthread_create_on_cpu(io_sq_thread,
						ctx, cpu,
						"io_uring-sq");
	} else {
...

That would take away some head-scratching for me.

Cheers,
Jeff

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] io_uring: use cpu_online() to check p->sq_thread_cpu instead of cpu_possible()
  2019-05-01 11:56 ` Jeff Moyer
@ 2019-05-01 14:15   ` Jens Axboe
  2019-05-01 14:32     ` Jeff Moyer
  0 siblings, 1 reply; 9+ messages in thread
From: Jens Axboe @ 2019-05-01 14:15 UTC (permalink / raw)
  To: Jeff Moyer, Shenghui Wang; +Cc: viro, linux-block, linux-fsdevel

On 5/1/19 5:56 AM, Jeff Moyer wrote:
> Shenghui Wang <shhuiw@foxmail.com> writes:
> 
>> This issue is found by running liburing/test/io_uring_setup test.
>>
>> When test run, the testcase "attempt to bind to invalid cpu" would not
>> pass with messages like:
>>    io_uring_setup(1, 0xbfc2f7c8), \
>> flags: IORING_SETUP_SQPOLL|IORING_SETUP_SQ_AFF, \
>> resv: 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000, \
>> sq_thread_cpu: 2
>>    expected -1, got 3
>>    FAIL
>>
>> On my system, there is:
>>    CPU(s) possible : 0-3
>>    CPU(s) online   : 0-1
>>    CPU(s) offline  : 2-3
>>    CPU(s) present  : 0-1
>>
>> The sq_thread_cpu 2 is offline on my system, so the bind should fail.
>> But cpu_possible() will pass the check. We shouldn't be able to bind
>> to an offline cpu. Use cpu_online() to do the check.
>>
>> After the change, the testcase run as expected: EINVAL will be returned
>> for cpu offlined.
>>
>> Signed-off-by: Shenghui Wang <shhuiw@foxmail.com>
>> ---
>>  fs/io_uring.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>> index 0e9fb2cb1984..aa3d39860a1c 100644
>> --- a/fs/io_uring.c
>> +++ b/fs/io_uring.c
>> @@ -2241,7 +2241,7 @@ static int io_sq_offload_start(struct io_ring_ctx *ctx,
>>  	ctx->sqo_mm = current->mm;
>>  
>>  	ret = -EINVAL;
>> -	if (!cpu_possible(p->sq_thread_cpu))
>> +	if (!cpu_online(p->sq_thread_cpu))
>>  		goto err;
>>  
>>  	if (ctx->flags & IORING_SETUP_SQPOLL) {
>> @@ -2258,7 +2258,7 @@ static int io_sq_offload_start(struct io_ring_ctx *ctx,
>>  
>>  			cpu = array_index_nospec(p->sq_thread_cpu, NR_CPUS);
>>  			ret = -EINVAL;
>> -			if (!cpu_possible(p->sq_thread_cpu))
>> +			if (!cpu_online(p->sq_thread_cpu))
>>  				goto err;
>>  
>>  			ctx->sqo_thread = kthread_create_on_cpu(io_sq_thread,
> 
> Hmm.  Why are we doing this check twice?  Oh... Jens, I think you
> braino'd commit 917257daa0fea.  Have a look.  You probably wanted to get
> rid of the first check for cpu_possible.

Added a fixup patch the other day:

http://git.kernel.dk/cgit/linux-block/commit/?h=for-linus&id=362bf8670efccebca22efda1ee5a5ee831ec5efb

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] io_uring: use cpu_online() to check p->sq_thread_cpu instead of cpu_possible()
  2019-05-01  7:24 Shenghui Wang
@ 2019-05-01 11:56 ` Jeff Moyer
  2019-05-01 14:15   ` Jens Axboe
  0 siblings, 1 reply; 9+ messages in thread
From: Jeff Moyer @ 2019-05-01 11:56 UTC (permalink / raw)
  To: axboe, Shenghui Wang; +Cc: viro, linux-block, linux-fsdevel

Shenghui Wang <shhuiw@foxmail.com> writes:

> This issue is found by running liburing/test/io_uring_setup test.
>
> When test run, the testcase "attempt to bind to invalid cpu" would not
> pass with messages like:
>    io_uring_setup(1, 0xbfc2f7c8), \
> flags: IORING_SETUP_SQPOLL|IORING_SETUP_SQ_AFF, \
> resv: 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000, \
> sq_thread_cpu: 2
>    expected -1, got 3
>    FAIL
>
> On my system, there is:
>    CPU(s) possible : 0-3
>    CPU(s) online   : 0-1
>    CPU(s) offline  : 2-3
>    CPU(s) present  : 0-1
>
> The sq_thread_cpu 2 is offline on my system, so the bind should fail.
> But cpu_possible() will pass the check. We shouldn't be able to bind
> to an offline cpu. Use cpu_online() to do the check.
>
> After the change, the testcase run as expected: EINVAL will be returned
> for cpu offlined.
>
> Signed-off-by: Shenghui Wang <shhuiw@foxmail.com>
> ---
>  fs/io_uring.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 0e9fb2cb1984..aa3d39860a1c 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -2241,7 +2241,7 @@ static int io_sq_offload_start(struct io_ring_ctx *ctx,
>  	ctx->sqo_mm = current->mm;
>  
>  	ret = -EINVAL;
> -	if (!cpu_possible(p->sq_thread_cpu))
> +	if (!cpu_online(p->sq_thread_cpu))
>  		goto err;
>  
>  	if (ctx->flags & IORING_SETUP_SQPOLL) {
> @@ -2258,7 +2258,7 @@ static int io_sq_offload_start(struct io_ring_ctx *ctx,
>  
>  			cpu = array_index_nospec(p->sq_thread_cpu, NR_CPUS);
>  			ret = -EINVAL;
> -			if (!cpu_possible(p->sq_thread_cpu))
> +			if (!cpu_online(p->sq_thread_cpu))
>  				goto err;
>  
>  			ctx->sqo_thread = kthread_create_on_cpu(io_sq_thread,

Hmm.  Why are we doing this check twice?  Oh... Jens, I think you
braino'd commit 917257daa0fea.  Have a look.  You probably wanted to get
rid of the first check for cpu_possible.

-Jeff

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] io_uring: use cpu_online() to check p->sq_thread_cpu instead of cpu_possible()
@ 2019-05-01  7:24 Shenghui Wang
  2019-05-01 11:56 ` Jeff Moyer
  0 siblings, 1 reply; 9+ messages in thread
From: Shenghui Wang @ 2019-05-01  7:24 UTC (permalink / raw)
  To: axboe, viro, linux-block, linux-fsdevel; +Cc: jmoyer

This issue is found by running liburing/test/io_uring_setup test.

When test run, the testcase "attempt to bind to invalid cpu" would not
pass with messages like:
   io_uring_setup(1, 0xbfc2f7c8), \
flags: IORING_SETUP_SQPOLL|IORING_SETUP_SQ_AFF, \
resv: 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000, \
sq_thread_cpu: 2
   expected -1, got 3
   FAIL

On my system, there is:
   CPU(s) possible : 0-3
   CPU(s) online   : 0-1
   CPU(s) offline  : 2-3
   CPU(s) present  : 0-1

The sq_thread_cpu 2 is offline on my system, so the bind should fail.
But cpu_possible() will pass the check. We shouldn't be able to bind
to an offline cpu. Use cpu_online() to do the check.

After the change, the testcase run as expected: EINVAL will be returned
for cpu offlined.

Signed-off-by: Shenghui Wang <shhuiw@foxmail.com>
---
 fs/io_uring.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 0e9fb2cb1984..aa3d39860a1c 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -2241,7 +2241,7 @@ static int io_sq_offload_start(struct io_ring_ctx *ctx,
 	ctx->sqo_mm = current->mm;
 
 	ret = -EINVAL;
-	if (!cpu_possible(p->sq_thread_cpu))
+	if (!cpu_online(p->sq_thread_cpu))
 		goto err;
 
 	if (ctx->flags & IORING_SETUP_SQPOLL) {
@@ -2258,7 +2258,7 @@ static int io_sq_offload_start(struct io_ring_ctx *ctx,
 
 			cpu = array_index_nospec(p->sq_thread_cpu, NR_CPUS);
 			ret = -EINVAL;
-			if (!cpu_possible(p->sq_thread_cpu))
+			if (!cpu_online(p->sq_thread_cpu))
 				goto err;
 
 			ctx->sqo_thread = kthread_create_on_cpu(io_sq_thread,
-- 
2.20.1




^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-05-07 14:40 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20190507080016.1945-1-shhuiw@foxmail.com>
2019-05-07 14:40 ` [PATCH] io_uring: use cpu_online() to check p->sq_thread_cpu instead of cpu_possible() Jens Axboe
2019-05-07  8:03 Shenghui Wang
2019-05-07 11:22 ` Jeff Moyer
  -- strict thread matches above, loose matches on Subject: below --
2019-05-01  7:24 Shenghui Wang
2019-05-01 11:56 ` Jeff Moyer
2019-05-01 14:15   ` Jens Axboe
2019-05-01 14:32     ` Jeff Moyer
2019-05-01 14:39       ` Jens Axboe
2019-05-01 15:40         ` Jeff Moyer

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).