[PATCH 0/2] Small cleanup around sparse report

[PATCH 0/2] Small cleanup around sparse report

[Pierre Gondois]

While checking spare's tool report, some RCU pointers appeared
to be not annotated. In the same effort, idle_cpu_without() seemed
to be subject to a small optimization.

Pierre Gondois (2):
  sched/topology: Annotate RCU pointers properly
  sched/fair: Use rq in idle_cpu_without()

 kernel/sched/fair.c     | 8 +++-----
 kernel/sched/topology.c | 4 ++--
 2 files changed, 5 insertions(+), 7 deletions(-)

-- 
2.25.1

[PATCH 1/2] sched/topology: Annotate RCU pointers properly

[Pierre Gondois]

Cleanup RCU-related spare errors by annotating RCU pointers.

sched_domains_numa_distance:
  error: incompatible types in comparison expression
  (different address spaces):
      int [noderef] __rcu *
      int *

sched_domains_numa_masks:
  error: incompatible types in comparison expression
  (different address spaces):
      struct cpumask **[noderef] __rcu *
      struct cpumask ***

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 kernel/sched/topology.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 10d1391e7416..0342a4f41f09 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -1542,8 +1542,8 @@ static int			sched_domains_numa_levels;
 static int			sched_domains_curr_level;
 
 int				sched_max_numa_distance;
-static int			*sched_domains_numa_distance;
-static struct cpumask		***sched_domains_numa_masks;
+static int			__rcu *sched_domains_numa_distance;
+static struct cpumask		** __rcu *sched_domains_numa_masks;
 #endif
 
 /*
-- 
2.25.1

[PATCH 2/2] sched/fair: Use rq in idle_cpu_without()

[Pierre Gondois]

idle_cpu_without() could receive a 'struct rq' instead of a
cpu number to avoid converting the cpu number to a 'struct rq'
two times. Indeed update_sg_wakeup_stats() already makes the
conversion.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 kernel/sched/fair.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 93e928e76959..d38fec26fd3d 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -10184,15 +10184,13 @@ static unsigned int task_running_on_cpu(int cpu, struct task_struct *p)
 
 /**
  * idle_cpu_without - would a given CPU be idle without p ?
- * @cpu: the processor on which idleness is tested.
+ * @rq: the rq on which idleness is tested.
  * @p: task which should be ignored.
  *
  * Return: 1 if the CPU would be idle. 0 otherwise.
  */
