linux-kernel - [PATCH] lib/group_cpus.c: simplify grp_spread_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>] [day] [month] [year] [list]

Message-ID: <20240120065543.739203-1-ming.lei@redhat.com>
Date: Sat, 20 Jan 2024 14:55:43 +0800
From: Ming Lei <ming.lei@...hat.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: linux-kernel@...r.kernel.org,
	Thomas Gleixner <tglx@...utronix.de>,
	Ming Lei <ming.lei@...hat.com>,
	Yury Norov <yury.norov@...il.com>
Subject: [PATCH] lib/group_cpus.c: simplify grp_spread_init_one()

What the inner loop needs to do is to assign each cpu in `siblmsk & nmsk`.

Clean it by using cpumask_next_and(), meantime add helper of grp_assign_cpu().

So grp_spread_init_one() becomes more readable now.

Cc: Yury Norov <yury.norov@...il.com>
Cc: Thomas Gleixner <tglx@...utronix.de>
Signed-off-by: Ming Lei <ming.lei@...hat.com>
---

Hello Andrew,

Please consider to use this one to replace the 1st, 2nd & 4th patches
in Yury's V5:

- avoid to add new kernel API
- avoid to update iterator variable inside loop, which is tricky
- fix bug in the 4th patch
- add grp_assign_cpu() to make code more readable & clean

 lib/group_cpus.c | 34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/lib/group_cpus.c b/lib/group_cpus.c
index ee272c4cefcc..6cbc7379d954 100644
--- a/lib/group_cpus.c
+++ b/lib/group_cpus.c
@@ -11,32 +11,36 @@
 
 #ifdef CONFIG_SMP
 
+static bool grp_assign_cpu(int cpu, struct cpumask *irqmsk,
+		struct cpumask *nmsk)
+{
+	if (cpu >= nr_cpu_ids)
+		return false;
+
+	cpumask_clear_cpu(cpu, nmsk);
+	cpumask_set_cpu(cpu, irqmsk);
+	return true;
+}
+
 static void grp_spread_init_one(struct cpumask *irqmsk, struct cpumask *nmsk,
 				unsigned int cpus_per_grp)
 {
 	const struct cpumask *siblmsk;
-	int cpu, sibl;
-
-	for ( ; cpus_per_grp > 0; ) {
-		cpu = cpumask_first(nmsk);
+	int cpu = -1, sibl;
 
-		/* Should not happen, but I'm too lazy to think about it */
-		if (cpu >= nr_cpu_ids)
+	while (cpus_per_grp > 0) {
+		cpu = cpumask_next(cpu, nmsk);
+		if (!grp_assign_cpu(cpu, irqmsk, nmsk))
 			return;
-
-		cpumask_clear_cpu(cpu, nmsk);
-		cpumask_set_cpu(cpu, irqmsk);
 		cpus_per_grp--;
 
 		/* If the cpu has siblings, use them first */
 		siblmsk = topology_sibling_cpumask(cpu);
-		for (sibl = -1; cpus_per_grp > 0; ) {
-			sibl = cpumask_next(sibl, siblmsk);
-			if (sibl >= nr_cpu_ids)
+		sibl = cpu;
+		while (cpus_per_grp > 0) {
+			sibl = cpumask_next_and(sibl, siblmsk, nmsk);
+			if (!grp_assign_cpu(sibl, irqmsk, nmsk))
 				break;
-			if (!cpumask_test_and_clear_cpu(sibl, nmsk))
-				continue;
-			cpumask_set_cpu(sibl, irqmsk);
 			cpus_per_grp--;
 		}
 	}
-- 
2.41.0