[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <29b6f42f-6917-4584-a53a-4711754c6fcf@arm.com>
Date: Tue, 16 Apr 2024 15:25:26 +0100
From: Christian Loehle <christian.loehle@....com>
To: Jens Axboe <axboe@...nel.dk>, linux-kernel@...r.kernel.org
Cc: peterz@...radead.org, tglx@...utronix.de
Subject: Re: [PATCH 4/4] sched/core: split iowait state into two states
On 16/04/2024 15:10, Christian Loehle wrote:
>>[snip]
>> + atomic_long_add(val, &task_rq(p)->nr_iowait);
>> #else
>> - atomic_inc(&task_rq(p)->nr_iowait);
>> + int val = 1 + ((int) p->in_iowait_acct << 16);
>> + atomic_add(val, &task_rq(p)->nr_iowait);
>> #endif
>> }
>>
>> static void task_iowait_dec(struct task_struct *p)
>> {
>> #ifdef CONFIG_64BIT
>> - atomic_long_dec(&task_rq(p)->nr_iowait);
>> + long val = 1 + ((long) p->in_iowait_acct << 32);
>> + atomic_long_sub(val, &task_rq(p)->nr_iowait);
>> #else
>> - atomic_dec(&task_rq(p)->nr_iowait);
>> + int val = 1 + ((int) p->in_iowait_acct << 16);
>> + atomic_sub(val, &task_rq(p)->nr_iowait);
>> #endif
>> }
>>
>> int rq_iowait(struct rq *rq)
>> {
>> #ifdef CONFIG_64BIT
>> - return atomic_long_read(&rq->nr_iowait);
>> + return atomic_long_read(&rq->nr_iowait) & ((1UL << 32) - 1);
>> +#else
>> + return atomic_read(&rq->nr_iowait) & ((1U << 16) - 1);
>> +#endif
>> +}
>> +
>> +int rq_iowait_acct(struct rq *rq)
>> +{
>> +#ifdef CONFIG_64BIT
>> + return atomic_long_read(&rq->nr_iowait) >> 32;
>> #else
>> - return atomic_read(&rq->nr_iowait);
>> + return atomic_read(&rq->nr_iowait) >> 16;
>> #endif
>> }
>>
>> @@ -5497,7 +5514,12 @@ unsigned long long nr_context_switches(void)
>> * it does become runnable.
>> */
>>
>> -unsigned int nr_iowait_cpu(int cpu)
>> +unsigned int nr_iowait_acct_cpu(int cpu)
>
> There is a comment above here by Daniel referring to "these two
> interfaces [...]", originally referring to nr_iowait_cpu() and
> get_iowait_load().
> get_iowait_load() was removed in commit a7fe5190c03f ("cpuidle: menu: Remove get_loadavg() from the performance multiplier")
> but nr_iowait_cpu() remains, so the comment should remain above it.
> Rewording is also way overdue, although that's hardly on your
> patch ;)
>
FWIW, but also feel free to reword yourself.
>From ba66fe24c64f5615b9820b0fe24857ecbf18fc5f Mon Sep 17 00:00:00 2001
From: Christian Loehle <christian.loehle@....com>
Date: Tue, 16 Apr 2024 15:20:22 +0100
Subject: [PATCH] sched/core: Reword comment about iowait interface
The original comment referenced an interface which was removed in
the meantime by commit a7fe5190c03f ("cpuidle: menu: Remove
get_loadavg() from the performance multiplier").
Furthermore the function has moved so move the comment accordingly.
Signed-off-by: Christian Loehle <christian.loehle@....com>
---
kernel/sched/core.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index d52d3118dd73..bb10b9bc37d6 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5507,18 +5507,19 @@ unsigned long long nr_context_switches(void)
return sum;
}
-/*
- * Consumers of these two interfaces, like for example the cpuidle menu
- * governor, are using nonsensical data. Preferring shallow idle state selection
- * for a CPU that has IO-wait which might not even end up running the task when
- * it does become runnable.
- */
unsigned int nr_iowait_acct_cpu(int cpu)
{
return rq_iowait_acct(cpu_rq(cpu));
}
+/*
+ * Consumers of this interface like the cpuidle menu governor are using
+ * nonsensical data. Preferring shallow idle state selection for a CPU that
+ * has IO-wait which might not even end up being selected for the task again
+ * when it does become runnable.
+ */
+
unsigned nr_iowait_cpu(int cpu)
{
return rq_iowait(cpu_rq(cpu));
--
2.34.1
Powered by blists - more mailing lists