-static int idle_cpu_without(int cpu, struct task_struct *p)
+static int idle_cpu_without(struct rq *rq, struct task_struct *p)
 {
-	struct rq *rq = cpu_rq(cpu);
-
 	if (rq->curr != rq->idle && rq->curr != p)
 		return 0;
 
@@ -10247,7 +10245,7 @@ static inline void update_sg_wakeup_stats(struct sched_domain *sd,
 		/*
 		 * No need to call idle_cpu_without() if nr_running is not 0
 		 */
-		if (!nr_running && idle_cpu_without(i, p))
+		if (!nr_running && idle_cpu_without(rq, p))
 			sgs->idle_cpus++;
 
 		/* Check if task fits in the CPU */
-- 
2.25.1

Re: [PATCH 2/2] sched/fair: Use rq in idle_cpu_without()

[Shrikanth Hegde]

On 1/3/24 6:26 PM, Pierre Gondois wrote:
> idle_cpu_without() could receive a 'struct rq' instead of a
> cpu number to avoid converting the cpu number to a 'struct rq'

nit: s/cpu/CPU

> two times. Indeed update_sg_wakeup_stats() already makes the
> conversion.

This change looks good. There maybe other candidates which might get simplified 
as well. for example, update_blocked_averages. (and then there are some 
like balance_push_set which maybe borderline when it comes to such simplification)

> 
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>  kernel/sched/fair.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 93e928e76959..d38fec26fd3d 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -10184,15 +10184,13 @@ static unsigned int task_running_on_cpu(int cpu, struct task_struct *p)
>  
>  /**
>   * idle_cpu_without - would a given CPU be idle without p ?
> - * @cpu: the processor on which idleness is tested.
> + * @rq: the rq on which idleness is tested.
>   * @p: task which should be ignored.
>   *
>   * Return: 1 if the CPU would be idle. 0 otherwise.
>   */
> -static int idle_cpu_without(int cpu, struct task_struct *p)
> +static int idle_cpu_without(struct rq *rq, struct task_struct *p)

This might need change in the function name too. perception here is that, is the 
CPU idle without task p. 
Otherwise LGTM. 

Reviewed-by: Shrikanth Hegde <sshegde@linux.vnet.ibm.com>

>  {
> -	struct rq *rq = cpu_rq(cpu);
> -
>  	if (rq->curr != rq->idle && rq->curr != p)
>  		return 0;
>  
> @@ -10247,7 +10245,7 @@ static inline void update_sg_wakeup_stats(struct sched_domain *sd,
>  		/*
>  		 * No need to call idle_cpu_without() if nr_running is not 0
>  		 */
> -		if (!nr_running && idle_cpu_without(i, p))
> +		if (!nr_running && idle_cpu_without(rq, p))
>  			sgs->idle_cpus++;
>  
>  		/* Check if task fits in the CPU */

Re: [PATCH 1/2] sched/topology: Annotate RCU pointers properly

[Valentin Schneider]

On 03/01/24 13:56, Pierre Gondois wrote:
> Cleanup RCU-related spare errors by annotating RCU pointers.
>
> sched_domains_numa_distance:
>   error: incompatible types in comparison expression
>   (different address spaces):
>       int [noderef] __rcu *
>       int *
>
> sched_domains_numa_masks:
>   error: incompatible types in comparison expression
>   (different address spaces):
>       struct cpumask **[noderef] __rcu *
>       struct cpumask ***
>
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>

That's from when the NUMA topologies were made dynamic, which should be:
Fixes: 0fb3978b0aac ("sched/numa: Fix NUMA topology for systems with CPU-less nodes")
> ---
>  kernel/sched/topology.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index 10d1391e7416..0342a4f41f09 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -1542,8 +1542,8 @@ static int			sched_domains_numa_levels;
>  static int			sched_domains_curr_level;
>
>  int				sched_max_numa_distance;
> -static int			*sched_domains_numa_distance;
> -static struct cpumask		***sched_domains_numa_masks;
> +static int			__rcu *sched_domains_numa_distance;
> +static struct cpumask		** __rcu *sched_domains_numa_masks;

I understand that's what sparse is asking for, but that looks odd to me. We
use it as:

  rcu_assign_pointer(sched_domains_numa_masks, foo);

so why isn't it

  __rcu ***sched_domains_numa_masks;

?

This isn't a pointer to an RCU-protected array of masks, this is an
RCU-protected double array of masks.
>  #endif
>
>  /*
> --
> 2.25.1

Re: [PATCH 1/2] sched/topology: Annotate RCU pointers properly

[kernel test robot]

Hi Pierre,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tip/sched/core]
[also build test WARNING on tip/master peterz-queue/sched/core tip/auto-latest linus/master v6.7-rc8 next-20240104]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Pierre-Gondois/sched-topology-Annotate-RCU-pointers-properly/20240103-210154
base:   tip/sched/core
patch link:    https://lore.kernel.org/r/20240103125648.194516-2-pierre.gondois%40arm.com
patch subject: [PATCH 1/2] sched/topology: Annotate RCU pointers properly
config: arm64-randconfig-r111-20240104 (https://download.01.org/0day-ci/archive/20240104/202401042241.EQntOcPK-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20240104/202401042241.EQntOcPK-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401042241.EQntOcPK-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
   kernel/sched/build_utility.c: note: in included file:
   kernel/sched/topology.c:485:19: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct perf_domain *pd @@     got struct perf_domain [noderef] __rcu *pd @@
   kernel/sched/topology.c:485:19: sparse:     expected struct perf_domain *pd
   kernel/sched/topology.c:485:19: sparse:     got struct perf_domain [noderef] __rcu *pd
   kernel/sched/topology.c:647:49: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct sched_domain *parent @@     got struct sched_domain [noderef] __rcu *parent @@
   kernel/sched/topology.c:647:49: sparse:     expected struct sched_domain *parent
   kernel/sched/topology.c:647:49: sparse:     got struct sched_domain [noderef] __rcu *parent
   kernel/sched/topology.c:732:50: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct sched_domain *parent @@     got struct sched_domain [noderef] __rcu *parent @@
   kernel/sched/topology.c:732:50: sparse:     expected struct sched_domain *parent
   kernel/sched/topology.c:732:50: sparse:     got struct sched_domain [noderef] __rcu *parent
   kernel/sched/topology.c:740:55: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct sched_domain [noderef] __rcu *[noderef] __rcu child @@     got struct sched_domain *[assigned] tmp @@
   kernel/sched/topology.c:740:55: sparse:     expected struct sched_domain [noderef] __rcu *[noderef] __rcu child
   kernel/sched/topology.c:740:55: sparse:     got struct sched_domain *[assigned] tmp
   kernel/sched/topology.c:753:29: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct sched_domain *[assigned] tmp @@     got struct sched_domain [noderef] __rcu *parent @@
   kernel/sched/topology.c:753:29: sparse:     expected struct sched_domain *[assigned] tmp
   kernel/sched/topology.c:753:29: sparse:     got struct sched_domain [noderef] __rcu *parent
   kernel/sched/topology.c:758:20: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct sched_domain *sd @@     got struct sched_domain [noderef] __rcu *parent @@
   kernel/sched/topology.c:758:20: sparse:     expected struct sched_domain *sd
   kernel/sched/topology.c:758:20: sparse:     got struct sched_domain [noderef] __rcu *parent
   kernel/sched/topology.c:779:13: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct sched_domain *[assigned] tmp @@     got struct sched_domain [noderef] __rcu *sd @@
   kernel/sched/topology.c:779:13: sparse:     expected struct sched_domain *[assigned] tmp
   kernel/sched/topology.c:779:13: sparse:     got struct sched_domain [noderef] __rcu *sd
   kernel/sched/topology.c:941:70: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct sched_domain *sd @@     got struct sched_domain [noderef] __rcu *child @@
   kernel/sched/topology.c:941:70: sparse:     expected struct sched_domain *sd
   kernel/sched/topology.c:941:70: sparse:     got struct sched_domain [noderef] __rcu *child
   kernel/sched/topology.c:970:59: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct sched_domain *sd @@     got struct sched_domain [noderef] __rcu *child @@
   kernel/sched/topology.c:970:59: sparse:     expected struct sched_domain *sd
   kernel/sched/topology.c:970:59: sparse:     got struct sched_domain [noderef] __rcu *child
   kernel/sched/topology.c:1016:57: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct sched_domain *sd @@     got struct sched_domain [noderef] __rcu *child @@
   kernel/sched/topology.c:1016:57: sparse:     expected struct sched_domain *sd
   kernel/sched/topology.c:1016:57: sparse:     got struct sched_domain [noderef] __rcu *child
   kernel/sched/topology.c:1018:25: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct sched_domain *sibling @@     got struct sched_domain [noderef] __rcu *child @@
   kernel/sched/topology.c:1018:25: sparse:     expected struct sched_domain *sibling
   kernel/sched/topology.c:1018:25: sparse:     got struct sched_domain [noderef] __rcu *child
   kernel/sched/topology.c:1026:55: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct sched_domain *sd @@     got struct sched_domain [noderef] __rcu *child @@
   kernel/sched/topology.c:1026:55: sparse:     expected struct sched_domain *sd
   kernel/sched/topology.c:1026:55: sparse:     got struct sched_domain [noderef] __rcu *child
   kernel/sched/topology.c:1028:25: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct sched_domain *sibling @@     got struct sched_domain [noderef] __rcu *child @@
   kernel/sched/topology.c:1028:25: sparse:     expected struct sched_domain *sibling
   kernel/sched/topology.c:1028:25: sparse:     got struct sched_domain [noderef] __rcu *child
   kernel/sched/topology.c:1098:62: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct sched_domain *sd @@     got struct sched_domain [noderef] __rcu *child @@
   kernel/sched/topology.c:1098:62: sparse:     expected struct sched_domain *sd
   kernel/sched/topology.c:1098:62: sparse:     got struct sched_domain [noderef] __rcu *child
   kernel/sched/topology.c:1202:40: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct sched_domain *child @@     got struct sched_domain [noderef] __rcu *child @@
   kernel/sched/topology.c:1202:40: sparse:     expected struct sched_domain *child
   kernel/sched/topology.c:1202:40: sparse:     got struct sched_domain [noderef] __rcu *child
   kernel/sched/topology.c:1622:43: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct sched_domain [noderef] __rcu *child @@     got struct sched_domain *child @@
   kernel/sched/topology.c:1622:43: sparse:     expected struct sched_domain [noderef] __rcu *child
   kernel/sched/topology.c:1622:43: sparse:     got struct sched_domain *child
>> kernel/sched/topology.c:1998:19: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected int *distances @@     got int [noderef] __rcu *static [addressable] [assigned] [toplevel] sched_domains_numa_distance @@
   kernel/sched/topology.c:1998:19: sparse:     expected int *distances
   kernel/sched/topology.c:1998:19: sparse:     got int [noderef] __rcu *static [addressable] [assigned] [toplevel] sched_domains_numa_distance
>> kernel/sched/topology.c:2000:15: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct cpumask ***masks @@     got struct cpumask **[noderef] __rcu *static [addressable] [assigned] [toplevel] sched_domains_numa_masks @@
   kernel/sched/topology.c:2000:15: sparse:     expected struct cpumask ***masks
   kernel/sched/topology.c:2000:15: sparse:     got struct cpumask **[noderef] __rcu *static [addressable] [assigned] [toplevel] sched_domains_numa_masks
   kernel/sched/topology.c:2321:31: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct sched_domain [noderef] __rcu *parent @@     got struct sched_domain *sd @@
   kernel/sched/topology.c:2321:31: sparse:     expected struct sched_domain [noderef] __rcu *parent
   kernel/sched/topology.c:2321:31: sparse:     got struct sched_domain *sd
   kernel/sched/topology.c:2425:57: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct sched_domain *[assigned] sd @@     got struct sched_domain [noderef] __rcu *parent @@
   kernel/sched/topology.c:2425:57: sparse:     expected struct sched_domain *[assigned] sd
   kernel/sched/topology.c:2425:57: sparse:     got struct sched_domain [noderef] __rcu *parent
   kernel/sched/topology.c:2446:56: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct sched_domain *child @@     got struct sched_domain [noderef] __rcu *child @@
   kernel/sched/topology.c:2446:56: sparse:     expected struct sched_domain *child
   kernel/sched/topology.c:2446:56: sparse:     got struct sched_domain [noderef] __rcu *child
   kernel/sched/topology.c:2445:57: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct sched_domain *[assigned] sd @@     got struct sched_domain [noderef] __rcu *parent @@
   kernel/sched/topology.c:2445:57: sparse:     expected struct sched_domain *[assigned] sd
   kernel/sched/topology.c:2445:57: sparse:     got struct sched_domain [noderef] __rcu *parent
   kernel/sched/topology.c:2500:57: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct sched_domain *[assigned] sd @@     got struct sched_domain [noderef] __rcu *parent @@
   kernel/sched/topology.c:2500:57: sparse:     expected struct sched_domain *[assigned] sd
   kernel/sched/topology.c:2500:57: sparse:     got struct sched_domain [noderef] __rcu *parent
   kernel/sched/build_utility.c: note: in included file (through include/linux/smp.h, include/linux/sched/clock.h):
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   kernel/sched/build_utility.c: note: in included file:
   kernel/sched/sched.h:1846:9: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct sched_domain *[assigned] sd @@     got struct sched_domain [noderef] __rcu *parent @@
   kernel/sched/sched.h:1846:9: sparse:     expected struct sched_domain *[assigned] sd
   kernel/sched/sched.h:1846:9: sparse:     got struct sched_domain [noderef] __rcu *parent
   kernel/sched/sched.h:1867:9: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct sched_domain *[assigned] sd @@     got struct sched_domain [noderef] __rcu *parent @@
   kernel/sched/sched.h:1867:9: sparse:     expected struct sched_domain *[assigned] sd
   kernel/sched/sched.h:1867:9: sparse:     got struct sched_domain [noderef] __rcu *parent
   kernel/sched/sched.h:1867:9: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct sched_domain *[assigned] sd @@     got struct sched_domain [noderef] __rcu *parent @@
   kernel/sched/sched.h:1867:9: sparse:     expected struct sched_domain *[assigned] sd
   kernel/sched/sched.h:1867:9: sparse:     got struct sched_domain [noderef] __rcu *parent
   kernel/sched/sched.h:1846:9: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct sched_domain *[assigned] sd @@     got struct sched_domain [noderef] __rcu *parent @@
   kernel/sched/sched.h:1846:9: sparse:     expected struct sched_domain *[assigned] sd
   kernel/sched/sched.h:1846:9: sparse:     got struct sched_domain [noderef] __rcu *parent
   kernel/sched/sched.h:1867:9: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct sched_domain *[assigned] sd @@     got struct sched_domain [noderef] __rcu *parent @@
   kernel/sched/sched.h:1867:9: sparse:     expected struct sched_domain *[assigned] sd
   kernel/sched/sched.h:1867:9: sparse:     got struct sched_domain [noderef] __rcu *parent
   kernel/sched/build_utility.c: note: in included file:
   kernel/sched/topology.c:741:39: sparse: sparse: dereference of noderef expression
   kernel/sched/topology.c:971:31: sparse: sparse: dereference of noderef expression
   kernel/sched/build_utility.c: note: in included file (through include/linux/smp.h, include/linux/sched/clock.h):
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   kernel/sched/build_utility.c: note: in included file:
   kernel/sched/topology.c:1643:19: sparse: sparse: dereference of noderef expression
   kernel/sched/topology.c:1658:48: sparse: sparse: dereference of noderef expression
   kernel/sched/topology.c:1723:40: sparse: sparse: dereference of noderef expression
   kernel/sched/topology.c:1934:86: sparse: sparse: dereference of noderef expression
   kernel/sched/topology.c:1983:9: sparse: sparse: dereference of noderef expression
   kernel/sched/topology.c:2053:82: sparse: sparse: dereference of noderef expression
   kernel/sched/topology.c:2054:78: sparse: sparse: dereference of noderef expression
   kernel/sched/topology.c:2054:78: sparse: sparse: dereference of noderef expression
   kernel/sched/topology.c:2065:53: sparse: sparse: dereference of noderef expression
   kernel/sched/topology.c:2066:80: sparse: sparse: dereference of noderef expression
   kernel/sched/topology.c:2066:80: sparse: sparse: dereference of noderef expression
   kernel/sched/topology.c:2483:51: sparse: sparse: dereference of noderef expression
   kernel/sched/topology.c:2484:49: sparse: sparse: dereference of noderef expression
   kernel/sched/topology.c:2486:52: sparse: sparse: dereference of noderef expression
   kernel/sched/build_utility.c: note: in included file:
   kernel/sched/sched.h:1867:9: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct sched_domain *[assigned] sd @@     got struct sched_domain [noderef] __rcu *parent @@
   kernel/sched/sched.h:1867:9: sparse:     expected struct sched_domain *[assigned] sd
   kernel/sched/sched.h:1867:9: sparse:     got struct sched_domain [noderef] __rcu *parent
   kernel/sched/build_utility.c: note: in included file:
   kernel/sched/core_sched.c:94:23: sparse: sparse: context imbalance in 'sched_core_update_cookie' - wrong count at exit

vim +1998 kernel/sched/topology.c

0083242c93759d Valentin Schneider 2021-08-18  1987  
0083242c93759d Valentin Schneider 2021-08-18  1988  
0fb3978b0aac3a Huang Ying         2022-02-14  1989  static void sched_reset_numa(void)
0fb3978b0aac3a Huang Ying         2022-02-14  1990  {
0fb3978b0aac3a Huang Ying         2022-02-14  1991  	int nr_levels, *distances;
0fb3978b0aac3a Huang Ying         2022-02-14  1992  	struct cpumask ***masks;
0083242c93759d Valentin Schneider 2021-08-18  1993  
0fb3978b0aac3a Huang Ying         2022-02-14  1994  	nr_levels = sched_domains_numa_levels;
0fb3978b0aac3a Huang Ying         2022-02-14  1995  	sched_domains_numa_levels = 0;
0fb3978b0aac3a Huang Ying         2022-02-14  1996  	sched_max_numa_distance = 0;
0fb3978b0aac3a Huang Ying         2022-02-14  1997  	sched_numa_topology_type = NUMA_DIRECT;
0fb3978b0aac3a Huang Ying         2022-02-14 @1998  	distances = sched_domains_numa_distance;
0fb3978b0aac3a Huang Ying         2022-02-14  1999  	rcu_assign_pointer(sched_domains_numa_distance, NULL);
0fb3978b0aac3a Huang Ying         2022-02-14 @2000  	masks = sched_domains_numa_masks;
0fb3978b0aac3a Huang Ying         2022-02-14  2001  	rcu_assign_pointer(sched_domains_numa_masks, NULL);
0fb3978b0aac3a Huang Ying         2022-02-14  2002  	if (distances || masks) {
0fb3978b0aac3a Huang Ying         2022-02-14  2003  		int i, j;
0083242c93759d Valentin Schneider 2021-08-18  2004  
0fb3978b0aac3a Huang Ying         2022-02-14  2005  		synchronize_rcu();
0fb3978b0aac3a Huang Ying         2022-02-14  2006  		kfree(distances);
0fb3978b0aac3a Huang Ying         2022-02-14  2007  		for (i = 0; i < nr_levels && masks; i++) {
0fb3978b0aac3a Huang Ying         2022-02-14  2008  			if (!masks[i])
0083242c93759d Valentin Schneider 2021-08-18  2009  				continue;
0fb3978b0aac3a Huang Ying         2022-02-14  2010  			for_each_node(j)
0fb3978b0aac3a Huang Ying         2022-02-14  2011  				kfree(masks[i][j]);
0fb3978b0aac3a Huang Ying         2022-02-14  2012  			kfree(masks[i]);
0fb3978b0aac3a Huang Ying         2022-02-14  2013  		}
0fb3978b0aac3a Huang Ying         2022-02-14  2014  		kfree(masks);
0fb3978b0aac3a Huang Ying         2022-02-14  2015  	}
0fb3978b0aac3a Huang Ying         2022-02-14  2016  	if (sched_domain_topology_saved) {
0fb3978b0aac3a Huang Ying         2022-02-14  2017  		kfree(sched_domain_topology);
0fb3978b0aac3a Huang Ying         2022-02-14  2018  		sched_domain_topology = sched_domain_topology_saved;
0fb3978b0aac3a Huang Ying         2022-02-14  2019  		sched_domain_topology_saved = NULL;
0083242c93759d Valentin Schneider 2021-08-18  2020  	}
0083242c93759d Valentin Schneider 2021-08-18  2021  }
0083242c93759d Valentin Schneider 2021-08-18  2022  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/2] sched/topology: Annotate RCU pointers properly

[Pierre Gondois]

Hello Valentin,

On 1/4/24 10:54, Valentin Schneider wrote:
> On 03/01/24 13:56, Pierre Gondois wrote:
>> Cleanup RCU-related spare errors by annotating RCU pointers.
>>
>> sched_domains_numa_distance:
>>    error: incompatible types in comparison expression
>>    (different address spaces):
>>        int [noderef] __rcu *
>>        int *
>>
>> sched_domains_numa_masks:
>>    error: incompatible types in comparison expression
>>    (different address spaces):
>>        struct cpumask **[noderef] __rcu *
>>        struct cpumask ***
>>
>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> 
> That's from when the NUMA topologies were made dynamic, which should be:
> Fixes: 0fb3978b0aac ("sched/numa: Fix NUMA topology for systems with CPU-less nodes")

Ok yes

>> ---
>>   kernel/sched/topology.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
>> index 10d1391e7416..0342a4f41f09 100644
>> --- a/kernel/sched/topology.c
>> +++ b/kernel/sched/topology.c
>> @@ -1542,8 +1542,8 @@ static int			sched_domains_numa_levels;
>>   static int			sched_domains_curr_level;
>>
>>   int				sched_max_numa_distance;
>> -static int			*sched_domains_numa_distance;
>> -static struct cpumask		***sched_domains_numa_masks;
>> +static int			__rcu *sched_domains_numa_distance;
>> +static struct cpumask		** __rcu *sched_domains_numa_masks;
> 
> I understand that's what sparse is asking for, but that looks odd to me. We
> use it as:
> 
>    rcu_assign_pointer(sched_domains_numa_masks, foo);
> 
> so why isn't it
> 
>    __rcu ***sched_domains_numa_masks;
> 
> ?
> 
> This isn't a pointer to an RCU-protected array of masks, this is an
> RCU-protected double array of masks.

I think:
   static struct cpumask ** __rcu *sched_domains_numa_masks;
should denote an RCU-protected array^3 of 'struct cpumask', when
   static struct cpumask __rcu ***sched_domains_numa_masks;
would denote an array^2 of RCU-protected 'struct cpumask*',
and assignments would look like:
   rcu_assign_pointer(**sched_domains_numa_masks, foo);

Meaning that, when taking as a better example:
   static int __rcu *sched_domains_numa_distance;
Here we would like to avoid having 'access after free' to the array of
integer allocated to sched_domains_numa_distance.

For sched_domains_numa_masks:
   static struct cpumask	** __rcu *sched_domains_numa_masks;
   rcu_assign_pointer(sched_domains_numa_masks, foo);
   bar = rcu_dereference(sched_domains_numa_masks);
once the first array pointed by sched_domains_numa_masks is accessed/assigned,
we know the RCU-framework makes it safe against 'accesses after free' when
accessing the level-2 array of sched_domains_numa_masks, or accessing the mask
(level 3).

Please let me know if the reasoning seems dodgy.

About the kernel test robot:
   kernel/sched/topology.c:1998:19: sparse: sparse: incorrect type in assignment (different address spaces) @@
        expected int *distances @@
        got int [noderef] __rcu *static [addressable] [assigned] [toplevel] sched_domains_numa_distance
   kernel/sched/topology.c:2000:15: sparse: sparse: incorrect type in assignment (different address spaces) @@
        expected struct cpumask ***masks @@
        got struct cpumask **[noderef] __rcu *static [addressable] [assigned] [toplevel] sched_domains_numa_masks @@
I think the warnings can be ignored since the two pointers (distances, masks)
are only dereferenced after a 'synchronize_rcu()'

Regards,
Pierre

>>   #endif
>>
>>   /*
>> --
>> 2.25.1
> 

Re: [PATCH 2/2] sched/fair: Use rq in idle_cpu_without()

[Pierre Gondois]

Hello Shrikanth,

On 1/4/24 06:15, Shrikanth Hegde wrote:
> 
> 
> On 1/3/24 6:26 PM, Pierre Gondois wrote:
>> idle_cpu_without() could receive a 'struct rq' instead of a
>> cpu number to avoid converting the cpu number to a 'struct rq'
> 
> nit: s/cpu/CPU
> 
>> two times. Indeed update_sg_wakeup_stats() already makes the
>> conversion.
> 
> This change looks good. There maybe other candidates which might get simplified
> as well. for example, update_blocked_averages. (and then there are some
> like balance_push_set which maybe borderline when it comes to such simplification)

Ok yes, I'll check the functions you pointed out.

> 
>>
>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>> ---
>>   kernel/sched/fair.c | 8 +++-----
>>   1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 93e928e76959..d38fec26fd3d 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -10184,15 +10184,13 @@ static unsigned int task_running_on_cpu(int cpu, struct task_struct *p)
>>   
>>   /**
>>    * idle_cpu_without - would a given CPU be idle without p ?
>> - * @cpu: the processor on which idleness is tested.
>> + * @rq: the rq on which idleness is tested.
>>    * @p: task which should be ignored.
>>    *
>>    * Return: 1 if the CPU would be idle. 0 otherwise.
>>    */
>> -static int idle_cpu_without(int cpu, struct task_struct *p)
>> +static int idle_cpu_without(struct rq *rq, struct task_struct *p)
> 
> This might need change in the function name too. perception here is that, is the
> CPU idle without task p.

Yes right, I'll rename to idle_rq_without() in the next version.

Regards,
Pierre

> Otherwise LGTM.
> 
> Reviewed-by: Shrikanth Hegde <sshegde@linux.vnet.ibm.com>
> 
>>   {
>> -	struct rq *rq = cpu_rq(cpu);
>> -
>>   	if (rq->curr != rq->idle && rq->curr != p)
>>   		return 0;
>>   
>> @@ -10247,7 +10245,7 @@ static inline void update_sg_wakeup_stats(struct sched_domain *sd,
>>   		/*
>>   		 * No need to call idle_cpu_without() if nr_running is not 0
>>   		 */
>> -		if (!nr_running && idle_cpu_without(i, p))
>> +		if (!nr_running && idle_cpu_without(rq, p))
>>   			sgs->idle_cpus++;
>>   
>>   		/* Check if task fits in the CPU */
> 
>