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
