io_uring/io-wq.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-)
Hi,
this series continues the affinity cleanup work started in
io_uring/sqpoll. It has been tested against the liburing testsuite
(make runtests), whereby the read-mshot test always fails:
Running test read-mshot.t
Buffer ring register failed -22
test_inc 0 0 failed
Test read-mshot.t failed with ret 1
However, this test also fails on a non-patched linux-next @
bc83b4d1f086. The test wq-aff.t succeeds if at least cpu 0,1 are in
the set and fails otherwise. This is expected, as the test wants
to pin on these cpus. I'll send a patch for liburing to skip that test
in case this pre-condition is not met.
Regarding backporting: I would like to backport these patches to 6.1 as
well, as they affect our realtime applications. However, in-between 6.1
and next there is a major change da64d6db3bd3 ("io_uring: One wqe per
wq"), which makes the backport tricky. While I don't think we want to
backport this change, would a dedicated backport of the two pinning
patches for the old multi-queue implementation have a chance to be accepted?
Best regards,
Felix Moessbauer
Siemens AG
Felix Moessbauer (2):
io_uring/io-wq: do not allow pinning outside of cpuset
io_uring/io-wq: limit io poller cpuset to ambient one
io_uring/io-wq.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)
--
2.39.2
On 9/10/24 8:33 AM, Felix Moessbauer wrote:
> Hi,
>
> this series continues the affinity cleanup work started in
> io_uring/sqpoll. It has been tested against the liburing testsuite
> (make runtests), whereby the read-mshot test always fails:
>
> Running test read-mshot.t
> Buffer ring register failed -22
> test_inc 0 0 failed
> Test read-mshot.t failed with ret 1
>
> However, this test also fails on a non-patched linux-next @
> bc83b4d1f086.
That sounds very odd... What liburing are you using? On old kernels
where provided buffer rings aren't available the test should just skip,
new ones it should pass. Only thing I can think of is that your liburing
repo isn't current?
> The test wq-aff.t succeeds if at least cpu 0,1 are in the set and
> fails otherwise. This is expected, as the test wants to pin on these
> cpus. I'll send a patch for liburing to skip that test in case this
> pre-condition is not met.
>
> Regarding backporting: I would like to backport these patches to 6.1 as
> well, as they affect our realtime applications. However, in-between 6.1
> and next there is a major change da64d6db3bd3 ("io_uring: One wqe per
> wq"), which makes the backport tricky. While I don't think we want to
> backport this change, would a dedicated backport of the two pinning
> patches for the old multi-queue implementation have a chance to be accepted?
Let's not backport that patch, just because it's pretty invasive. It's
fine to have a separate backport patch of them for -stable, in this case
we'll have one version for stable kernels new enough to have that
change, and one for older versions. Thankfully not that many to care
about.
--
Jens Axboe
On Tue, 2024-09-10 at 08:53 -0600, Jens Axboe wrote:
> On 9/10/24 8:33 AM, Felix Moessbauer wrote:
> > Hi,
> >
> > this series continues the affinity cleanup work started in
> > io_uring/sqpoll. It has been tested against the liburing testsuite
> > (make runtests), whereby the read-mshot test always fails:
> >
> > Running test read-mshot.t
> > Buffer ring register failed -22
> > test_inc 0 0
> > failed
> >
> > Test read-mshot.t failed with ret 1
> >
> > However, this test also fails on a non-patched linux-next @
> > bc83b4d1f086.
>
> That sounds very odd... What liburing are you using? On old kernels
> where provided buffer rings aren't available the test should just
> skip,
> new ones it should pass. Only thing I can think of is that your
> liburing
> repo isn't current?
Hmm... I tested against
https://github.com/axboe/liburing/commit/74fefa1b51ee35a2014ca6e7667d7c10e9c5b06f
I'll redo the test against the unpatched kernel to be 100% sure that it
is not related to my patches. The -22 is likely an -EINVAL.
>
> > The test wq-aff.t succeeds if at least cpu 0,1 are in the set and
> > fails otherwise. This is expected, as the test wants to pin on
> > these
> > cpus. I'll send a patch for liburing to skip that test in case this
> > pre-condition is not met.
> >
> > Regarding backporting: I would like to backport these patches to
> > 6.1 as
> > well, as they affect our realtime applications. However, in-between
> > 6.1
> > and next there is a major change da64d6db3bd3 ("io_uring: One wqe
> > per
> > wq"), which makes the backport tricky. While I don't think we want
> > to
> > backport this change, would a dedicated backport of the two pinning
> > patches for the old multi-queue implementation have a chance to be
> > accepted?
>
> Let's not backport that patch, just because it's pretty invasive.
> It's
> fine to have a separate backport patch of them for -stable, in this
> case
> we'll have one version for stable kernels new enough to have that
> change, and one for older versions. Thankfully not that many to care
> about.
Ok, that is fine for me. Then let's first get things right in this
series and then I'll send the backport.
Best regards,
Felix
>
--
Siemens AG, Technology
Linux Expert Center
On 9/10/24 9:08 AM, MOESSBAUER, Felix wrote:
> On Tue, 2024-09-10 at 08:53 -0600, Jens Axboe wrote:
>> On 9/10/24 8:33 AM, Felix Moessbauer wrote:
>>> Hi,
>>>
>>> this series continues the affinity cleanup work started in
>>> io_uring/sqpoll. It has been tested against the liburing testsuite
>>> (make runtests), whereby the read-mshot test always fails:
>>>
>>> Running test read-mshot.t
>>> Buffer ring register failed -22
>>> test_inc 0 0
>>> failed
>>>
>>> Test read-mshot.t failed with ret 1
>>>
>>> However, this test also fails on a non-patched linux-next @
>>> bc83b4d1f086.
>>
>> That sounds very odd... What liburing are you using? On old kernels
>> where provided buffer rings aren't available the test should just
>> skip,
>> new ones it should pass. Only thing I can think of is that your
>> liburing
>> repo isn't current?
>
> Hmm... I tested against
> https://github.com/axboe/liburing/commit/74fefa1b51ee35a2014ca6e7667d7c10e9c5b06f
That should certainly be fine.
> I'll redo the test against the unpatched kernel to be 100% sure that it
> is not related to my patches. The -22 is likely an -EINVAL.
I'd be highly surprised if it's related to your patches! Here's what I
get on the current kernel:
axboe@m2max-kvm ~/g/liburing (master)> test/read-mshot.t
axboe@m2max-kvm ~/g/liburing (master)> echo $status
0
and on an older 6.6-stable that doesn't support it:
axboe@m2max-kvm ~/g/liburing (master)> test/read-mshot.t
skip
axboe@m2max-kvm ~/g/liburing (master) [77]> echo $status
77
and then I tried 6.1 since that seems to be your base and get the same
result as 6.6-stable. So not quite sure why it fails on your end, but in
any case, I pushed a commit that I think will sort it for you.
>>> The test wq-aff.t succeeds if at least cpu 0,1 are in the set and
>>> fails otherwise. This is expected, as the test wants to pin on
>>> these
>>> cpus. I'll send a patch for liburing to skip that test in case this
>>> pre-condition is not met.
>>>
>>> Regarding backporting: I would like to backport these patches to
>>> 6.1 as
>>> well, as they affect our realtime applications. However, in-between
>>> 6.1
>>> and next there is a major change da64d6db3bd3 ("io_uring: One wqe
>>> per
>>> wq"), which makes the backport tricky. While I don't think we want
>>> to
>>> backport this change, would a dedicated backport of the two pinning
>>> patches for the old multi-queue implementation have a chance to be
>>> accepted?
>>
>> Let's not backport that patch, just because it's pretty invasive.
>> It's
>> fine to have a separate backport patch of them for -stable, in this
>> case
>> we'll have one version for stable kernels new enough to have that
>> change, and one for older versions. Thankfully not that many to care
>> about.
>
> Ok, that is fine for me. Then let's first get things right in this
> series and then I'll send the backport.
Exactly, that's the plan. Thanks!
--
Jens Axboe
On Tue, 2024-09-10 at 09:17 -0600, Jens Axboe wrote: > On 9/10/24 9:08 AM, MOESSBAUER, Felix wrote: > > On Tue, 2024-09-10 at 08:53 -0600, Jens Axboe wrote: > > > On 9/10/24 8:33 AM, Felix Moessbauer wrote: > > > > Hi, > > > > > > > > this series continues the affinity cleanup work started in > > > > io_uring/sqpoll. It has been tested against the liburing > > > > testsuite > > > > (make runtests), whereby the read-mshot test always fails: > > > > > > > > Running test read-mshot.t > > > > Buffer ring register failed -22 > > > > test_inc 0 0 > > > > failed > > > > > > > > > > > > Test read-mshot.t failed with ret 1 > > > > > > > > However, this test also fails on a non-patched linux-next @ > > > > bc83b4d1f086. > > > > > > That sounds very odd... What liburing are you using? On old > > > kernels > > > where provided buffer rings aren't available the test should just > > > skip, > > > new ones it should pass. Only thing I can think of is that your > > > liburing > > > repo isn't current? > > > > Hmm... I tested against > > https://github.com/axboe/liburing/commit/74fefa1b51ee35a2014ca6e7667d7c10e9c5b06f > > That should certainly be fine. > > > I'll redo the test against the unpatched kernel to be 100% sure > > that it > > is not related to my patches. The -22 is likely an -EINVAL. > > I'd be highly surprised if it's related to your patches! Here's what > I > get on the current kernel: > > axboe@m2max-kvm ~/g/liburing (master)> test/read-mshot.t > axboe@m2max-kvm ~/g/liburing (master)> echo $status Without your patches for liburing, this test definitely fails on linux- next @ bc83b4d1f086 (in qemu). Same error as above. Some more information: $ uname -a Linux test-iou 6.11.0-rc7 #1 SMP PREEMPT_DYNAMIC Thu, 01 Jan 1970 01:00:00 +0000 x86_64 GNU/Linux Strange... > 0 > > and on an older 6.6-stable that doesn't support it: > > axboe@m2max-kvm ~/g/liburing (master)> test/read-mshot.t > skip > axboe@m2max-kvm ~/g/liburing (master) [77]> echo $status > 77 > > and then I tried 6.1 since that seems to be your base and get the > same > result as 6.6-stable. So not quite sure why it fails on your end, but > in > any case, I pushed a commit that I think will sort it for you. With liburing@3505047a35df and my kernel patches, all tests pass. By that, I assume my patches themselves are fine. I'll just update the commit messages to fix the oddities and send a functionally identical v2. Felix -- Siemens AG, Technology Linux Expert Center
On 9/10/24 9:37 AM, MOESSBAUER, Felix wrote: > On Tue, 2024-09-10 at 09:17 -0600, Jens Axboe wrote: >> On 9/10/24 9:08 AM, MOESSBAUER, Felix wrote: >>> On Tue, 2024-09-10 at 08:53 -0600, Jens Axboe wrote: >>>> On 9/10/24 8:33 AM, Felix Moessbauer wrote: >>>>> Hi, >>>>> >>>>> this series continues the affinity cleanup work started in >>>>> io_uring/sqpoll. It has been tested against the liburing >>>>> testsuite >>>>> (make runtests), whereby the read-mshot test always fails: >>>>> >>>>> Running test read-mshot.t >>>>> Buffer ring register failed -22 >>>>> test_inc 0 0 >>>>> failed >>>>> >>>>> >>>>> Test read-mshot.t failed with ret 1 >>>>> >>>>> However, this test also fails on a non-patched linux-next @ >>>>> bc83b4d1f086. >>>> >>>> That sounds very odd... What liburing are you using? On old >>>> kernels >>>> where provided buffer rings aren't available the test should just >>>> skip, >>>> new ones it should pass. Only thing I can think of is that your >>>> liburing >>>> repo isn't current? >>> >>> Hmm... I tested against >>> https://github.com/axboe/liburing/commit/74fefa1b51ee35a2014ca6e7667d7c10e9c5b06f >> >> That should certainly be fine. >> >>> I'll redo the test against the unpatched kernel to be 100% sure >>> that it >>> is not related to my patches. The -22 is likely an -EINVAL. >> >> I'd be highly surprised if it's related to your patches! Here's what >> I >> get on the current kernel: >> >> axboe@m2max-kvm ~/g/liburing (master)> test/read-mshot.t >> axboe@m2max-kvm ~/g/liburing (master)> echo $status > > Without your patches for liburing, this test definitely fails on linux- > next @ bc83b4d1f086 (in qemu). Same error as above. Some more > information: > $ uname -a > Linux test-iou 6.11.0-rc7 #1 SMP PREEMPT_DYNAMIC Thu, 01 Jan 1970 > 01:00:00 +0000 x86_64 GNU/Linux > > Strange... It could just be that I never tested that version on a kernel that has support for ring provided buffers, but not for incrementally consumed ones. Though that should be in -next for a while now, so even that doesn't make sense... Oh well, should work now. > By that, I assume my patches themselves are fine. I'll just update the > commit messages to fix the oddities and send a functionally identical > v2. Sounds good, thanks. -- Jens Axboe
© 2016 - 2024 Red Hat, Inc.