[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZEHAkGEP/k9m7lKW@yury-ThinkPad>
Date: Thu, 20 Apr 2023 15:45:49 -0700
From: Yury Norov <yury.norov@...il.com>
To: Tariq Toukan <ttoukan.linux@...il.com>
Cc: Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
linux-rdma@...r.kernel.org, linux-kernel@...r.kernel.org,
Saeed Mahameed <saeedm@...dia.com>,
Pawel Chmielewski <pawel.chmielewski@...el.com>,
Leon Romanovsky <leon@...nel.org>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Paolo Abeni <pabeni@...hat.com>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Rasmus Villemoes <linux@...musvillemoes.dk>,
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>,
Daniel Bristot de Oliveira <bristot@...hat.com>,
Valentin Schneider <vschneid@...hat.com>,
Gal Pressman <gal@...dia.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Heiko Carstens <hca@...ux.ibm.com>,
Barry Song <baohua@...nel.org>
Subject: Re: [PATCH v2 4/8] net: mlx5: switch comp_irqs_request() to using
for_each_numa_cpu
On Thu, Apr 20, 2023 at 11:27:26AM +0300, Tariq Toukan wrote:
> I like this clean API.
Thanks :)
> nit:
> Previously cpu_online_mask was used here. Is this change intentional?
> We can fix it in a followup patch if this is the only comment on the series.
>
> Reviewed-by: Tariq Toukan <tariqt@...dia.com>
The only CPUs listed in the sched_domains_numa_masks are 'available',
i.e. online CPUs. The for_each_numa_cpu() ANDs user-provided cpumask
with a map associate to the hop, and that means that if we AND with
possible mask, we'll eventually walk online CPUs only.
To make sure, I experimented with the modified test:
diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
index 6becb044a66f..c8d557731080 100644
--- a/lib/test_bitmap.c
+++ b/lib/test_bitmap.c
@@ -760,8 +760,13 @@ static void __init test_for_each_numa(void)
unsigned int hop, c = 0;
rcu_read_lock();
- for_each_numa_cpu(cpu, hop, node, cpu_online_mask)
+ pr_err("Node %d:\t", node);
+ for_each_numa_cpu(cpu, hop, node, cpu_possible_mask) {
expect_eq_uint(cpumask_local_spread(c++, node), cpu);
+ pr_cont("%3d", cpu);
+
+ }
+ pr_err("\n");
rcu_read_unlock();
}
}
This is the NUMA topology of my test machine after the boot:
root@...ian:~# numactl -H
available: 4 nodes (0-3)
node 0 cpus: 0 1 2 3
node 0 size: 1861 MB
node 0 free: 1792 MB
node 1 cpus: 4 5
node 1 size: 1914 MB
node 1 free: 1823 MB
node 2 cpus: 6 7
node 2 size: 1967 MB
node 2 free: 1915 MB
node 3 cpus: 8 9 10 11 12 13 14 15
node 3 size: 7862 MB
node 3 free: 7259 MB
node distances:
node 0 1 2 3
0: 10 50 30 70
1: 50 10 70 30
2: 30 70 10 50
3: 70 30 50 10
And this is what test prints:
root@...ian:~# insmod test_bitmap.ko
test_bitmap: loaded.
test_bitmap: parselist: 14: input is '0-2047:128/256' OK, Time: 472
test_bitmap: bitmap_print_to_pagebuf: input is '0-32767
', Time: 2665
test_bitmap: Node 0: 0 1 2 3 6 7 4 5 8 9 10 11 12 13 14 15
test_bitmap:
test_bitmap: Node 1: 4 5 8 9 10 11 12 13 14 15 0 1 2 3 6 7
test_bitmap:
test_bitmap: Node 2: 6 7 0 1 2 3 8 9 10 11 12 13 14 15 4 5
test_bitmap:
test_bitmap: Node 3: 8 9 10 11 12 13 14 15 4 5 6 7 0 1 2 3
test_bitmap:
test_bitmap: all 6614 tests passed
Now, disable a couple of CPUs:
root@...ian:~# chcpu -d 1-2
smpboot: CPU 1 is now offline
CPU 1 disabled
smpboot: CPU 2 is now offline
CPU 2 disabled
And try again:
root@...ian:~# rmmod test_bitmap
rmmod: ERROR: ../libkmod/libkmod[ 320.275904] test_bitmap: unloaded.
root@...ian:~# numactl -H
available: 4 nodes (0-3)
node 0 cpus: 0 3
node 0 size: 1861 MB
node 0 free: 1792 MB
node 1 cpus: 4 5
node 1 size: 1914 MB
node 1 free: 1823 MB
node 2 cpus: 6 7
node 2 size: 1967 MB
node 2 free: 1915 MB
node 3 cpus: 8 9 10 11 12 13 14 15
node 3 size: 7862 MB
node 3 free: 7259 MB
node distances:
node 0 1 2 3
0: 10 50 30 70
1: 50 10 70 30
2: 30 70 10 50
3: 70 30 50 10
root@...ian:~# insmod test_bitmap.ko
test_bitmap: loaded.
test_bitmap: parselist: 14: input is '0-2047:128/256' OK, Time: 491
test_bitmap: bitmap_print_to_pagebuf: input is '0-32767
', Time: 2174
test_bitmap: Node 0: 0 3 6 7 4 5 8 9 10 11 12 13 14 15
test_bitmap:
test_bitmap: Node 1: 4 5 8 9 10 11 12 13 14 15 0 3 6 7
test_bitmap:
test_bitmap: Node 2: 6 7 0 3 8 9 10 11 12 13 14 15 4 5
test_bitmap:
test_bitmap: Node 3: 8 9 10 11 12 13 14 15 4 5 6 7 0 3
test_bitmap:
test_bitmap: all 6606 tests passed
I used cpu_possible_mask because I wanted to keep the patch
consistent: before we traversed NUMA hop masks, now we traverse the
same hop masks AND user-provided mask, so the latter should include
all possible CPUs.
If you think it's better to have cpu_online_mask in the driver, let's
make it in a separate patch?
Thanks,
Yury
Powered by blists - more mailing lists