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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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 <[email protected]>
> {
> - 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 */
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 <[email protected]>
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
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/[email protected]/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20240104/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
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
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 <[email protected]>
>
> 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
>
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 <[email protected]>
>> ---
>> 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 <[email protected]>
>
>> {
>> - 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 */
>
>