Re: [PATCH v2] io_uring: enable toggle of iowait usage when waiting on CQEs

[Jens Axboe <axboe@kernel.dk>]

On 3/18/25 2:07 PM, Pavel Begunkov wrote:
> On 3/18/25 18:36, Jens Axboe wrote:
>> On 3/18/25 12:39 AM, Pavel Begunkov wrote:
>>> On 3/17/25 14:07, Jens Axboe wrote:
>>>> On 3/16/25 12:57 AM, Pavel Begunkov wrote:
>>>>> On 3/14/25 18:48, Jens Axboe wrote:
>>>>>> By default, io_uring marks a waiting task as being in iowait, if it's
>>>>>> sleeping waiting on events and there are pending requests. This isn't
>>>>>> necessarily always useful, and may be confusing on non-storage setups
>>>>>> where iowait isn't expected. It can also cause extra power usage, by
>>>>>
>>>>> I think this passage hints on controlling iowait stats, and in my opinion
>>>>> we shouldn't conflate stats and optimisations. Global iowait stats
>>>>> is there to stay, but ideally we want to never account io_uring as iowait.
>>>>> That's while there were talks about removing optimisation toggle at all
>>>>> (and do it as internal cpufreq magic, I suppose).
>>>>>
>>>>> How about posing it as an optimisation option only and that iowait stat
>>>>> is a side effect that can change. Explicitly spelling that in the commit
>>>>> message and in a comment on top of the flag in an attempt to avoid the
>>>>> uapi regression trap. We'd also need it in the option's man when it's
>>>>> written. And I'd also add "hint" to the flag name, like
>>>>> IORING_ENTER_HINT_NO_IOWAIT, as we might need to nop it if anything
>>>>> changes on the cpufreq side.
>>>>
>>>> Having potentially the control of both would be useful, the stat
>>>
>>> It's not the right place to control the stat accounting though,
>>> apps don't care about iowait, it's usually monitored by a different
>>> entity / person from outside the app, so responsibilities don't
>>> match. It's fine if you fully control the stack, but just imagine
>>
>> Sometimes those are one and the same thing, though - there's just the
>> one application running. That's not uncommon in data centers.
> 
> Yep, but that's only a subset, and for others the very fact of the
> feature existence creates a mess, which might be fine or not.

But at least with an opt-out flag, if you don't care, you never need to
worry about it.

>>> a bunch of apps using different frameworks with io_uring inside
>>> that make different choices about it. The final iowait reading
>>> would be just a mess. With this patch at least we can say it's
>>> an unfortunate side effect.
>>> If we can separately control the accounting, a sysctl knob would
>>> probably be better, i.e. to be set globally from outside of an
>>> app, but I don't think we care enough to add extra logic / overhead
>>> for handling it.
>>
>> That's not a bad idea, maybe we just do that for starters? We can always
> 
> Do we really want it though? What are you trying to achieve, fixing
> the iowait stat problem or providing an optimisation option? Because
> as I see it, what's good for one is bad for the other, unfortunately.
> A sysctl is not a great option as an optimisation, because with that
> all apps in the system has either to be storage or net to be optimal
> in relation to iowait / power consumption. That one you won't even
> be able to use in a good number of server setups while getting
> optimal power consumption, even if you own the entire stack.
> 
> It sounds to me like the best option is to choose which one we want
> to solve at the moment. Global / sysctl option for the stat, but I'm
> not sure it's that important atm, people complain less nowadays
> as well. Enter flag goes fine for the iowait optimisation, but
> messes with the stat. IMHO, that should be fine if we're clear
> about it and that the stat part of it can change. That's what
> I'd suggest doing.
> 
> The third option is to try to solve them both, but seems your
> patches got buried in a discussion, and working it around at
> io_uring side doesn't sound pretty, like two flags you
> mentioned.

I'm not digging into that again. Once those guys figure out what they
want, we can address it on our side.

> Another option is to just have v2 and tell that the optimisation
> and the accounting is the same, having some mess on the stat
> side, and deal with the consequences when the in-kernel semantics
> changes.

After thinking about it a bit more, I do think v2 is the best approach.
And the name is probably fine, IORING_ENTER_NO_IOWAIT. If we at some
point end up having the ability to control boost and stats separately,
we could add IORING_ENTER_IOWAIT_BOOST or something. That'll allow you
to control both separately.

What do you think? I do want to get this sorted for 6.15, I feel like
we've been ignoring or stalling on this issue for way too long.

-- 
Jens Axboe