* SQPOLL question
@ 2020-09-06 15:44 Josef
2020-09-06 15:49 ` Jens Axboe
0 siblings, 1 reply; 13+ messages in thread
From: Josef @ 2020-09-06 15:44 UTC (permalink / raw)
To: io-uring; +Cc: norman
Hi,
I'm trying to implement SQPOLL in netty at the moment, basically the
fd are registered by io_uring_register(2), which returns 0, but the
write event seems to fail with bad file descriptor error(-9) when
SQPOLL flag is enabled
small example to reproduce it:
https://gist.github.com/1Jo1/171790d549134b5b81ee51b23fb15cd0
what exactly am I doing wrong here? :)
---
Josef
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: SQPOLL question
2020-09-06 15:44 SQPOLL question Josef
@ 2020-09-06 15:49 ` Jens Axboe
2020-09-06 16:24 ` Josef
0 siblings, 1 reply; 13+ messages in thread
From: Jens Axboe @ 2020-09-06 15:49 UTC (permalink / raw)
To: Josef, io-uring; +Cc: norman
On 9/6/20 9:44 AM, Josef wrote:
> Hi,
>
> I'm trying to implement SQPOLL in netty at the moment, basically the
> fd are registered by io_uring_register(2), which returns 0, but the
> write event seems to fail with bad file descriptor error(-9) when
> SQPOLL flag is enabled
>
>
> small example to reproduce it:
> https://gist.github.com/1Jo1/171790d549134b5b81ee51b23fb15cd0
>
> what exactly am I doing wrong here? :)
You're using the 'fd' as the file descriptor, for registered files
you want to use the index instead. Since it's the only fd you
registered, the index would be 0 and that's what you should use.
It's worth mentioning that for 5.10 and on, SQPOLL will no longer
require registered files.
--
Jens Axboe
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: SQPOLL question
2020-09-06 15:49 ` Jens Axboe
@ 2020-09-06 16:24 ` Josef
2020-09-06 16:25 ` Jens Axboe
0 siblings, 1 reply; 13+ messages in thread
From: Josef @ 2020-09-06 16:24 UTC (permalink / raw)
To: Jens Axboe, io-uring; +Cc: norman
> You're using the 'fd' as the file descriptor, for registered files
> you want to use the index instead. Since it's the only fd you
> registered, the index would be 0 and that's what you should use.
oh..yeah it works, thanks :)
> It's worth mentioning that for 5.10 and on, SQPOLL will no longer
> require registered files.
that's awesome, it would be really handy as I just implemented a kind
of workaround in netty :)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: SQPOLL question
2020-09-06 16:24 ` Josef
@ 2020-09-06 16:25 ` Jens Axboe
2020-09-07 10:23 ` Josef
0 siblings, 1 reply; 13+ messages in thread
From: Jens Axboe @ 2020-09-06 16:25 UTC (permalink / raw)
To: Josef, io-uring; +Cc: norman
On 9/6/20 10:24 AM, Josef wrote:
>> You're using the 'fd' as the file descriptor, for registered files
>> you want to use the index instead. Since it's the only fd you
>> registered, the index would be 0 and that's what you should use.
>
> oh..yeah it works, thanks :)
Great!
>> It's worth mentioning that for 5.10 and on, SQPOLL will no longer
>> require registered files.
>
> that's awesome, it would be really handy as I just implemented a kind
> of workaround in netty :)
On top of that, capabilities will also be reduced from root to
CAP_SYS_NICE instead, and sharing across rings for the SQPOLL thread
will be supported. So it'll be a lot more useful/flexible in general.
--
Jens Axboe
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: SQPOLL question
2020-09-06 16:25 ` Jens Axboe
@ 2020-09-07 10:23 ` Josef
2020-09-07 12:49 ` Jens Axboe
0 siblings, 1 reply; 13+ messages in thread
From: Josef @ 2020-09-07 10:23 UTC (permalink / raw)
To: Jens Axboe, io-uring; +Cc: norman
> On top of that, capabilities will also be reduced from root to
> CAP_SYS_NICE instead, and sharing across rings for the SQPOLL thread
> will be supported. So it'll be a lot more useful/flexible in general.
oha that's nice, I'm pretty excited :)
I'm just wondering if all op are supported when the SQPOLL flag is
set? the accept op seems to fail with -EINVAL, when I enable SQPOLL
to reproduce it:
https://gist.github.com/1Jo1/accb91b737abb55d07487799739ad70a
(just want to test a non blocking accept op in SQPOLL mode)
---
Josef
On Sun, 6 Sep 2020 at 18:25, Jens Axboe <[email protected]> wrote:
>
> On 9/6/20 10:24 AM, Josef wrote:
> >> You're using the 'fd' as the file descriptor, for registered files
> >> you want to use the index instead. Since it's the only fd you
> >> registered, the index would be 0 and that's what you should use.
> >
> > oh..yeah it works, thanks :)
>
> Great!
>
> >> It's worth mentioning that for 5.10 and on, SQPOLL will no longer
> >> require registered files.
> >
> > that's awesome, it would be really handy as I just implemented a kind
> > of workaround in netty :)
>
> On top of that, capabilities will also be reduced from root to
> CAP_SYS_NICE instead, and sharing across rings for the SQPOLL thread
> will be supported. So it'll be a lot more useful/flexible in general.
>
> --
> Jens Axboe
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: SQPOLL question
2020-09-07 10:23 ` Josef
@ 2020-09-07 12:49 ` Jens Axboe
2020-09-07 14:58 ` Josef
0 siblings, 1 reply; 13+ messages in thread
From: Jens Axboe @ 2020-09-07 12:49 UTC (permalink / raw)
To: Josef, io-uring; +Cc: norman
On 9/7/20 4:23 AM, Josef wrote:
>> On top of that, capabilities will also be reduced from root to
>> CAP_SYS_NICE instead, and sharing across rings for the SQPOLL thread
>> will be supported. So it'll be a lot more useful/flexible in general.
>
> oha that's nice, I'm pretty excited :)
>
> I'm just wondering if all op are supported when the SQPOLL flag is
> set? the accept op seems to fail with -EINVAL, when I enable SQPOLL
>
> to reproduce it:
> https://gist.github.com/1Jo1/accb91b737abb55d07487799739ad70a
> (just want to test a non blocking accept op in SQPOLL mode)
Yes, that is known, you cannot open/close descriptors with the
SQPOLL that requires fixed files, as that requires modifying the
file descriptor. 5.10 should not have any limitations.
--
Jens Axboe
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: SQPOLL question
2020-09-07 12:49 ` Jens Axboe
@ 2020-09-07 14:58 ` Josef
2020-09-07 15:51 ` Jens Axboe
0 siblings, 1 reply; 13+ messages in thread
From: Josef @ 2020-09-07 14:58 UTC (permalink / raw)
To: Jens Axboe, io-uring; +Cc: norman
> Yes, that is known, you cannot open/close descriptors with the
> SQPOLL that requires fixed files, as that requires modifying the
> file descriptor. 5.10 should not have any limitations.
ok I got it, let me know when the implementation/testing is finished
for SQPOLL then I could test my netty implementation
---
Josef
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: SQPOLL question
2020-09-07 14:58 ` Josef
@ 2020-09-07 15:51 ` Jens Axboe
2020-09-08 6:47 ` Josef
0 siblings, 1 reply; 13+ messages in thread
From: Jens Axboe @ 2020-09-07 15:51 UTC (permalink / raw)
To: Josef, io-uring; +Cc: norman
On 9/7/20 8:58 AM, Josef wrote:
>> Yes, that is known, you cannot open/close descriptors with the
>> SQPOLL that requires fixed files, as that requires modifying the
>> file descriptor. 5.10 should not have any limitations.
>
> ok I got it, let me know when the implementation/testing is finished
> for SQPOLL then I could test my netty implementation
If you're up for it, you could just clone my for-5.10/io_uring and base
your SQPOLL testing on that. Should be finished, modulo bugs...
--
Jens Axboe
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: SQPOLL question
2020-09-07 15:51 ` Jens Axboe
@ 2020-09-08 6:47 ` Josef
2020-09-08 14:36 ` Jens Axboe
0 siblings, 1 reply; 13+ messages in thread
From: Josef @ 2020-09-08 6:47 UTC (permalink / raw)
To: io-uring, Jens Axboe; +Cc: norman
> If you're up for it, you could just clone my for-5.10/io_uring and base
> your SQPOLL testing on that. Should be finished, modulo bugs...
yeah did some benchmark tests and I'm quite impressed, however accept
op seems to fail with -EBADF when the flag IOSQE_ASYNC is set, is that
known?
---
Josef
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: SQPOLL question
2020-09-08 6:47 ` Josef
@ 2020-09-08 14:36 ` Jens Axboe
2020-09-08 14:57 ` Jens Axboe
0 siblings, 1 reply; 13+ messages in thread
From: Jens Axboe @ 2020-09-08 14:36 UTC (permalink / raw)
To: Josef, io-uring; +Cc: norman
On 9/8/20 12:47 AM, Josef wrote:
>> If you're up for it, you could just clone my for-5.10/io_uring and base
>> your SQPOLL testing on that. Should be finished, modulo bugs...
>
> yeah did some benchmark tests and I'm quite impressed, however accept
> op seems to fail with -EBADF when the flag IOSQE_ASYNC is set, is that
> known?
Nope, ran a quick test case here on the current tree, works for me.
Are you using for-5.10 and SQEPOLL + ASYNC accept? I'll give that a
test spin.
--
Jens Axboe
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: SQPOLL question
2020-09-08 14:36 ` Jens Axboe
@ 2020-09-08 14:57 ` Jens Axboe
2020-09-08 17:42 ` Josef
0 siblings, 1 reply; 13+ messages in thread
From: Jens Axboe @ 2020-09-08 14:57 UTC (permalink / raw)
To: Josef, io-uring; +Cc: norman
On 9/8/20 8:36 AM, Jens Axboe wrote:
> On 9/8/20 12:47 AM, Josef wrote:
>>> If you're up for it, you could just clone my for-5.10/io_uring and base
>>> your SQPOLL testing on that. Should be finished, modulo bugs...
>>
>> yeah did some benchmark tests and I'm quite impressed, however accept
>> op seems to fail with -EBADF when the flag IOSQE_ASYNC is set, is that
>> known?
>
> Nope, ran a quick test case here on the current tree, works for me.
>
> Are you using for-5.10 and SQEPOLL + ASYNC accept? I'll give that a
> test spin.
This should do it for your testing, need to confirm this is absolutely
safe. But it'll make it work for the 5.10/io_uring setup of allowing
file open/closes.
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 80913973337a..e21a7a9c6a59 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -6757,7 +6757,7 @@ static enum sq_ret __io_sq_thread(struct io_ring_ctx *ctx,
mutex_lock(&ctx->uring_lock);
if (likely(!percpu_ref_is_dying(&ctx->refs)))
- ret = io_submit_sqes(ctx, to_submit, NULL, -1);
+ ret = io_submit_sqes(ctx, to_submit, ctx->ring_file, ctx->ring_fd);
mutex_unlock(&ctx->uring_lock);
if (!io_sqring_full(ctx) && wq_has_sleeper(&ctx->sqo_sq_wait))
@@ -8966,6 +8966,11 @@ static int io_uring_create(unsigned entries, struct io_uring_params *p,
goto err;
}
+ if (p->flags & IORING_SETUP_SQPOLL) {
+ ctx->ring_fd = fd;
+ ctx->ring_file = file;
+ }
+
ret = io_sq_offload_create(ctx, p);
if (ret)
goto err;
--
Jens Axboe
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: SQPOLL question
2020-09-08 14:57 ` Jens Axboe
@ 2020-09-08 17:42 ` Josef
2020-09-08 17:46 ` Jens Axboe
0 siblings, 1 reply; 13+ messages in thread
From: Josef @ 2020-09-08 17:42 UTC (permalink / raw)
To: Jens Axboe, io-uring; +Cc: norman
> Are you using for-5.10 and SQEPOLL + ASYNC accept? I'll give that a
> test spin.
yes exactly
> This should do it for your testing, need to confirm this is absolutely
> safe. But it'll make it work for the 5.10/io_uring setup of allowing
> file open/closes.
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 80913973337a..e21a7a9c6a59 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -6757,7 +6757,7 @@ static enum sq_ret __io_sq_thread(struct io_ring_ctx *ctx,
>
> mutex_lock(&ctx->uring_lock);
> if (likely(!percpu_ref_is_dying(&ctx->refs)))
> - ret = io_submit_sqes(ctx, to_submit, NULL, -1);
> + ret = io_submit_sqes(ctx, to_submit, ctx->ring_file, ctx->ring_fd);
> mutex_unlock(&ctx->uring_lock);
>
> if (!io_sqring_full(ctx) && wq_has_sleeper(&ctx->sqo_sq_wait))
> @@ -8966,6 +8966,11 @@ static int io_uring_create(unsigned entries, struct io_uring_params *p,
> goto err;
> }
>
> + if (p->flags & IORING_SETUP_SQPOLL) {
> + ctx->ring_fd = fd;
> + ctx->ring_file = file;
> + }
> +
> ret = io_sq_offload_create(ctx, p);
> if (ret)
> goto err;
>
sorry I couldn't apply this patch, my last commit is
https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git/commit/?h=for-5.10/io_uring&id=9c2446cffaf55da88e7a9a7c0a5aeb02a9eba2c0
what's your last commit?
it's a small patch, so I'll try it manually :)
---
Josef
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: SQPOLL question
2020-09-08 17:42 ` Josef
@ 2020-09-08 17:46 ` Jens Axboe
0 siblings, 0 replies; 13+ messages in thread
From: Jens Axboe @ 2020-09-08 17:46 UTC (permalink / raw)
To: Josef, io-uring; +Cc: norman
On 9/8/20 11:42 AM, Josef wrote:
>> Are you using for-5.10 and SQEPOLL + ASYNC accept? I'll give that a
>> test spin.
>
> yes exactly
>
>> This should do it for your testing, need to confirm this is absolutely
>> safe. But it'll make it work for the 5.10/io_uring setup of allowing
>> file open/closes.
>>
>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>> index 80913973337a..e21a7a9c6a59 100644
>> --- a/fs/io_uring.c
>> +++ b/fs/io_uring.c
>> @@ -6757,7 +6757,7 @@ static enum sq_ret __io_sq_thread(struct io_ring_ctx *ctx,
>>
>> mutex_lock(&ctx->uring_lock);
>> if (likely(!percpu_ref_is_dying(&ctx->refs)))
>> - ret = io_submit_sqes(ctx, to_submit, NULL, -1);
>> + ret = io_submit_sqes(ctx, to_submit, ctx->ring_file, ctx->ring_fd);
>> mutex_unlock(&ctx->uring_lock);
>>
>> if (!io_sqring_full(ctx) && wq_has_sleeper(&ctx->sqo_sq_wait))
>> @@ -8966,6 +8966,11 @@ static int io_uring_create(unsigned entries, struct io_uring_params *p,
>> goto err;
>> }
>>
>> + if (p->flags & IORING_SETUP_SQPOLL) {
>> + ctx->ring_fd = fd;
>> + ctx->ring_file = file;
>> + }
>> +
>> ret = io_sq_offload_create(ctx, p);
>> if (ret)
>> goto err;
>>
>
> sorry I couldn't apply this patch, my last commit is
> https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git/commit/?h=for-5.10/io_uring&id=9c2446cffaf55da88e7a9a7c0a5aeb02a9eba2c0
> what's your last commit?
>
> it's a small patch, so I'll try it manually :)
Oops sorry, pushed out the queue. Should apply cleanly on top of that.
--
Jens Axboe
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-09-08 17:46 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-06 15:44 SQPOLL question Josef
2020-09-06 15:49 ` Jens Axboe
2020-09-06 16:24 ` Josef
2020-09-06 16:25 ` Jens Axboe
2020-09-07 10:23 ` Josef
2020-09-07 12:49 ` Jens Axboe
2020-09-07 14:58 ` Josef
2020-09-07 15:51 ` Jens Axboe
2020-09-08 6:47 ` Josef
2020-09-08 14:36 ` Jens Axboe
2020-09-08 14:57 ` Jens Axboe
2020-09-08 17:42 ` Josef
2020-09-08 17:46 ` Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox