* 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