[PATCH v1 0/3] correct printing for rq->nr_uninterruptible and some updates

[PATCH v1 0/3] correct printing for rq->nr_uninterruptible and some updates

[晏艳(采苓)]

The patch 1/3 coverts rq->nr_uninterruptible type from unsigned int to int
that prints negative numbers correctly. The rest patches are some little updates,
including updating the descriptions of commments, removing duplicate included headers.

Yan Yan (3):
  sched/debug: use int type and fix wrong print for
    rq->nr_uninterruptible
  sched/debug: update description of print
  sched: remove duplicate included headers psi.h

 kernel/sched/build_utility.c | 1 -
 kernel/sched/debug.c         | 4 ++--
 kernel/sched/loadavg.c       | 2 +-
 kernel/sched/sched.h         | 2 +-
 4 files changed, 4 insertions(+), 5 deletions(-)

-- 
2.32.0.3.g01195cf9f

[PATCH v1 1/3] sched/debug: use int type and fix wrong print for rq->nr_uninterruptible

[晏艳(采苓)]

commit e6fe3f422be1 ("sched: Make multiple runqueue task counters
32-bit") changed rq->nr_uninterruptible from 'unsigned long'
to 'unsigned int', but left the wrong print to
/sys/kernel/debug/sched/debug and to the console.

For example:
Current type is 'unsigned int' and value is fffffff7, and the print
will run the sentences,
"do {                                    \
    if (sizeof(rq->x) == 4)                     \
        SEQ_printf(m, "  .%-30s: %ld\n", #x, (long)(rq->x));    \
    else                                \
        SEQ_printf(m, "  .%-30s: %Ld\n", #x, (long long)(rq->x));\
} while (0)"

The result will be 4294967287 on 64-bit machines to print (long)(rq->x)
while old type 'unsigned long' will print -9.

And the other places that use its value will cast to int to return
expected output, so we convert its type from 'unsigned int' to int.

Signed-off-by: Yan Yan <yanyan.yan@antgroup.com>
---
 kernel/sched/loadavg.c | 2 +-
 kernel/sched/sched.h   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/loadavg.c b/kernel/sched/loadavg.c
index 52c8f8226b0d..b9867495fe8b 100644
--- a/kernel/sched/loadavg.c
+++ b/kernel/sched/loadavg.c
@@ -80,7 +80,7 @@ long calc_load_fold_active(struct rq *this_rq, long adjust)
 	long nr_active, delta = 0;
 
 	nr_active = this_rq->nr_running - adjust;
-	nr_active += (int)this_rq->nr_uninterruptible;
+	nr_active += this_rq->nr_uninterruptible;
 
 	if (nr_active != this_rq->calc_load_active) {
 		delta = nr_active - this_rq->calc_load_active;
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 060616944d7a..23c643948331 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1006,7 +1006,7 @@ struct rq {
 	 * one CPU and if it got migrated afterwards it may decrease
 	 * it on another CPU. Always updated under the runqueue lock:
 	 */
-	unsigned int		nr_uninterruptible;
+	int			nr_uninterruptible;
 
 	struct task_struct __rcu	*curr;
 	struct task_struct	*idle;
-- 
2.32.0.3.g01195cf9f

[PATCH v1 2/3] sched/debug: update description of print

[晏艳(采苓)]

commit d27e9ae2f244 ("sched: Move /proc/sched_debug to debugfs")
moved /proc/sched_debug to /sys/kernel/debug/sched/debug without
updating the description. Here update it.

Signed-off-by: Yan Yan <yanyan.yan@antgroup.com>
---
 kernel/sched/debug.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index 1637b65ba07a..c484c02f740b 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -8,8 +8,8 @@
  */
 
 /*
- * This allows printing both to /proc/sched_debug and
- * to the console
+ * This allows printing both to /sys/kernel/debug/sched/debug
+ * and to the console
  */
 #define SEQ_printf(m, x...)			\
  do {						\
-- 
2.32.0.3.g01195cf9f

[PATCH v1 3/3] sched: remove duplicate included headers psi.h

[晏艳(采苓)]

Signed-off-by: Yan Yan <yanyan.yan@antgroup.com>
---
 kernel/sched/build_utility.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kernel/sched/build_utility.c b/kernel/sched/build_utility.c
index 99bdd96f454f..80a3df49ab47 100644
--- a/kernel/sched/build_utility.c
+++ b/kernel/sched/build_utility.c
@@ -34,7 +34,6 @@
 #include <linux/nospec.h>
 #include <linux/proc_fs.h>
 #include <linux/psi.h>
-#include <linux/psi.h>
 #include <linux/ptrace_api.h>
 #include <linux/sched_clock.h>
 #include <linux/security.h>
-- 
2.32.0.3.g01195cf9f

Re: [PATCH v1 1/3] sched/debug: use int type and fix wrong print for rq->nr_uninterruptible

[晏艳(采苓)]

在 2023/4/19 15:46, 晏艳 写道:
>
>
> 在 2023/4/12 11:07, 晏艳(采苓) 写道:
>> commit e6fe3f422be1 ("sched: Make multiple runqueue task counters
>> 32-bit") changed rq->nr_uninterruptible from 'unsigned long'
>> to 'unsigned int', but left the wrong print to
>> /sys/kernel/debug/sched/debug and to the console.
>>
>> For example:
>> Current type is 'unsigned int' and value is fffffff7, and the print
>> will run the sentences,
>> "do {                                    \
>>      if (sizeof(rq->x) == 4)                     \
>>          SEQ_printf(m, "  .%-30s: %ld\n", #x, (long)(rq->x));    \
>>      else                                \
>>          SEQ_printf(m, "  .%-30s: %Ld\n", #x, (long long)(rq->x));\
>> } while (0)"
>>
>> The result will be 4294967287 on 64-bit machines to print (long)(rq->x)
>> while old type 'unsigned long' will print -9.
>>
>> And the other places that use its value will cast to int to return
>> expected output, so we convert its type from 'unsigned int' to int.
>>
>> Signed-off-by: Yan Yan<yanyan.yan@antgroup.com>
>> ---
>>   kernel/sched/loadavg.c | 2 +-
>>   kernel/sched/sched.h   | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
> Ping, any comment?
>
> thanks,
> -Yan Yan

Ping, any comment?

thanks,

-Yan Yan

>> diff --git a/kernel/sched/loadavg.c b/kernel/sched/loadavg.c
>> index 52c8f8226b0d..b9867495fe8b 100644
>> --- a/kernel/sched/loadavg.c
>> +++ b/kernel/sched/loadavg.c
>> @@ -80,7 +80,7 @@ long calc_load_fold_active(struct rq *this_rq, long adjust)
>>   	long nr_active, delta = 0;
>>   
>>   	nr_active = this_rq->nr_running - adjust;
>> -	nr_active += (int)this_rq->nr_uninterruptible;
>> +	nr_active += this_rq->nr_uninterruptible;
>>   
>>   	if (nr_active != this_rq->calc_load_active) {
>>   		delta = nr_active - this_rq->calc_load_active;
>> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
>> index 060616944d7a..23c643948331 100644
>> --- a/kernel/sched/sched.h
>> +++ b/kernel/sched/sched.h
>> @@ -1006,7 +1006,7 @@ struct rq {
>>   	 * one CPU and if it got migrated afterwards it may decrease
>>   	 * it on another CPU. Always updated under the runqueue lock:
>>   	 */
>> -	unsigned int		nr_uninterruptible;
>> +	int			nr_uninterruptible;
>>   
>>   	struct task_struct __rcu	*curr;
>>   	struct task_struct	*idle;

Re: [PATCH v1 1/3] sched/debug: use int type and fix wrong print for rq->nr_uninterruptible

[Peter Zijlstra]

On Wed, Apr 12, 2023 at 11:07:29AM +0800, 晏艳(采苓) wrote:
> commit e6fe3f422be1 ("sched: Make multiple runqueue task counters
> 32-bit") changed rq->nr_uninterruptible from 'unsigned long'
> to 'unsigned int', but left the wrong print to
> /sys/kernel/debug/sched/debug and to the console.
> 
> For example:
> Current type is 'unsigned int' and value is fffffff7, and the print
> will run the sentences,
> "do {                                    \
>     if (sizeof(rq->x) == 4)                     \
>         SEQ_printf(m, "  .%-30s: %ld\n", #x, (long)(rq->x));    \

Arguably this is wrong, because LP64 has sizeof(long) != 4. This ought
to be something like:

+               SEQ_printf(m, "  .%-30s: %d\n", #x, (int)(rq->x));      \

>     else                                \
>         SEQ_printf(m, "  .%-30s: %Ld\n", #x, (long long)(rq->x));\
> } while (0)"
> 
> The result will be 4294967287 on 64-bit machines to print (long)(rq->x)
> while old type 'unsigned long' will print -9.
> 
> And the other places that use its value will cast to int to return
> expected output, so we convert its type from 'unsigned int' to int.
> 
> Signed-off-by: Yan Yan <yanyan.yan@antgroup.com>
> ---
>  kernel/sched/loadavg.c | 2 +-
>  kernel/sched/sched.h   | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/sched/loadavg.c b/kernel/sched/loadavg.c
> index 52c8f8226b0d..b9867495fe8b 100644
> --- a/kernel/sched/loadavg.c
> +++ b/kernel/sched/loadavg.c
> @@ -80,7 +80,7 @@ long calc_load_fold_active(struct rq *this_rq, long adjust)
>  	long nr_active, delta = 0;
>  
>  	nr_active = this_rq->nr_running - adjust;
> -	nr_active += (int)this_rq->nr_uninterruptible;
> +	nr_active += this_rq->nr_uninterruptible;
>  
>  	if (nr_active != this_rq->calc_load_active) {
>  		delta = nr_active - this_rq->calc_load_active;
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 060616944d7a..23c643948331 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -1006,7 +1006,7 @@ struct rq {
>  	 * one CPU and if it got migrated afterwards it may decrease
>  	 * it on another CPU. Always updated under the runqueue lock:
>  	 */
> -	unsigned int		nr_uninterruptible;
> +	int			nr_uninterruptible;

Bit meh on this; but I suppose since calc_load_fold_active() already had
an explicit cast, it won't make the 'but signed overflow is UB' crowd
more unhappy.