Re: [PATCH 4/4] sched/core: split iowait state into two states

[Christian Loehle]

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.