[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9cd8bae3-ba32-4b44-a4c0-63f5e5a3de35@kernel.dk>
Date: Tue, 10 Sep 2024 09:17:00 -0600
From: Jens Axboe <axboe@...nel.dk>
To: "MOESSBAUER, Felix" <felix.moessbauer@...mens.com>
Cc: "io-uring@...r.kernel.org" <io-uring@...r.kernel.org>,
"cgroups@...r.kernel.org" <cgroups@...r.kernel.org>,
"Bezdeka, Florian" <florian.bezdeka@...mens.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"longman@...hat.com" <longman@...hat.com>,
"asml.silence@...il.com" <asml.silence@...il.com>,
"Schmidt, Adriaan" <adriaan.schmidt@...mens.com>,
"dqminh@...udflare.com" <dqminh@...udflare.com>
Subject: Re: [PATCH 0/2] io_uring/io-wq: respect cgroup cpusets
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@...ax-kvm ~/g/liburing (master)> test/read-mshot.t
axboe@...ax-kvm ~/g/liburing (master)> echo $status
0
and on an older 6.6-stable that doesn't support it:
axboe@...ax-kvm ~/g/liburing (master)> test/read-mshot.t
skip
axboe@...ax-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
Powered by blists - more mailing lists