linux-kernel - Re: [PATCH] sched: simplify sched_init

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aKSGx_86_Fo6sYZ7@yury>
Date: Tue, 19 Aug 2025 10:14:31 -0400
From: Yury Norov <yury.norov@...il.com>
To: Valentin Schneider <vschneid@...hat.com>
Cc: Srikar Dronamraju <srikar@...ux.ibm.com>,
	Ingo Molnar <mingo@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Juri Lelli <juri.lelli@...hat.com>,
	Vincent Guittot <vincent.guittot@...aro.org>,
	Dietmar Eggemann <dietmar.eggemann@....com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Ben Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] sched: simplify sched_init_numa()

On Tue, Aug 19, 2025 at 10:52:19AM +0200, Valentin Schneider wrote:
> On 21/07/25 14:02, Srikar Dronamraju wrote:
> > * Yury Norov <yury.norov@...il.com> [2025-07-19 17:07:51]:
> >
> >> From: Yury Norov (NVIDIA) <yury.norov@...il.com>
> >>
> >> The function opencodes for_each_set_bit() macro.
> >>
> >> Signed-off-by: Yury Norov (NVIDIA) <yury.norov@...il.com>
> >> ---
> >>  kernel/sched/topology.c | 7 +++----
> >>  1 file changed, 3 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> >> index b958fe48e020..7dc3c79aa480 100644
> >> --- a/kernel/sched/topology.c
> >> +++ b/kernel/sched/topology.c
> >> @@ -1932,10 +1932,9 @@ void sched_init_numa(int offline_node)
> >>              return;
> >>      }
> >>
> >> -	for (i = 0, j = 0; i < nr_levels; i++, j++) {
> >> -		j = find_next_bit(distance_map, NR_DISTANCE_VALUES, j);
> >> -		distances[i] = j;
> >> -	}
> >> +	for_each_set_bit(j, distance_map, NR_DISTANCE_VALUES)
> >> +		distances[i++] = j;
> >> +
> >
> > Dont we need to reset the value of i;
> >
> 
> That.

Yes we need.
 
> > Also now we may be iterating for NR_DISTANCE_VALUES instead of nr_levels.
> > It should be okay, since NR_DISTANCE_VALUES is just 8.
> >
> 
> DISTANCE_BITS is 8, NR_DISTANCE_VALUES is 1 << 8 aka 256.

I mistaken this DISTANCE_BITS vs NR_DISTANCE_VALUES. The first one is
8, so we can benefit from small_const_nbits() optimization. But we
actually iterate over NR_DISTANCE_VALUES - and no benefit for using
it over nr_levels.

> But here the use of for_each_set_bit() means we'll get just one extra
> iteration compared to using `nr_levels`. That said, since we *have* to
> compute `nr_levels` to allocate sched_domains_numa_distance, IMO we're not
> gaining much by using for_each_set_bit() here.

I'm not sure I understand that. If we switch the loop terminator to
nr_levels, the for_each() will be the exact functional equivalent.

I'll get back to this function shortly and send v2.

Thanks for the discussion.