public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@kernel.dk>
To: Peter Zijlstra <peterz@infradead.org>
Cc: linux-kernel@vger.kernel.org, tglx@linutronix.de,
	rafael@kernel.org, daniel.lezcano@linaro.org,
	linux-pm@vger.kernel.org
Subject: Re: [PATCHSET v6 0/4] Split iowait into two states
Date: Wed, 4 Sep 2024 08:41:23 -0600	[thread overview]
Message-ID: <30eeae06-0d8a-4968-ba57-d723162a0782@kernel.dk> (raw)
In-Reply-To: <20240904142841.GL4723@noisy.programming.kicks-ass.net>

On 9/4/24 8:28 AM, Peter Zijlstra wrote:
> On Mon, Aug 19, 2024 at 09:39:45AM -0600, Jens Axboe wrote:
>> Hi,
>>
>> This is v6 of the patchset where the current in_iowait state is split
>> into two parts:
>>
>> 1) The "task is sleeping waiting on IO", and would like cpufreq goodness
>>    in terms of sleep and wakeup latencies.
>> 2) The above, and also accounted as such in the iowait stats.
>>
>> The current ->in_iowait covers both, this series splits it into two types
>> of state so that each can be controlled seperately.
> 
> Yeah, but *WHY* !?!? I have some vague memories from last time around,
> but patches should really keep this information.

To decouple the frequency boost on short waits from the accounting side,
as lots of tooling equates iowait time with busy time and reports it as
such. Yeah that's garbage and a reporting issue, but decades of
education hasn't really improved on that. We should've dumped iowait
once we moved away from 1-2 processor system or had preemptible kernels,
but alas we did not and here we are in 2024.

>> Patches 1..3 are prep patches, changing the type of
>> task_struct->nr_iowait and adding helpers to manipulate the iowait counts.
>>
>> Patch 4 does the actual splitting.
>>
>> This has been sitting for a while, would be nice to get this queued up
>> for 6.12. Comments welcome!
> 
> Ufff, and all this because menu-governor does something insane :-(
> 
> Rafael, why can't we simply remove this from menu? All the nr_iowait*()
> users are basically broken and I would much rather fix broken rather
> than work around broken like this.
> 
> That is, from where I'm sitting this all makes the io-wait situation far
> worse instead of better.

IMHO what we need is a way to propagate expected wait times for a
sleeper. Right now iowait serves this purpose in a very crude way, in
that it doesn't really tell you the expected wait, just that it's a
short one.

If we simply remove iowait frequency boosting, then we'll have big
regressions particularly for low/sync storage IO.

-- 
Jens Axboe


  reply	other threads:[~2024-09-04 14:41 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-19 15:39 [PATCHSET v6 0/4] Split iowait into two states Jens Axboe
2024-08-19 15:39 ` [PATCH 1/4] sched/core: add helpers for iowait handling Jens Axboe
2024-08-19 15:39 ` [PATCH 2/4] sched/core: change rq->nr_iowait type to atomic_long_t Jens Axboe
2024-08-20  2:14   ` Zhang Qiao
2024-08-19 15:39 ` [PATCH 3/4] sched/core: have io_schedule_prepare() return a long Jens Axboe
2024-09-05  9:57   ` Peter Zijlstra
2024-08-19 15:39 ` [PATCH 4/4] sched/core: split iowait state into two states Jens Axboe
2024-09-05 10:55   ` Peter Zijlstra
2024-08-21 14:54 ` [PATCHSET v6 0/4] Split iowait " Christian Loehle
2024-08-21 15:04   ` Jens Axboe
2024-08-21 15:57     ` Christian Loehle
2024-08-24 15:34       ` Jens Axboe
2024-09-04 14:28 ` Peter Zijlstra
2024-09-04 14:41   ` Jens Axboe [this message]
2024-09-04 14:49     ` Jens Axboe
2024-09-05  9:51     ` Peter Zijlstra
2024-09-04 14:42   ` Rafael J. Wysocki
2024-09-04 15:18     ` Rafael J. Wysocki
2024-09-05  9:29       ` Christian Loehle
2024-09-05 10:40         ` Rafael J. Wysocki
2024-09-05  9:36       ` Peter Zijlstra
2024-09-05 10:31         ` Christian Loehle
2024-09-05 11:00           ` Peter Zijlstra
2024-09-05 11:09             ` Christian Loehle
2025-03-31  9:02 ` Pavel Begunkov
2025-03-31 10:33   ` Christian Loehle
2025-04-01  8:21     ` Pavel Begunkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=30eeae06-0d8a-4968-ba57-d723162a0782@kernel.dk \
    --to=axboe@kernel.dk \
    --cc=daniel.lezcano@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=rafael@kernel.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox