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

[Pavel Begunkov <asml.silence@gmail.com>]

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

> 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...

> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 9116bcc90346..dc7a08db8921 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -3790,6 +3790,8 @@ ttwu_do_activate(struct rq *rq, struct task_struct *p, int wake_flags,
>   	if (p->in_iowait) {
>   		delayacct_blkio_end(p);
>   		atomic_dec(&task_rq(p)->nr_iowait);
> +	} else if (p->in_short_wait) {
> +		atomic_dec(&task_rq(p)->nr_short_wait);
>   	}
>   
>   	activate_task(rq, p, en_flags);
> @@ -4356,6 +4358,8 @@ int try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
>   			if (p->in_iowait) {
>   				delayacct_blkio_end(p);
>   				atomic_dec(&task_rq(p)->nr_iowait);
> +			} else if (p->in_short_wait) {
> +				atomic_dec(&task_rq(p)->nr_short_wait);

... which would get this branch folded into the previous one,
which should be more welcome by the sched guys.

-- 
Pavel Begunkov