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
