Re: [PATCH v1 1/4] io_uring: only account cqring wait time as iowait if enabled for a ring

[Jens Axboe <axboe@kernel.dk>]

On 2/26/24 7:56 AM, Pavel Begunkov wrote:
> On 2/25/24 21:11, Jens Axboe wrote:
>> On 2/25/24 9:43 AM, Jens Axboe wrote:
>>> If you are motivated, please dig into it. If not, I guess I will take a
>>> look this week.
> 
> I tried to split the atomic as mentioned, but I don't think anybody
> cares, it was 0.1% in perf, so wouldn't even be benchmarkeable,
> and it's iowait only patch anyway. If anything you'd need to read
> two vars every tick now, so nevermind

Agree, I did ponder that too, but seems not worth it at all.

>> The straight forward approach - add a nr_short_wait and ->in_short_wait
>> and ensure that the idle governor factors that in. Not sure how
>> palatable it is, would be nice fold iowait under this, but doesn't
>> really work with how we pass back the previous state.
> 
> It might look nicer if instead adding nr_short_waiters you'd
> do nr_iowait_account for the iowait% and leave nr_iowait
> for cpufreq.
> 
> The block iowaiting / io_schedule / etc. would need to set
> both flags...

That's what I meant with the nesting too, but then we need to return
flags from eg io_schedule_prepare(). Not a big issue as I think that's
the only spot, and we can even just keep the type the same. Callers
should treat it as a cookie/token anyway.

I'll make that change.

-- 
Jens Axboe