Message ID | 20230420051946.7463-8-yury.norov@gmail.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp91574vqo; Wed, 19 Apr 2023 22:33:51 -0700 (PDT) X-Google-Smtp-Source: AKy350bOJGHQWRNSIq6qNJ3HOvvPO0D1f2ceW+0s0wqqf2vOxrLhmZQKWeRl5sfTiU4HMmA6O1hJ X-Received: by 2002:a05:6a21:9985:b0:ef:4922:a70a with SMTP id ve5-20020a056a21998500b000ef4922a70amr715268pzb.30.1681968831087; Wed, 19 Apr 2023 22:33:51 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1681968831; cv=none; d=google.com; s=arc-20160816; b=GRBKbe5lvAWtt3oGqHDaJOHKe4F3rPj0xNj784X4ZQgbrsMNfkvIp83El0D1PaCtS6 9W2eOyNlykvnxDUPUHubcdsi+Kclb1fQ8bSodFjzs1WOEy93wiW5RMhdkoKCgeHasIvF ExabafPSLuHVvb2Nlnc3OilPPb7vyed0sL7kMKuKO9qdThA+xMHBsCnNzLQnOtdgO6sh acAbKqdgPjhBmFHkWfR9804bXKzXgGwqXhTcledoN6qmWRFo9dk/94LnZGdSZeMaN6gf wuCXrF6qNov6VPOeTpKQD9KuvXZZeLIm1O8PV0VQpW549jTzThU7dw36R8edl6ns2/VA SNmw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=OSjdqRXXXgYSdBLqK0Rzny0ztxBdvMaFW0s7IjJDzbk=; b=adxwzgAS+K9YSF/O6R1kjPONdPnOSW9eX+tIzYobwLgQcbEFzj0fSv9gFqDsBOp9CZ aMCVJfMuCDPFeA+nGuWMOV9rSyk/O7t5lFrZLXQBDvyYnhmL/fS2Mkt8M65E3BxyDGEU ZfadQ48kdZN+C89eOfQ7Ma77g4TJkgo5F6hSotC57HEvxc4CTZhlSXZ1EchJI9i291GB fNtZO/FjU8peQdx7QEA18LdYH3aWRQ8suxGqxlQsXB5aAZK/G4EEc7IxdlILWmsjfcIq Nqk5bYw6/dYCUdOg1Rx/0sbGznoxsEiHV1S6Ig9t+B37zKGTroRyZu0B5u+B+hbmoeX9 5iVw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20221208 header.b=Ybxwe0Ye; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id z12-20020a655a4c000000b0051323bafb4bsi652878pgs.841.2023.04.19.22.33.38; Wed, 19 Apr 2023 22:33:51 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20221208 header.b=Ybxwe0Ye; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233820AbjDTFVK (ORCPT <rfc822;peter110.wang@gmail.com> + 99 others); Thu, 20 Apr 2023 01:21:10 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37546 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233819AbjDTFUh (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 20 Apr 2023 01:20:37 -0400 Received: from mail-pl1-x634.google.com (mail-pl1-x634.google.com [IPv6:2607:f8b0:4864:20::634]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 051AC61AB; Wed, 19 Apr 2023 22:20:04 -0700 (PDT) Received: by mail-pl1-x634.google.com with SMTP id d9443c01a7336-1a6c5acf6ccso5829165ad.3; Wed, 19 Apr 2023 22:20:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1681968004; x=1684560004; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=OSjdqRXXXgYSdBLqK0Rzny0ztxBdvMaFW0s7IjJDzbk=; b=Ybxwe0Yed6TDQbZt4P4jJnwzwNYoazEymPMwfm7y2iWBuo10gwWPCtJXLWAI2jp88R 7G/ZrXA4ugRV40R+HD8eK6Uqf+RCesq67q/t+Fgb94vuYbz193xfsE9pcylZrhvDYkyO 4+r+C0yP0GYVslLqpFnHTmKKReOrIkrmSY9ZT/0/4F6Np20g+kPjTVTbGC9/OMCQ/7gc +kZllz8/kI893xCtYA+nPmryHa1BfbJTqxI0b+jkyUlgRvFVLDmRYztiohR6fCgWqXii ZsnoBGWw9v+EgD0Ae5WNtPCYiQahOybJSXPdCl4kazJ1X4mZqTHy3wLcH3n9ofx5Rn21 KqVA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1681968004; x=1684560004; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=OSjdqRXXXgYSdBLqK0Rzny0ztxBdvMaFW0s7IjJDzbk=; b=HauaI6ZWqKCYXs68jAM9390TmX3LB3RkbKdara/+suRee709/qHalqaZsb11/25cmb zQaHgJf8HfbxfWa8enSHYzDTyY7Ts/IfeI4kcgl6RrKTKQLUMZ50dw/X4e9Ql2+/JRBa EKouCmLYO5FqUFQvLcwaOoSiNeDkeI3zBiB+LZyaNQCgAU3ynbBgiifxVesdStLJyI/k cwSI+V6DZhvTr4Arb0Le6pi/2leGDi0KG8gA/Cdx2kV1LjdipjXhkU+CXKaw5mo95OnK /HpH0LnVqcF5IvETfMuolxgjVdqub7IzpXXwlg0EQqJNH0Wb9UloElvknWjYjtcBt43g B82g== X-Gm-Message-State: AAQBX9ei5KB6/pCS54HvPbEiWBVZRWDT1+wa101sbpbjfzRUvKbsWq35 wJdnfP20xDOcwf/5R70GpFw= X-Received: by 2002:a17:902:ce88:b0:19d:1834:92b9 with SMTP id f8-20020a170902ce8800b0019d183492b9mr316932plg.56.1681968004129; Wed, 19 Apr 2023 22:20:04 -0700 (PDT) Received: from localhost ([2603:3024:e02:8500:653b:861d:e1ca:16ac]) by smtp.gmail.com with ESMTPSA id gq3-20020a17090b104300b0024b4a06a4fesm167781pjb.5.2023.04.19.22.20.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 19 Apr 2023 22:20:03 -0700 (PDT) From: Yury Norov <yury.norov@gmail.com> To: Jakub Kicinski <kuba@kernel.org>, netdev@vger.kernel.org, linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Yury Norov <yury.norov@gmail.com>, Saeed Mahameed <saeedm@nvidia.com>, Pawel Chmielewski <pawel.chmielewski@intel.com>, Leon Romanovsky <leon@kernel.org>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Paolo Abeni <pabeni@redhat.com>, Andy Shevchenko <andriy.shevchenko@linux.intel.com>, Rasmus Villemoes <linux@rasmusvillemoes.dk>, Ingo Molnar <mingo@redhat.com>, Peter Zijlstra <peterz@infradead.org>, Juri Lelli <juri.lelli@redhat.com>, Vincent Guittot <vincent.guittot@linaro.org>, Dietmar Eggemann <dietmar.eggemann@arm.com>, Steven Rostedt <rostedt@goodmis.org>, Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>, Daniel Bristot de Oliveira <bristot@redhat.com>, Valentin Schneider <vschneid@redhat.com>, Tariq Toukan <tariqt@nvidia.com>, Gal Pressman <gal@nvidia.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Heiko Carstens <hca@linux.ibm.com>, Barry Song <baohua@kernel.org> Subject: [PATCH v2 7/8] lib: add test for for_each_numa_{cpu,hop_mask}() Date: Wed, 19 Apr 2023 22:19:45 -0700 Message-Id: <20230420051946.7463-8-yury.norov@gmail.com> X-Mailer: git-send-email 2.37.2 In-Reply-To: <20230420051946.7463-1-yury.norov@gmail.com> References: <20230420051946.7463-1-yury.norov@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1763672149111895982?= X-GMAIL-MSGID: =?utf-8?q?1763672149111895982?= |
Series |
sched/topology: add for_each_numa_cpu() macro
|
|
Commit Message
Yury Norov
April 20, 2023, 5:19 a.m. UTC
The test ensures that enumerators' output is consistent with
cpumask_local_spread().
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
lib/test_bitmap.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
Comments
On 19/04/23 22:19, Yury Norov wrote: > + for (node = 0; node < sched_domains_numa_levels; node++) { > + unsigned int hop, c = 0; > + > + rcu_read_lock(); > + for_each_numa_cpu(cpu, hop, node, cpu_online_mask) > + expect_eq_uint(cpumask_local_spread(c++, node), cpu); > + rcu_read_unlock(); > + } I'm not fond of the export of sched_domains_numa_levels, especially considering it's just there for tests. Furthermore, is there any value is testing parity with cpumask_local_spread()? Rather, shouldn't we check that using this API does yield CPUs of increasing NUMA distance? Something like for_each_node(node) { unsigned int prev_cpu, hop = 0; cpu = cpumask_first(cpumask_of_node(node)); prev_cpu = cpu; rcu_read_lock(); /* Assert distance is monotonically increasing */ for_each_numa_cpu(cpu, hop, node, cpu_online_mask) { expect_ge_uint(cpu_to_node(cpu), cpu_to_node(prev_cpu)); prev_cpu = cpu; } rcu_read_unlock(); }
Hi Valentin, Thanks for review! On Mon, Apr 24, 2023 at 06:09:52PM +0100, Valentin Schneider wrote: > On 19/04/23 22:19, Yury Norov wrote: > > + for (node = 0; node < sched_domains_numa_levels; node++) { > > + unsigned int hop, c = 0; > > + > > + rcu_read_lock(); > > + for_each_numa_cpu(cpu, hop, node, cpu_online_mask) > > + expect_eq_uint(cpumask_local_spread(c++, node), cpu); > > + rcu_read_unlock(); > > + } > > I'm not fond of the export of sched_domains_numa_levels, especially > considering it's just there for tests. > > Furthermore, is there any value is testing parity with > cpumask_local_spread()? I wanted to emphasize that new NUMA-aware functions are coherent with each other, just like find_nth_bit() is coherent with find_next_bit(). But all that coherence looks important only in non-NUMA case, because client code may depend on fact that next CPU is never less than current. This doesn't hold for NUMA iterators anyways... > Rather, shouldn't we check that using this API does > yield CPUs of increasing NUMA distance? > > Something like > > for_each_node(node) { > unsigned int prev_cpu, hop = 0; > > cpu = cpumask_first(cpumask_of_node(node)); > prev_cpu = cpu; > > rcu_read_lock(); > > /* Assert distance is monotonically increasing */ > for_each_numa_cpu(cpu, hop, node, cpu_online_mask) { > expect_ge_uint(cpu_to_node(cpu), cpu_to_node(prev_cpu)); > prev_cpu = cpu; > } > > rcu_read_unlock(); > } Your version of the test looks more straightforward. I need to think for more, but it looks like I can take it in v3. Thanks, Yury
On 25/04/23 22:50, Yury Norov wrote: > Hi Valentin, > > Thanks for review! > > On Mon, Apr 24, 2023 at 06:09:52PM +0100, Valentin Schneider wrote: >> On 19/04/23 22:19, Yury Norov wrote: >> > + for (node = 0; node < sched_domains_numa_levels; node++) { >> > + unsigned int hop, c = 0; >> > + >> > + rcu_read_lock(); >> > + for_each_numa_cpu(cpu, hop, node, cpu_online_mask) >> > + expect_eq_uint(cpumask_local_spread(c++, node), cpu); >> > + rcu_read_unlock(); >> > + } >> >> I'm not fond of the export of sched_domains_numa_levels, especially >> considering it's just there for tests. >> >> Furthermore, is there any value is testing parity with >> cpumask_local_spread()? > > I wanted to emphasize that new NUMA-aware functions are coherent with > each other, just like find_nth_bit() is coherent with find_next_bit(). > > But all that coherence looks important only in non-NUMA case, because > client code may depend on fact that next CPU is never less than current. > This doesn't hold for NUMA iterators anyways... > Ah right, I see your point. But yes, distance-ordered walks break this assumption. >> Rather, shouldn't we check that using this API does >> yield CPUs of increasing NUMA distance? >> >> Something like >> >> for_each_node(node) { >> unsigned int prev_cpu, hop = 0; >> >> cpu = cpumask_first(cpumask_of_node(node)); >> prev_cpu = cpu; >> >> rcu_read_lock(); >> >> /* Assert distance is monotonically increasing */ >> for_each_numa_cpu(cpu, hop, node, cpu_online_mask) { >> expect_ge_uint(cpu_to_node(cpu), cpu_to_node(prev_cpu)); >> prev_cpu = cpu; >> } >> >> rcu_read_unlock(); >> } > > Your version of the test looks more straightforward. I need to think > for more, but it looks like I can take it in v3. > I realized I only wrote half the relevant code - comparing node IDs is meaningless, I meant to compare distances as we walk through the CPUs... I tested the below against a few NUMA topologies and it seems to be sane: diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c index 6becb044a66f0..8f8512d139d58 100644 --- a/lib/test_bitmap.c +++ b/lib/test_bitmap.c @@ -174,11 +174,23 @@ __check_eq_str(const char *srcfile, unsigned int line, return eq; } -#define __expect_eq(suffix, ...) \ +static bool __init +__check_ge_uint(const char *srcfile, unsigned int line, + const unsigned int a, unsigned int b) +{ + if (a < b) { + pr_err("[%s:%u] expected a(%u) >= b(%u)\n", + srcfile, line, a, b); + return false; + } + return true; +} + +#define __expect_op(op, suffix, ...) \ ({ \ int result = 0; \ total_tests++; \ - if (!__check_eq_ ## suffix(__FILE__, __LINE__, \ + if (!__check_## op ## _ ## suffix(__FILE__, __LINE__, \ ##__VA_ARGS__)) { \ failed_tests++; \ result = 1; \ @@ -186,6 +198,9 @@ __check_eq_str(const char *srcfile, unsigned int line, result; \ }) +#define __expect_eq(suffix, ...) __expect_op(eq, suffix, ##__VA_ARGS__) +#define __expect_ge(suffix, ...) __expect_op(ge, suffix, ##__VA_ARGS__) + #define expect_eq_uint(...) __expect_eq(uint, ##__VA_ARGS__) #define expect_eq_bitmap(...) __expect_eq(bitmap, ##__VA_ARGS__) #define expect_eq_pbl(...) __expect_eq(pbl, ##__VA_ARGS__) @@ -193,6 +208,8 @@ __check_eq_str(const char *srcfile, unsigned int line, #define expect_eq_clump8(...) __expect_eq(clump8, ##__VA_ARGS__) #define expect_eq_str(...) __expect_eq(str, ##__VA_ARGS__) +#define expect_ge_uint(...) __expect_ge(uint, ##__VA_ARGS__) + static void __init test_zero_clear(void) { DECLARE_BITMAP(bmap, 1024); @@ -756,12 +773,23 @@ static void __init test_for_each_numa(void) { unsigned int cpu, node; - for (node = 0; node < sched_domains_numa_levels; node++) { - unsigned int hop, c = 0; + for_each_node(node) { + unsigned int start_cpu, prev_dist, hop = 0; + + cpu = cpumask_first(cpumask_of_node(node)); + prev_dist = node_distance(node, node); + start_cpu = cpu; rcu_read_lock(); - for_each_numa_cpu(cpu, hop, node, cpu_online_mask) - expect_eq_uint(cpumask_local_spread(c++, node), cpu); + + /* Assert distance is monotonically increasing */ + for_each_numa_cpu(cpu, hop, node, cpu_online_mask) { + unsigned int dist = node_distance(cpu_to_node(cpu), cpu_to_node(start_cpu)); + + expect_ge_uint(dist, prev_dist); + prev_dist = dist; + } + rcu_read_unlock(); } }
> I realized I only wrote half the relevant code - comparing node IDs is > meaningless, I meant to compare distances as we walk through the > CPUs... I tested the below against a few NUMA topologies and it seems to be > sane: > > @@ -756,12 +773,23 @@ static void __init test_for_each_numa(void) > { > unsigned int cpu, node; > > - for (node = 0; node < sched_domains_numa_levels; node++) { > - unsigned int hop, c = 0; > + for_each_node(node) { > + unsigned int start_cpu, prev_dist, hop = 0; > + > + cpu = cpumask_first(cpumask_of_node(node)); > + prev_dist = node_distance(node, node); > + start_cpu = cpu; > > rcu_read_lock(); > - for_each_numa_cpu(cpu, hop, node, cpu_online_mask) > - expect_eq_uint(cpumask_local_spread(c++, node), cpu); > + > + /* Assert distance is monotonically increasing */ > + for_each_numa_cpu(cpu, hop, node, cpu_online_mask) { > + unsigned int dist = node_distance(cpu_to_node(cpu), cpu_to_node(start_cpu)); Interestingly, node_distance() is an arch-specific function. Generic implementation is quite useless: #define node_distance(from,to) ((from) == (to) ? LOCAL_DISTANCE : REMOTE_DISTANCE) Particularly, arm64 takes the above. With node_distance() implemented like that, we can barely test something... Taking that into the account, I think it's better to test iterator against cpumask_local_spread(), like in v2. I'll add a comment about that in v3. > + > + expect_ge_uint(dist, prev_dist); > + prev_dist = dist; > + } > + > rcu_read_unlock(); > } > }
On 26/04/23 13:51, Yury Norov wrote: >> I realized I only wrote half the relevant code - comparing node IDs is >> meaningless, I meant to compare distances as we walk through the >> CPUs... I tested the below against a few NUMA topologies and it seems to be >> sane: >> >> @@ -756,12 +773,23 @@ static void __init test_for_each_numa(void) >> { >> unsigned int cpu, node; >> >> - for (node = 0; node < sched_domains_numa_levels; node++) { >> - unsigned int hop, c = 0; >> + for_each_node(node) { >> + unsigned int start_cpu, prev_dist, hop = 0; >> + >> + cpu = cpumask_first(cpumask_of_node(node)); >> + prev_dist = node_distance(node, node); >> + start_cpu = cpu; >> >> rcu_read_lock(); >> - for_each_numa_cpu(cpu, hop, node, cpu_online_mask) >> - expect_eq_uint(cpumask_local_spread(c++, node), cpu); >> + >> + /* Assert distance is monotonically increasing */ >> + for_each_numa_cpu(cpu, hop, node, cpu_online_mask) { >> + unsigned int dist = node_distance(cpu_to_node(cpu), cpu_to_node(start_cpu)); > > Interestingly, node_distance() is an arch-specific function. Generic > implementation is quite useless: > > #define node_distance(from,to) ((from) == (to) ? LOCAL_DISTANCE : REMOTE_DISTANCE) > > Particularly, arm64 takes the above. With node_distance() implemented > like that, we can barely test something... > riscv and arm64 rely on drivers/base/arch_numa.c to provide __node_distance() (cf. CONFIG_GENERIC_ARCH_NUMA). x86, sparc, powerpc and ia64 define __node_distance() loongarch and mips define their own node_distance(). So all of those archs will have a usable node_distance(), the others won't and that means the scheduler can't do anything about it - the scheduler relies on node_distance() to understand the topolgoy!
diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c index a8005ad3bd58..1b5f805f6879 100644 --- a/lib/test_bitmap.c +++ b/lib/test_bitmap.c @@ -12,6 +12,7 @@ #include <linux/printk.h> #include <linux/slab.h> #include <linux/string.h> +#include <linux/topology.h> #include <linux/uaccess.h> #include "../tools/testing/selftests/kselftest_module.h" @@ -751,6 +752,33 @@ static void __init test_for_each_set_bit_wrap(void) } } +static void __init test_for_each_numa(void) +{ + unsigned int cpu, node; + + for (node = 0; node < sched_domains_numa_levels; node++) { + const struct cpumask *m, *p = cpu_none_mask; + unsigned int c = 0; + + rcu_read_lock(); + for_each_numa_hop_mask(m, node) { + for_each_cpu_andnot(cpu, m, p) + expect_eq_uint(cpumask_local_spread(c++, node), cpu); + p = m; + } + rcu_read_unlock(); + } + + for (node = 0; node < sched_domains_numa_levels; node++) { + unsigned int hop, c = 0; + + rcu_read_lock(); + for_each_numa_cpu(cpu, hop, node, cpu_online_mask) + expect_eq_uint(cpumask_local_spread(c++, node), cpu); + rcu_read_unlock(); + } +} + static void __init test_for_each_set_bit(void) { DECLARE_BITMAP(orig, 500); @@ -1237,6 +1265,7 @@ static void __init selftest(void) test_for_each_clear_bitrange_from(); test_for_each_set_clump8(); test_for_each_set_bit_wrap(); + test_for_each_numa(); } KSTM_MODULE_LOADERS(test_bitmap);