linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/8] support for bitmap (and hence CPU) list "N" abbreviation
@ 2021-02-21  8:08 Paul Gortmaker
  2021-02-21  8:08 ` [PATCH 1/8] lib: test_bitmap: clearly separate ERANGE from EINVAL tests Paul Gortmaker
                   ` (9 more replies)
  0 siblings, 10 replies; 14+ messages in thread
From: Paul Gortmaker @ 2021-02-21  8:08 UTC (permalink / raw)
  To: linux-kernel
  Cc: Li Zefan, Ingo Molnar, Yury Norov, Thomas Gleixner,
	Josh Triplett, Peter Zijlstra, Paul E. McKenney,
	Frederic Weisbecker, Rasmus Villemoes, Andy Shevchenko,
	Paul Gortmaker

This is the 5th and final version of this series.  We got some good
improvements, like adding self-tests, using "N" as "just another number"
that could be used anywhere, and making things not CPU specific.

But now it is time to close this review out since is down to just
hand-wringing over hypothetical use cases, bikeshedding on upper/lower
case, and a wild goose chase on trying to avoid adding a function arg.

So, once again - thanks to all who provided input; it was all considered
even if not all of it was used.  And in that vein, just to be clear:

1) There will be no adaptive modifying or guessing what the user meant if
a range turns out to be invalid.  The caller will be responsible for
handling the -EINVAL just as things are currently today.

2) There will be no use of "L" or lower case "n" because there is simply
no need for it.  Yes, it would be simple enough to add, but it complicates
things and would also be impossible to remove later, once it went mainline.


The original text from v4 follows:

The basic objective here was to add support for "nohz_full=8-N" and/or
"rcu_nocbs="4-N" -- essentially introduce "N" as a portable reference
to the last core, evaluated at boot for anything using a CPU list.

The thinking behind this, is that people carve off a few early CPUs to
support housekeeping tasks, and perhaps dedicate one to a busy I/O
peripheral, and then the remaining pool of CPUs out to the end are a
part of a commonly configured pool used for the real work the user
cares about.

Extend that logic out to a fleet of machines - some new, and some
nearing EOL, and you've probably got a wide range of core counts to
contend with - even though the early number of cores dedicated to the
system overhead probably doesn't vary.

This change would enable sysadmins to have a common bootarg across all
such systems, and would also avoid any off-by-one fencepost errors that
happen for users who might briefly forget that core counts start at zero.

Originally I did this at the CPU subsys level, but Yury suggested it
be moved down further to bitmap level itself, which made the core 
implementation smaller and less complex, but the series longer.

New self tests are added to better exercise what bitmap range/region
currently supports, and new tests are added for the new "N" support.

Also tested boot arg and the post-boot cgroup use case as per below:

   root@hackbox:~# cat /proc/cmdline 
   BOOT_IMAGE=/boot/bzImage root=/dev/sda1 rcu_nocbs=2,3,8-N:1/2
   root@hackbox:~# dmesg|grep Offl
   rcu:     Offload RCU callbacks from CPUs: 2-3,8,10,12,14.

   root@hackbox:/sys/fs/cgroup/cpuset/foo# cat cpuset.cpus
   
   root@hackbox:/sys/fs/cgroup/cpuset/foo# /bin/echo 10-N > cpuset.cpus
   root@hackbox:/sys/fs/cgroup/cpuset/foo# cat cpuset.cpus
   10-15
   root@hackbox:/sys/fs/cgroup/cpuset/foo# /bin/echo N-N:N/N > cpuset.cpus
   root@hackbox:/sys/fs/cgroup/cpuset/foo# cat cpuset.cpus
   15

This was on a 16 core machine with CONFIG_NR_CPUS=16 in .config file.

Note that "N" is a dynamic quantity, and can change scope if the bitmap
is changed in size.  So at the risk of stating the obvious, don't use it
for "burn_eFuse=128-N" or "secure_erase_firmware=32-N" type stuff.

Paul.
---

[v5: go back to v3 location of "nbits" in region.  Add acks/reviewed.]

[v4: pair nbits with region, instead of inside it.  Split EINVAL and
 ERANGE tests.  Don't handle start/end/offset within a macro to
 abstract away nbits usage.  Added some Reviwed-by/Ack tags.]
 https://lore.kernel.org/lkml/20210209225907.78405-1-paul.gortmaker@windriver.com/

[v3: Allow "N" to be used anywhere in the region spec, i.e. "N-N:N/N" vs.
 just being allowed at end of range like "0-N".  Add new self-tests.  Drop
 "all" and "none" aliases as redundant and not worth the extra complication. ]
 https://lore.kernel.org/lkml/20210126171141.122639-1-paul.gortmaker@windriver.com/

[v2: push code down from cpu subsys to core bitmap code as per
 Yury's comments.  Change "last" to simply be "N" as per PeterZ.]
 https://lore.kernel.org/lkml/20210121223355.59780-1-paul.gortmaker@windriver.com/

[v1: https://lore.kernel.org/lkml/20210106004850.GA11682@paulmck-ThinkPad-P72/

Cc: Li Zefan <lizefan@huawei.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Yury Norov <yury.norov@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>



Paul Gortmaker (8):
  lib: test_bitmap: clearly separate ERANGE from EINVAL tests.
  lib: test_bitmap: add tests to trigger ERANGE case.
  lib: test_bitmap: add more start-end:offset/len tests
  lib: bitmap: fold nbits into region struct
  lib: bitmap: move ERANGE check from set_region to check_region
  lib: bitmap: support "N" as an alias for size of bitmap
  lib: test_bitmap: add tests for "N" alias
  rcu: deprecate "all" option to rcu_nocbs=

 .../admin-guide/kernel-parameters.rst         |  7 +++
 .../admin-guide/kernel-parameters.txt         |  4 +-
 kernel/rcu/tree_plugin.h                      |  6 +--
 lib/bitmap.c                                  | 49 +++++++++++--------
 lib/test_bitmap.c                             | 46 ++++++++++++++---
 5 files changed, 79 insertions(+), 33 deletions(-)

-- 
2.30.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/8] lib: test_bitmap: clearly separate ERANGE from EINVAL tests.
  2021-02-21  8:08 [PATCH v5 0/8] support for bitmap (and hence CPU) list "N" abbreviation Paul Gortmaker
@ 2021-02-21  8:08 ` Paul Gortmaker
  2021-02-21  8:08 ` [PATCH 2/8] lib: test_bitmap: add tests to trigger ERANGE case Paul Gortmaker
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Paul Gortmaker @ 2021-02-21  8:08 UTC (permalink / raw)
  To: linux-kernel
  Cc: Li Zefan, Ingo Molnar, Yury Norov, Thomas Gleixner,
	Josh Triplett, Peter Zijlstra, Paul E. McKenney,
	Frederic Weisbecker, Rasmus Villemoes, Andy Shevchenko,
	Paul Gortmaker

This block of tests was meant to find/flag incorrect use of the ":"
and "/" separators (syntax errors) and invalid (zero) group len.

However they were specified with an 8 bit width and 32 bit operations,
so they really contained two errors (EINVAL and ERANGE).

Promote them to 32 bit so it is clear what they are meant to target.
Then we can add tests specific for ERANGE (no syntax errors, just
doing 32bit op on 8 bit width, plus a typical 9-on-8 fencepost error).

Cc: Yury Norov <yury.norov@gmail.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 lib/test_bitmap.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
index 4425a1dd4ef1..589f2a34ceba 100644
--- a/lib/test_bitmap.c
+++ b/lib/test_bitmap.c
@@ -338,12 +338,12 @@ static const struct test_bitmap_parselist parselist_tests[] __initconst = {
 	{-EINVAL, "-1",	NULL, 8, 0},
 	{-EINVAL, "-0",	NULL, 8, 0},
 	{-EINVAL, "10-1", NULL, 8, 0},
-	{-EINVAL, "0-31:", NULL, 8, 0},
-	{-EINVAL, "0-31:0", NULL, 8, 0},
-	{-EINVAL, "0-31:0/", NULL, 8, 0},
-	{-EINVAL, "0-31:0/0", NULL, 8, 0},
-	{-EINVAL, "0-31:1/0", NULL, 8, 0},
-	{-EINVAL, "0-31:10/1", NULL, 8, 0},
+	{-EINVAL, "0-31:", NULL, 32, 0},
+	{-EINVAL, "0-31:0", NULL, 32, 0},
+	{-EINVAL, "0-31:0/", NULL, 32, 0},
+	{-EINVAL, "0-31:0/0", NULL, 32, 0},
+	{-EINVAL, "0-31:1/0", NULL, 32, 0},
+	{-EINVAL, "0-31:10/1", NULL, 32, 0},
 	{-EOVERFLOW, "0-98765432123456789:10/1", NULL, 8, 0},
 
 	{-EINVAL, "a-31", NULL, 8, 0},
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/8] lib: test_bitmap: add tests to trigger ERANGE case.
  2021-02-21  8:08 [PATCH v5 0/8] support for bitmap (and hence CPU) list "N" abbreviation Paul Gortmaker
  2021-02-21  8:08 ` [PATCH 1/8] lib: test_bitmap: clearly separate ERANGE from EINVAL tests Paul Gortmaker
@ 2021-02-21  8:08 ` Paul Gortmaker
  2021-02-21  8:08 ` [PATCH 3/8] lib: test_bitmap: add more start-end:offset/len tests Paul Gortmaker
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Paul Gortmaker @ 2021-02-21  8:08 UTC (permalink / raw)
  To: linux-kernel
  Cc: Li Zefan, Ingo Molnar, Yury Norov, Thomas Gleixner,
	Josh Triplett, Peter Zijlstra, Paul E. McKenney,
	Frederic Weisbecker, Rasmus Villemoes, Andy Shevchenko,
	Paul Gortmaker

Add tests that specify a valid range, but one that is outside the
width of the bitmap for which it is to be applied to.  These should
trigger an -ERANGE response from the code.

Cc: Yury Norov <yury.norov@gmail.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 lib/test_bitmap.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
index 589f2a34ceba..172ffbfa83c4 100644
--- a/lib/test_bitmap.c
+++ b/lib/test_bitmap.c
@@ -338,6 +338,8 @@ static const struct test_bitmap_parselist parselist_tests[] __initconst = {
 	{-EINVAL, "-1",	NULL, 8, 0},
 	{-EINVAL, "-0",	NULL, 8, 0},
 	{-EINVAL, "10-1", NULL, 8, 0},
+	{-ERANGE, "8-8", NULL, 8, 0},
+	{-ERANGE, "0-31", NULL, 8, 0},
 	{-EINVAL, "0-31:", NULL, 32, 0},
 	{-EINVAL, "0-31:0", NULL, 32, 0},
 	{-EINVAL, "0-31:0/", NULL, 32, 0},
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/8] lib: test_bitmap: add more start-end:offset/len tests
  2021-02-21  8:08 [PATCH v5 0/8] support for bitmap (and hence CPU) list "N" abbreviation Paul Gortmaker
  2021-02-21  8:08 ` [PATCH 1/8] lib: test_bitmap: clearly separate ERANGE from EINVAL tests Paul Gortmaker
  2021-02-21  8:08 ` [PATCH 2/8] lib: test_bitmap: add tests to trigger ERANGE case Paul Gortmaker
@ 2021-02-21  8:08 ` Paul Gortmaker
  2021-02-21  8:08 ` [PATCH 4/8] lib: bitmap: fold nbits into region struct Paul Gortmaker
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Paul Gortmaker @ 2021-02-21  8:08 UTC (permalink / raw)
  To: linux-kernel
  Cc: Li Zefan, Ingo Molnar, Yury Norov, Thomas Gleixner,
	Josh Triplett, Peter Zijlstra, Paul E. McKenney,
	Frederic Weisbecker, Rasmus Villemoes, Andy Shevchenko,
	Paul Gortmaker

There are inputs to bitmap_parselist() that would probably never
be entered manually by a person, but might result from some kind of
automated input generator.  Things like ranges of length 1, or group
lengths longer than nbits, overlaps, or offsets of zero.

Adding these tests serve two purposes:

1) document what might seem odd but nonetheless valid input.

2) don't regress from what we currently accept as valid.

Cc: Yury Norov <yury.norov@gmail.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Yury Norov <yury.norov@gmail.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 lib/test_bitmap.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
index 172ffbfa83c4..9c6a88c480c1 100644
--- a/lib/test_bitmap.c
+++ b/lib/test_bitmap.c
@@ -35,6 +35,8 @@ static const unsigned long exp1[] __initconst = {
 	BITMAP_FROM_U64(0x3333333311111111ULL),
 	BITMAP_FROM_U64(0xffffffff77777777ULL),
 	BITMAP_FROM_U64(0),
+	BITMAP_FROM_U64(0x00008000),
+	BITMAP_FROM_U64(0x80000000),
 };
 
 static const unsigned long exp2[] __initconst = {
@@ -335,6 +337,26 @@ static const struct test_bitmap_parselist parselist_tests[] __initconst = {
 	{0, " ,  ,,  , ,   ",		&exp1[12 * step], 8, 0},
 	{0, " ,  ,,  , ,   \n",		&exp1[12 * step], 8, 0},
 
+	{0, "0-0",			&exp1[0], 32, 0},
+	{0, "1-1",			&exp1[1 * step], 32, 0},
+	{0, "15-15",			&exp1[13 * step], 32, 0},
+	{0, "31-31",			&exp1[14 * step], 32, 0},
+
+	{0, "0-0:0/1",			&exp1[12 * step], 32, 0},
+	{0, "0-0:1/1",			&exp1[0], 32, 0},
+	{0, "0-0:1/31",			&exp1[0], 32, 0},
+	{0, "0-0:31/31",		&exp1[0], 32, 0},
+	{0, "1-1:1/1",			&exp1[1 * step], 32, 0},
+	{0, "0-15:16/31",		&exp1[2 * step], 32, 0},
+	{0, "15-15:1/2",		&exp1[13 * step], 32, 0},
+	{0, "15-15:31/31",		&exp1[13 * step], 32, 0},
+	{0, "15-31:1/31",		&exp1[13 * step], 32, 0},
+	{0, "16-31:16/31",		&exp1[3 * step], 32, 0},
+	{0, "31-31:31/31",		&exp1[14 * step], 32, 0},
+
+	{0, "0-31:1/3,1-31:1/3,2-31:1/3",	&exp1[8 * step], 32, 0},
+	{0, "1-10:8/12,8-31:24/29,0-31:0/3",	&exp1[9 * step], 32, 0},
+
 	{-EINVAL, "-1",	NULL, 8, 0},
 	{-EINVAL, "-0",	NULL, 8, 0},
 	{-EINVAL, "10-1", NULL, 8, 0},
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/8] lib: bitmap: fold nbits into region struct
  2021-02-21  8:08 [PATCH v5 0/8] support for bitmap (and hence CPU) list "N" abbreviation Paul Gortmaker
                   ` (2 preceding siblings ...)
  2021-02-21  8:08 ` [PATCH 3/8] lib: test_bitmap: add more start-end:offset/len tests Paul Gortmaker
@ 2021-02-21  8:08 ` Paul Gortmaker
  2021-02-21  8:08 ` [PATCH 5/8] lib: bitmap: move ERANGE check from set_region to check_region Paul Gortmaker
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Paul Gortmaker @ 2021-02-21  8:08 UTC (permalink / raw)
  To: linux-kernel
  Cc: Li Zefan, Ingo Molnar, Yury Norov, Thomas Gleixner,
	Josh Triplett, Peter Zijlstra, Paul E. McKenney,
	Frederic Weisbecker, Rasmus Villemoes, Andy Shevchenko,
	Paul Gortmaker

This will reduce parameter passing and enable using nbits as part
of future dynamic region parameter parsing.

Cc: Yury Norov <yury.norov@gmail.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Suggested-by: Yury Norov <yury.norov@gmail.com>
Acked-by: Yury Norov <yury.norov@gmail.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 lib/bitmap.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/lib/bitmap.c b/lib/bitmap.c
index 75006c4036e9..162e2850c622 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -487,24 +487,24 @@ EXPORT_SYMBOL(bitmap_print_to_pagebuf);
 
 /*
  * Region 9-38:4/10 describes the following bitmap structure:
- * 0	   9  12    18			38
- * .........****......****......****......
- *	    ^  ^     ^			 ^
- *      start  off   group_len	       end
+ * 0	   9  12    18			38	     N
+ * .........****......****......****..................
+ *	    ^  ^     ^			 ^	     ^
+ *      start  off   group_len	       end	 nbits
  */
 struct region {
 	unsigned int start;
 	unsigned int off;
 	unsigned int group_len;
 	unsigned int end;
+	unsigned int nbits;
 };
 
-static int bitmap_set_region(const struct region *r,
-				unsigned long *bitmap, int nbits)
+static int bitmap_set_region(const struct region *r, unsigned long *bitmap)
 {
 	unsigned int start;
 
-	if (r->end >= nbits)
+	if (r->end >= r->nbits)
 		return -ERANGE;
 
 	for (start = r->start; start <= r->end; start += r->group_len)
@@ -640,7 +640,8 @@ int bitmap_parselist(const char *buf, unsigned long *maskp, int nmaskbits)
 	struct region r;
 	long ret;
 
-	bitmap_zero(maskp, nmaskbits);
+	r.nbits = nmaskbits;
+	bitmap_zero(maskp, r.nbits);
 
 	while (buf) {
 		buf = bitmap_find_region(buf);
@@ -655,7 +656,7 @@ int bitmap_parselist(const char *buf, unsigned long *maskp, int nmaskbits)
 		if (ret)
 			return ret;
 
-		ret = bitmap_set_region(&r, maskp, nmaskbits);
+		ret = bitmap_set_region(&r, maskp);
 		if (ret)
 			return ret;
 	}
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5/8] lib: bitmap: move ERANGE check from set_region to check_region
  2021-02-21  8:08 [PATCH v5 0/8] support for bitmap (and hence CPU) list "N" abbreviation Paul Gortmaker
                   ` (3 preceding siblings ...)
  2021-02-21  8:08 ` [PATCH 4/8] lib: bitmap: fold nbits into region struct Paul Gortmaker
@ 2021-02-21  8:08 ` Paul Gortmaker
  2021-02-21  8:08 ` [PATCH 6/8] lib: bitmap: support "N" as an alias for size of bitmap Paul Gortmaker
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Paul Gortmaker @ 2021-02-21  8:08 UTC (permalink / raw)
  To: linux-kernel
  Cc: Li Zefan, Ingo Molnar, Yury Norov, Thomas Gleixner,
	Josh Triplett, Peter Zijlstra, Paul E. McKenney,
	Frederic Weisbecker, Rasmus Villemoes, Andy Shevchenko,
	Paul Gortmaker

It makes sense to do all the checks in check_region() and not 1/2
in check_region and 1/2 in set_region.

Since set_region is called immediately after check_region, the net
effect on runtime is zero, but it gets rid of an if (...) return...

Cc: Yury Norov <yury.norov@gmail.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Yury Norov <yury.norov@gmail.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 lib/bitmap.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/lib/bitmap.c b/lib/bitmap.c
index 162e2850c622..833f152a2c43 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -500,17 +500,12 @@ struct region {
 	unsigned int nbits;
 };
 
-static int bitmap_set_region(const struct region *r, unsigned long *bitmap)
+static void bitmap_set_region(const struct region *r, unsigned long *bitmap)
 {
 	unsigned int start;
 
-	if (r->end >= r->nbits)
-		return -ERANGE;
-
 	for (start = r->start; start <= r->end; start += r->group_len)
 		bitmap_set(bitmap, start, min(r->end - start + 1, r->off));
-
-	return 0;
 }
 
 static int bitmap_check_region(const struct region *r)
@@ -518,6 +513,9 @@ static int bitmap_check_region(const struct region *r)
 	if (r->start > r->end || r->group_len == 0 || r->off > r->group_len)
 		return -EINVAL;
 
+	if (r->end >= r->nbits)
+		return -ERANGE;
+
 	return 0;
 }
 
@@ -656,9 +654,7 @@ int bitmap_parselist(const char *buf, unsigned long *maskp, int nmaskbits)
 		if (ret)
 			return ret;
 
-		ret = bitmap_set_region(&r, maskp);
-		if (ret)
-			return ret;
+		bitmap_set_region(&r, maskp);
 	}
 
 	return 0;
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 6/8] lib: bitmap: support "N" as an alias for size of bitmap
  2021-02-21  8:08 [PATCH v5 0/8] support for bitmap (and hence CPU) list "N" abbreviation Paul Gortmaker
                   ` (4 preceding siblings ...)
  2021-02-21  8:08 ` [PATCH 5/8] lib: bitmap: move ERANGE check from set_region to check_region Paul Gortmaker
@ 2021-02-21  8:08 ` Paul Gortmaker
  2021-02-21  8:08 ` [PATCH 7/8] lib: test_bitmap: add tests for "N" alias Paul Gortmaker
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Paul Gortmaker @ 2021-02-21  8:08 UTC (permalink / raw)
  To: linux-kernel
  Cc: Li Zefan, Ingo Molnar, Yury Norov, Thomas Gleixner,
	Josh Triplett, Peter Zijlstra, Paul E. McKenney,
	Frederic Weisbecker, Rasmus Villemoes, Andy Shevchenko,
	Paul Gortmaker

While this is done for all bitmaps, the original use case in mind was
for CPU masks and cpulist_parse() as described below.

It seems that a common configuration is to use the 1st couple cores for
housekeeping tasks.  This tends to leave the remaining ones to form a
pool of similarly configured cores to take on the real workload of
interest to the user.

So on machine A - with 32 cores, it could be 0-3 for "system" and then
4-31 being used in boot args like nohz_full=, or rcu_nocbs= as part of
setting up the worker pool of CPUs.

But then newer machine B is added, and it has 48 cores, and so while
the 0-3 part remains unchanged, the pool setup cpu list becomes 4-47.

Multiple deployment becomes easier when we can just simply replace 31
and 47 with "N" and let the system substitute in the actual number at
boot; a number that it knows better than we do.

Cc: Yury Norov <yury.norov@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Suggested-by: Yury Norov <yury.norov@gmail.com> # move it from CPU code
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 .../admin-guide/kernel-parameters.rst         |  7 ++++++
 lib/bitmap.c                                  | 22 ++++++++++++++-----
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.rst b/Documentation/admin-guide/kernel-parameters.rst
index 682ab28b5c94..7733a773f5f8 100644
--- a/Documentation/admin-guide/kernel-parameters.rst
+++ b/Documentation/admin-guide/kernel-parameters.rst
@@ -68,6 +68,13 @@ For example one can add to the command line following parameter:
 
 where the final item represents CPUs 100,101,125,126,150,151,...
 
+The value "N" can be used to represent the numerically last CPU on the system,
+i.e "foo_cpus=16-N" would be equivalent to "16-31" on a 32 core system.
+
+Keep in mind that "N" is dynamic, so if system changes cause the bitmap width
+to change, such as less cores in the CPU list, then N and any ranges using N
+will also change.  Use the same on a small 4 core system, and "16-N" becomes
+"16-3" and now the same boot input will be flagged as invalid (start > end).
 
 
 This document may not be entirely up to date and comprehensive. The command
diff --git a/lib/bitmap.c b/lib/bitmap.c
index 833f152a2c43..9f4626a4c95f 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -519,11 +519,17 @@ static int bitmap_check_region(const struct region *r)
 	return 0;
 }
 
-static const char *bitmap_getnum(const char *str, unsigned int *num)
+static const char *bitmap_getnum(const char *str, unsigned int *num,
+				 unsigned int lastbit)
 {
 	unsigned long long n;
 	unsigned int len;
 
+	if (str[0] == 'N') {
+		*num = lastbit;
+		return str + 1;
+	}
+
 	len = _parse_integer(str, 10, &n);
 	if (!len)
 		return ERR_PTR(-EINVAL);
@@ -571,7 +577,9 @@ static const char *bitmap_find_region_reverse(const char *start, const char *end
 
 static const char *bitmap_parse_region(const char *str, struct region *r)
 {
-	str = bitmap_getnum(str, &r->start);
+	unsigned int lastbit = r->nbits - 1;
+
+	str = bitmap_getnum(str, &r->start, lastbit);
 	if (IS_ERR(str))
 		return str;
 
@@ -581,7 +589,7 @@ static const char *bitmap_parse_region(const char *str, struct region *r)
 	if (*str != '-')
 		return ERR_PTR(-EINVAL);
 
-	str = bitmap_getnum(str + 1, &r->end);
+	str = bitmap_getnum(str + 1, &r->end, lastbit);
 	if (IS_ERR(str))
 		return str;
 
@@ -591,14 +599,14 @@ static const char *bitmap_parse_region(const char *str, struct region *r)
 	if (*str != ':')
 		return ERR_PTR(-EINVAL);
 
-	str = bitmap_getnum(str + 1, &r->off);
+	str = bitmap_getnum(str + 1, &r->off, lastbit);
 	if (IS_ERR(str))
 		return str;
 
 	if (*str != '/')
 		return ERR_PTR(-EINVAL);
 
-	return bitmap_getnum(str + 1, &r->group_len);
+	return bitmap_getnum(str + 1, &r->group_len, lastbit);
 
 no_end:
 	r->end = r->start;
@@ -625,6 +633,10 @@ static const char *bitmap_parse_region(const char *str, struct region *r)
  * From each group will be used only defined amount of bits.
  * Syntax: range:used_size/group_size
  * Example: 0-1023:2/256 ==> 0,1,256,257,512,513,768,769
+ * The value 'N' can be used as a dynamically substituted token for the
+ * maximum allowed value; i.e (nmaskbits - 1).  Keep in mind that it is
+ * dynamic, so if system changes cause the bitmap width to change, such
+ * as more cores in a CPU list, then any ranges using N will also change.
  *
  * Returns: 0 on success, -errno on invalid input strings. Error values:
  *
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 7/8] lib: test_bitmap: add tests for "N" alias
  2021-02-21  8:08 [PATCH v5 0/8] support for bitmap (and hence CPU) list "N" abbreviation Paul Gortmaker
                   ` (5 preceding siblings ...)
  2021-02-21  8:08 ` [PATCH 6/8] lib: bitmap: support "N" as an alias for size of bitmap Paul Gortmaker
@ 2021-02-21  8:08 ` Paul Gortmaker
  2021-02-21  8:08 ` [PATCH 8/8] rcu: deprecate "all" option to rcu_nocbs= Paul Gortmaker
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Paul Gortmaker @ 2021-02-21  8:08 UTC (permalink / raw)
  To: linux-kernel
  Cc: Li Zefan, Ingo Molnar, Yury Norov, Thomas Gleixner,
	Josh Triplett, Peter Zijlstra, Paul E. McKenney,
	Frederic Weisbecker, Rasmus Villemoes, Andy Shevchenko,
	Paul Gortmaker

These are copies of existing tests, with just 31 --> N.  This ensures
the recently added "N" alias transparently works in any normally
numeric fields of a region specification.

Cc: Yury Norov <yury.norov@gmail.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 lib/test_bitmap.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
index 9c6a88c480c1..a6048278d027 100644
--- a/lib/test_bitmap.c
+++ b/lib/test_bitmap.c
@@ -354,6 +354,16 @@ static const struct test_bitmap_parselist parselist_tests[] __initconst = {
 	{0, "16-31:16/31",		&exp1[3 * step], 32, 0},
 	{0, "31-31:31/31",		&exp1[14 * step], 32, 0},
 
+	{0, "N-N",			&exp1[14 * step], 32, 0},
+	{0, "0-0:1/N",			&exp1[0], 32, 0},
+	{0, "0-0:N/N",			&exp1[0], 32, 0},
+	{0, "0-15:16/N",		&exp1[2 * step], 32, 0},
+	{0, "15-15:N/N",		&exp1[13 * step], 32, 0},
+	{0, "15-N:1/N",			&exp1[13 * step], 32, 0},
+	{0, "16-N:16/N",		&exp1[3 * step], 32, 0},
+	{0, "N-N:N/N",			&exp1[14 * step], 32, 0},
+
+	{0, "0-N:1/3,1-N:1/3,2-N:1/3",		&exp1[8 * step], 32, 0},
 	{0, "0-31:1/3,1-31:1/3,2-31:1/3",	&exp1[8 * step], 32, 0},
 	{0, "1-10:8/12,8-31:24/29,0-31:0/3",	&exp1[9 * step], 32, 0},
 
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 8/8] rcu: deprecate "all" option to rcu_nocbs=
  2021-02-21  8:08 [PATCH v5 0/8] support for bitmap (and hence CPU) list "N" abbreviation Paul Gortmaker
                   ` (6 preceding siblings ...)
  2021-02-21  8:08 ` [PATCH 7/8] lib: test_bitmap: add tests for "N" alias Paul Gortmaker
@ 2021-02-21  8:08 ` Paul Gortmaker
  2021-02-22 23:43 ` [PATCH v5 0/8] support for bitmap (and hence CPU) list "N" abbreviation Paul E. McKenney
  2021-02-23  1:05 ` Yury Norov
  9 siblings, 0 replies; 14+ messages in thread
From: Paul Gortmaker @ 2021-02-21  8:08 UTC (permalink / raw)
  To: linux-kernel
  Cc: Li Zefan, Ingo Molnar, Yury Norov, Thomas Gleixner,
	Josh Triplett, Peter Zijlstra, Paul E. McKenney,
	Frederic Weisbecker, Rasmus Villemoes, Andy Shevchenko,
	Paul Gortmaker

With the core bitmap support now accepting "N" as a placeholder for
the end of the bitmap, "all" can be represented as "0-N" and has the
advantage of not being specific to RCU (or any other subsystem).

So deprecate the use of "all" by removing documentation references
to it.  The support itself needs to remain for now, since we don't
know how many people out there are using it currently, but since it
is in an __init area anyway, it isn't worth losing sleep over.

Cc: Yury Norov <yury.norov@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Josh Triplett <josh@joshtriplett.org>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 4 +---
 kernel/rcu/tree_plugin.h                        | 6 ++----
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index a10b545c2070..a116c0ff0a91 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4037,9 +4037,7 @@
 				see CONFIG_RAS_CEC help text.
 
 	rcu_nocbs=	[KNL]
-			The argument is a cpu list, as described above,
-			except that the string "all" can be used to
-			specify every CPU on the system.
+			The argument is a cpu list, as described above.
 
 			In kernels built with CONFIG_RCU_NOCB_CPU=y, set
 			the specified list of CPUs to be no-callback CPUs.
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 7e291ce0a1d6..56788dfde922 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -1463,14 +1463,12 @@ static void rcu_cleanup_after_idle(void)
 
 /*
  * Parse the boot-time rcu_nocb_mask CPU list from the kernel parameters.
- * The string after the "rcu_nocbs=" is either "all" for all CPUs, or a
- * comma-separated list of CPUs and/or CPU ranges.  If an invalid list is
- * given, a warning is emitted and all CPUs are offloaded.
+ * If the list is invalid, a warning is emitted and all CPUs are offloaded.
  */
 static int __init rcu_nocb_setup(char *str)
 {
 	alloc_bootmem_cpumask_var(&rcu_nocb_mask);
-	if (!strcasecmp(str, "all"))
+	if (!strcasecmp(str, "all"))		/* legacy: use "0-N" instead */
 		cpumask_setall(rcu_nocb_mask);
 	else
 		if (cpulist_parse(str, rcu_nocb_mask)) {
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v5 0/8] support for bitmap (and hence CPU) list "N" abbreviation
  2021-02-21  8:08 [PATCH v5 0/8] support for bitmap (and hence CPU) list "N" abbreviation Paul Gortmaker
                   ` (7 preceding siblings ...)
  2021-02-21  8:08 ` [PATCH 8/8] rcu: deprecate "all" option to rcu_nocbs= Paul Gortmaker
@ 2021-02-22 23:43 ` Paul E. McKenney
  2021-02-23  1:05 ` Yury Norov
  9 siblings, 0 replies; 14+ messages in thread
From: Paul E. McKenney @ 2021-02-22 23:43 UTC (permalink / raw)
  To: Paul Gortmaker
  Cc: linux-kernel, Li Zefan, Ingo Molnar, Yury Norov, Thomas Gleixner,
	Josh Triplett, Peter Zijlstra, Frederic Weisbecker,
	Rasmus Villemoes, Andy Shevchenko

On Sun, Feb 21, 2021 at 03:08:19AM -0500, Paul Gortmaker wrote:
> This is the 5th and final version of this series.  We got some good
> improvements, like adding self-tests, using "N" as "just another number"
> that could be used anywhere, and making things not CPU specific.
> 
> But now it is time to close this review out since is down to just
> hand-wringing over hypothetical use cases, bikeshedding on upper/lower
> case, and a wild goose chase on trying to avoid adding a function arg.
> 
> So, once again - thanks to all who provided input; it was all considered
> even if not all of it was used.  And in that vein, just to be clear:
> 
> 1) There will be no adaptive modifying or guessing what the user meant if
> a range turns out to be invalid.  The caller will be responsible for
> handling the -EINVAL just as things are currently today.
> 
> 2) There will be no use of "L" or lower case "n" because there is simply
> no need for it.  Yes, it would be simple enough to add, but it complicates
> things and would also be impossible to remove later, once it went mainline.

Queued and pushed, thank you!!!

							Thanx, Paul

> The original text from v4 follows:
> 
> The basic objective here was to add support for "nohz_full=8-N" and/or
> "rcu_nocbs="4-N" -- essentially introduce "N" as a portable reference
> to the last core, evaluated at boot for anything using a CPU list.
> 
> The thinking behind this, is that people carve off a few early CPUs to
> support housekeeping tasks, and perhaps dedicate one to a busy I/O
> peripheral, and then the remaining pool of CPUs out to the end are a
> part of a commonly configured pool used for the real work the user
> cares about.
> 
> Extend that logic out to a fleet of machines - some new, and some
> nearing EOL, and you've probably got a wide range of core counts to
> contend with - even though the early number of cores dedicated to the
> system overhead probably doesn't vary.
> 
> This change would enable sysadmins to have a common bootarg across all
> such systems, and would also avoid any off-by-one fencepost errors that
> happen for users who might briefly forget that core counts start at zero.
> 
> Originally I did this at the CPU subsys level, but Yury suggested it
> be moved down further to bitmap level itself, which made the core 
> implementation smaller and less complex, but the series longer.
> 
> New self tests are added to better exercise what bitmap range/region
> currently supports, and new tests are added for the new "N" support.
> 
> Also tested boot arg and the post-boot cgroup use case as per below:
> 
>    root@hackbox:~# cat /proc/cmdline 
>    BOOT_IMAGE=/boot/bzImage root=/dev/sda1 rcu_nocbs=2,3,8-N:1/2
>    root@hackbox:~# dmesg|grep Offl
>    rcu:     Offload RCU callbacks from CPUs: 2-3,8,10,12,14.
> 
>    root@hackbox:/sys/fs/cgroup/cpuset/foo# cat cpuset.cpus
>    
>    root@hackbox:/sys/fs/cgroup/cpuset/foo# /bin/echo 10-N > cpuset.cpus
>    root@hackbox:/sys/fs/cgroup/cpuset/foo# cat cpuset.cpus
>    10-15
>    root@hackbox:/sys/fs/cgroup/cpuset/foo# /bin/echo N-N:N/N > cpuset.cpus
>    root@hackbox:/sys/fs/cgroup/cpuset/foo# cat cpuset.cpus
>    15
> 
> This was on a 16 core machine with CONFIG_NR_CPUS=16 in .config file.
> 
> Note that "N" is a dynamic quantity, and can change scope if the bitmap
> is changed in size.  So at the risk of stating the obvious, don't use it
> for "burn_eFuse=128-N" or "secure_erase_firmware=32-N" type stuff.
> 
> Paul.
> ---
> 
> [v5: go back to v3 location of "nbits" in region.  Add acks/reviewed.]
> 
> [v4: pair nbits with region, instead of inside it.  Split EINVAL and
>  ERANGE tests.  Don't handle start/end/offset within a macro to
>  abstract away nbits usage.  Added some Reviwed-by/Ack tags.]
>  https://lore.kernel.org/lkml/20210209225907.78405-1-paul.gortmaker@windriver.com/
> 
> [v3: Allow "N" to be used anywhere in the region spec, i.e. "N-N:N/N" vs.
>  just being allowed at end of range like "0-N".  Add new self-tests.  Drop
>  "all" and "none" aliases as redundant and not worth the extra complication. ]
>  https://lore.kernel.org/lkml/20210126171141.122639-1-paul.gortmaker@windriver.com/
> 
> [v2: push code down from cpu subsys to core bitmap code as per
>  Yury's comments.  Change "last" to simply be "N" as per PeterZ.]
>  https://lore.kernel.org/lkml/20210121223355.59780-1-paul.gortmaker@windriver.com/
> 
> [v1: https://lore.kernel.org/lkml/20210106004850.GA11682@paulmck-ThinkPad-P72/
> 
> Cc: Li Zefan <lizefan@huawei.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Yury Norov <yury.norov@gmail.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Josh Triplett <josh@joshtriplett.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: "Paul E. McKenney" <paulmck@kernel.org>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> 
> 
> Paul Gortmaker (8):
>   lib: test_bitmap: clearly separate ERANGE from EINVAL tests.
>   lib: test_bitmap: add tests to trigger ERANGE case.
>   lib: test_bitmap: add more start-end:offset/len tests
>   lib: bitmap: fold nbits into region struct
>   lib: bitmap: move ERANGE check from set_region to check_region
>   lib: bitmap: support "N" as an alias for size of bitmap
>   lib: test_bitmap: add tests for "N" alias
>   rcu: deprecate "all" option to rcu_nocbs=
> 
>  .../admin-guide/kernel-parameters.rst         |  7 +++
>  .../admin-guide/kernel-parameters.txt         |  4 +-
>  kernel/rcu/tree_plugin.h                      |  6 +--
>  lib/bitmap.c                                  | 49 +++++++++++--------
>  lib/test_bitmap.c                             | 46 ++++++++++++++---
>  5 files changed, 79 insertions(+), 33 deletions(-)
> 
> -- 
> 2.30.0
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v5 0/8] support for bitmap (and hence CPU) list "N" abbreviation
  2021-02-21  8:08 [PATCH v5 0/8] support for bitmap (and hence CPU) list "N" abbreviation Paul Gortmaker
                   ` (8 preceding siblings ...)
  2021-02-22 23:43 ` [PATCH v5 0/8] support for bitmap (and hence CPU) list "N" abbreviation Paul E. McKenney
@ 2021-02-23  1:05 ` Yury Norov
  2021-02-24  1:54   ` Paul E. McKenney
  9 siblings, 1 reply; 14+ messages in thread
From: Yury Norov @ 2021-02-23  1:05 UTC (permalink / raw)
  To: Paul Gortmaker
  Cc: linux-kernel, Li Zefan, Ingo Molnar, Thomas Gleixner,
	Josh Triplett, Peter Zijlstra, Paul E. McKenney,
	Frederic Weisbecker, Rasmus Villemoes, Andy Shevchenko

On Sun, Feb 21, 2021 at 03:08:19AM -0500, Paul Gortmaker wrote:
> This is the 5th and final version of this series.  We got some good
> improvements, like adding self-tests, using "N" as "just another number"
> that could be used anywhere, and making things not CPU specific.
> 
> But now it is time to close this review out since is down to just
> hand-wringing over hypothetical use cases, bikeshedding on upper/lower
> case, and a wild goose chase on trying to avoid adding a function arg.
> 
> So, once again - thanks to all who provided input; it was all considered
> even if not all of it was used.  And in that vein, just to be clear:
> 
> 1) There will be no adaptive modifying or guessing what the user meant if
> a range turns out to be invalid.  The caller will be responsible for
> handling the -EINVAL just as things are currently today.
> 
> 2) There will be no use of "L" or lower case "n" because there is simply
> no need for it.  Yes, it would be simple enough to add, but it complicates
> things and would also be impossible to remove later, once it went mainline.
> 
> 
> The original text from v4 follows:
> 
> The basic objective here was to add support for "nohz_full=8-N" and/or
> "rcu_nocbs="4-N" -- essentially introduce "N" as a portable reference
> to the last core, evaluated at boot for anything using a CPU list.
> 
> The thinking behind this, is that people carve off a few early CPUs to
> support housekeeping tasks, and perhaps dedicate one to a busy I/O
> peripheral, and then the remaining pool of CPUs out to the end are a
> part of a commonly configured pool used for the real work the user
> cares about.
> 
> Extend that logic out to a fleet of machines - some new, and some
> nearing EOL, and you've probably got a wide range of core counts to
> contend with - even though the early number of cores dedicated to the
> system overhead probably doesn't vary.
> 
> This change would enable sysadmins to have a common bootarg across all
> such systems, and would also avoid any off-by-one fencepost errors that
> happen for users who might briefly forget that core counts start at zero.
> 
> Originally I did this at the CPU subsys level, but Yury suggested it
> be moved down further to bitmap level itself, which made the core 
> implementation smaller and less complex, but the series longer.
> 
> New self tests are added to better exercise what bitmap range/region
> currently supports, and new tests are added for the new "N" support.
> 
> Also tested boot arg and the post-boot cgroup use case as per below:
> 
>    root@hackbox:~# cat /proc/cmdline 
>    BOOT_IMAGE=/boot/bzImage root=/dev/sda1 rcu_nocbs=2,3,8-N:1/2
>    root@hackbox:~# dmesg|grep Offl
>    rcu:     Offload RCU callbacks from CPUs: 2-3,8,10,12,14.
> 
>    root@hackbox:/sys/fs/cgroup/cpuset/foo# cat cpuset.cpus
>    
>    root@hackbox:/sys/fs/cgroup/cpuset/foo# /bin/echo 10-N > cpuset.cpus
>    root@hackbox:/sys/fs/cgroup/cpuset/foo# cat cpuset.cpus
>    10-15
>    root@hackbox:/sys/fs/cgroup/cpuset/foo# /bin/echo N-N:N/N > cpuset.cpus
>    root@hackbox:/sys/fs/cgroup/cpuset/foo# cat cpuset.cpus
>    15
> 
> This was on a 16 core machine with CONFIG_NR_CPUS=16 in .config file.
> 
> Note that "N" is a dynamic quantity, and can change scope if the bitmap
> is changed in size.  So at the risk of stating the obvious, don't use it
> for "burn_eFuse=128-N" or "secure_erase_firmware=32-N" type stuff.
> 
> Paul.
> ---

Acked-by: Yury Norov <yury.norov@gmail.com>
 
> [v5: go back to v3 location of "nbits" in region.  Add acks/reviewed.]
> 
> [v4: pair nbits with region, instead of inside it.  Split EINVAL and
>  ERANGE tests.  Don't handle start/end/offset within a macro to
>  abstract away nbits usage.  Added some Reviwed-by/Ack tags.]
>  https://lore.kernel.org/lkml/20210209225907.78405-1-paul.gortmaker@windriver.com/
> 
> [v3: Allow "N" to be used anywhere in the region spec, i.e. "N-N:N/N" vs.
>  just being allowed at end of range like "0-N".  Add new self-tests.  Drop
>  "all" and "none" aliases as redundant and not worth the extra complication. ]
>  https://lore.kernel.org/lkml/20210126171141.122639-1-paul.gortmaker@windriver.com/
> 
> [v2: push code down from cpu subsys to core bitmap code as per
>  Yury's comments.  Change "last" to simply be "N" as per PeterZ.]
>  https://lore.kernel.org/lkml/20210121223355.59780-1-paul.gortmaker@windriver.com/
> 
> [v1: https://lore.kernel.org/lkml/20210106004850.GA11682@paulmck-ThinkPad-P72/
> 
> Cc: Li Zefan <lizefan@huawei.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Yury Norov <yury.norov@gmail.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Josh Triplett <josh@joshtriplett.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: "Paul E. McKenney" <paulmck@kernel.org>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> 
> 
> Paul Gortmaker (8):
>   lib: test_bitmap: clearly separate ERANGE from EINVAL tests.
>   lib: test_bitmap: add tests to trigger ERANGE case.
>   lib: test_bitmap: add more start-end:offset/len tests
>   lib: bitmap: fold nbits into region struct
>   lib: bitmap: move ERANGE check from set_region to check_region
>   lib: bitmap: support "N" as an alias for size of bitmap
>   lib: test_bitmap: add tests for "N" alias
>   rcu: deprecate "all" option to rcu_nocbs=
> 
>  .../admin-guide/kernel-parameters.rst         |  7 +++
>  .../admin-guide/kernel-parameters.txt         |  4 +-
>  kernel/rcu/tree_plugin.h                      |  6 +--
>  lib/bitmap.c                                  | 49 +++++++++++--------
>  lib/test_bitmap.c                             | 46 ++++++++++++++---
>  5 files changed, 79 insertions(+), 33 deletions(-)
> 
> -- 
> 2.30.0

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v5 0/8] support for bitmap (and hence CPU) list "N" abbreviation
  2021-02-23  1:05 ` Yury Norov
@ 2021-02-24  1:54   ` Paul E. McKenney
  0 siblings, 0 replies; 14+ messages in thread
From: Paul E. McKenney @ 2021-02-24  1:54 UTC (permalink / raw)
  To: Yury Norov
  Cc: Paul Gortmaker, linux-kernel, Li Zefan, Ingo Molnar,
	Thomas Gleixner, Josh Triplett, Peter Zijlstra,
	Frederic Weisbecker, Rasmus Villemoes, Andy Shevchenko

On Mon, Feb 22, 2021 at 05:05:06PM -0800, Yury Norov wrote:
> On Sun, Feb 21, 2021 at 03:08:19AM -0500, Paul Gortmaker wrote:
> > This is the 5th and final version of this series.  We got some good
> > improvements, like adding self-tests, using "N" as "just another number"
> > that could be used anywhere, and making things not CPU specific.
> > 
> > But now it is time to close this review out since is down to just
> > hand-wringing over hypothetical use cases, bikeshedding on upper/lower
> > case, and a wild goose chase on trying to avoid adding a function arg.
> > 
> > So, once again - thanks to all who provided input; it was all considered
> > even if not all of it was used.  And in that vein, just to be clear:
> > 
> > 1) There will be no adaptive modifying or guessing what the user meant if
> > a range turns out to be invalid.  The caller will be responsible for
> > handling the -EINVAL just as things are currently today.
> > 
> > 2) There will be no use of "L" or lower case "n" because there is simply
> > no need for it.  Yes, it would be simple enough to add, but it complicates
> > things and would also be impossible to remove later, once it went mainline.
> > 
> > 
> > The original text from v4 follows:
> > 
> > The basic objective here was to add support for "nohz_full=8-N" and/or
> > "rcu_nocbs="4-N" -- essentially introduce "N" as a portable reference
> > to the last core, evaluated at boot for anything using a CPU list.
> > 
> > The thinking behind this, is that people carve off a few early CPUs to
> > support housekeeping tasks, and perhaps dedicate one to a busy I/O
> > peripheral, and then the remaining pool of CPUs out to the end are a
> > part of a commonly configured pool used for the real work the user
> > cares about.
> > 
> > Extend that logic out to a fleet of machines - some new, and some
> > nearing EOL, and you've probably got a wide range of core counts to
> > contend with - even though the early number of cores dedicated to the
> > system overhead probably doesn't vary.
> > 
> > This change would enable sysadmins to have a common bootarg across all
> > such systems, and would also avoid any off-by-one fencepost errors that
> > happen for users who might briefly forget that core counts start at zero.
> > 
> > Originally I did this at the CPU subsys level, but Yury suggested it
> > be moved down further to bitmap level itself, which made the core 
> > implementation smaller and less complex, but the series longer.
> > 
> > New self tests are added to better exercise what bitmap range/region
> > currently supports, and new tests are added for the new "N" support.
> > 
> > Also tested boot arg and the post-boot cgroup use case as per below:
> > 
> >    root@hackbox:~# cat /proc/cmdline 
> >    BOOT_IMAGE=/boot/bzImage root=/dev/sda1 rcu_nocbs=2,3,8-N:1/2
> >    root@hackbox:~# dmesg|grep Offl
> >    rcu:     Offload RCU callbacks from CPUs: 2-3,8,10,12,14.
> > 
> >    root@hackbox:/sys/fs/cgroup/cpuset/foo# cat cpuset.cpus
> >    
> >    root@hackbox:/sys/fs/cgroup/cpuset/foo# /bin/echo 10-N > cpuset.cpus
> >    root@hackbox:/sys/fs/cgroup/cpuset/foo# cat cpuset.cpus
> >    10-15
> >    root@hackbox:/sys/fs/cgroup/cpuset/foo# /bin/echo N-N:N/N > cpuset.cpus
> >    root@hackbox:/sys/fs/cgroup/cpuset/foo# cat cpuset.cpus
> >    15
> > 
> > This was on a 16 core machine with CONFIG_NR_CPUS=16 in .config file.
> > 
> > Note that "N" is a dynamic quantity, and can change scope if the bitmap
> > is changed in size.  So at the risk of stating the obvious, don't use it
> > for "burn_eFuse=128-N" or "secure_erase_firmware=32-N" type stuff.
> > 
> > Paul.
> > ---
> 
> Acked-by: Yury Norov <yury.norov@gmail.com>

Applied, thank you all!

							Thanx, Paul

> > [v5: go back to v3 location of "nbits" in region.  Add acks/reviewed.]
> > 
> > [v4: pair nbits with region, instead of inside it.  Split EINVAL and
> >  ERANGE tests.  Don't handle start/end/offset within a macro to
> >  abstract away nbits usage.  Added some Reviwed-by/Ack tags.]
> >  https://lore.kernel.org/lkml/20210209225907.78405-1-paul.gortmaker@windriver.com/
> > 
> > [v3: Allow "N" to be used anywhere in the region spec, i.e. "N-N:N/N" vs.
> >  just being allowed at end of range like "0-N".  Add new self-tests.  Drop
> >  "all" and "none" aliases as redundant and not worth the extra complication. ]
> >  https://lore.kernel.org/lkml/20210126171141.122639-1-paul.gortmaker@windriver.com/
> > 
> > [v2: push code down from cpu subsys to core bitmap code as per
> >  Yury's comments.  Change "last" to simply be "N" as per PeterZ.]
> >  https://lore.kernel.org/lkml/20210121223355.59780-1-paul.gortmaker@windriver.com/
> > 
> > [v1: https://lore.kernel.org/lkml/20210106004850.GA11682@paulmck-ThinkPad-P72/
> > 
> > Cc: Li Zefan <lizefan@huawei.com>
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Cc: Yury Norov <yury.norov@gmail.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Josh Triplett <josh@joshtriplett.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: "Paul E. McKenney" <paulmck@kernel.org>
> > Cc: Frederic Weisbecker <fweisbec@gmail.com>
> > Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > 
> > 
> > 
> > Paul Gortmaker (8):
> >   lib: test_bitmap: clearly separate ERANGE from EINVAL tests.
> >   lib: test_bitmap: add tests to trigger ERANGE case.
> >   lib: test_bitmap: add more start-end:offset/len tests
> >   lib: bitmap: fold nbits into region struct
> >   lib: bitmap: move ERANGE check from set_region to check_region
> >   lib: bitmap: support "N" as an alias for size of bitmap
> >   lib: test_bitmap: add tests for "N" alias
> >   rcu: deprecate "all" option to rcu_nocbs=
> > 
> >  .../admin-guide/kernel-parameters.rst         |  7 +++
> >  .../admin-guide/kernel-parameters.txt         |  4 +-
> >  kernel/rcu/tree_plugin.h                      |  6 +--
> >  lib/bitmap.c                                  | 49 +++++++++++--------
> >  lib/test_bitmap.c                             | 46 ++++++++++++++---
> >  5 files changed, 79 insertions(+), 33 deletions(-)
> > 
> > -- 
> > 2.30.0

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 7/8] lib: test_bitmap: add tests for "N" alias
  2021-02-09 22:58 [PATCH v4 " Paul Gortmaker
@ 2021-02-09 22:59 ` Paul Gortmaker
  0 siblings, 0 replies; 14+ messages in thread
From: Paul Gortmaker @ 2021-02-09 22:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Li Zefan, Ingo Molnar, Yury Norov, Thomas Gleixner,
	Josh Triplett, Peter Zijlstra, Paul E. McKenney,
	Frederic Weisbecker, Rasmus Villemoes, Andy Shevchenko,
	Paul Gortmaker

These are copies of existing tests, with just 31 --> N.  This ensures
the recently added "N" alias transparently works in any normally
numeric fields of a region specification.

Cc: Yury Norov <yury.norov@gmail.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 lib/test_bitmap.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
index 9c6a88c480c1..a6048278d027 100644
--- a/lib/test_bitmap.c
+++ b/lib/test_bitmap.c
@@ -354,6 +354,16 @@ static const struct test_bitmap_parselist parselist_tests[] __initconst = {
 	{0, "16-31:16/31",		&exp1[3 * step], 32, 0},
 	{0, "31-31:31/31",		&exp1[14 * step], 32, 0},
 
+	{0, "N-N",			&exp1[14 * step], 32, 0},
+	{0, "0-0:1/N",			&exp1[0], 32, 0},
+	{0, "0-0:N/N",			&exp1[0], 32, 0},
+	{0, "0-15:16/N",		&exp1[2 * step], 32, 0},
+	{0, "15-15:N/N",		&exp1[13 * step], 32, 0},
+	{0, "15-N:1/N",			&exp1[13 * step], 32, 0},
+	{0, "16-N:16/N",		&exp1[3 * step], 32, 0},
+	{0, "N-N:N/N",			&exp1[14 * step], 32, 0},
+
+	{0, "0-N:1/3,1-N:1/3,2-N:1/3",		&exp1[8 * step], 32, 0},
 	{0, "0-31:1/3,1-31:1/3,2-31:1/3",	&exp1[8 * step], 32, 0},
 	{0, "1-10:8/12,8-31:24/29,0-31:0/3",	&exp1[9 * step], 32, 0},
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 7/8] lib: test_bitmap: add tests for "N" alias
  2021-01-26 17:11 [PATCH v3 0/8] support for bitmap (and hence CPU) list "N" abbreviation Paul Gortmaker
@ 2021-01-26 17:11 ` Paul Gortmaker
  0 siblings, 0 replies; 14+ messages in thread
From: Paul Gortmaker @ 2021-01-26 17:11 UTC (permalink / raw)
  To: linux-kernel
  Cc: lizefan, mingo, tglx, josh, yury.norov, peterz, paulmck,
	fweisbec, linux, andriy.shevchenko, Paul Gortmaker

These are copies of existing tests, with just 31 --> N.  This ensures
the recently added "N" alias transparently works in any normally
numeric fields of a region specification.

Cc: Yury Norov <yury.norov@gmail.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 lib/test_bitmap.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
index 807d1e8dd59c..2bcea2517c03 100644
--- a/lib/test_bitmap.c
+++ b/lib/test_bitmap.c
@@ -354,6 +354,16 @@ static const struct test_bitmap_parselist parselist_tests[] __initconst = {
 	{0, "16-31:16/31",		&exp1[3 * step], 32, 0},
 	{0, "31-31:31/31",		&exp1[14 * step], 32, 0},
 
+	{0, "N-N",			&exp1[14 * step], 32, 0},
+	{0, "0-0:1/N",			&exp1[0], 32, 0},
+	{0, "0-0:N/N",			&exp1[0], 32, 0},
+	{0, "0-15:16/N",		&exp1[2 * step], 32, 0},
+	{0, "15-15:N/N",		&exp1[13 * step], 32, 0},
+	{0, "15-N:1/N",			&exp1[13 * step], 32, 0},
+	{0, "16-N:16/N",		&exp1[3 * step], 32, 0},
+	{0, "N-N:N/N",			&exp1[14 * step], 32, 0},
+
+	{0, "0-N:1/3,1-N:1/3,2-N:1/3",		&exp1[8 * step], 32, 0},
 	{0, "0-31:1/3,1-31:1/3,2-31:1/3",	&exp1[8 * step], 32, 0},
 	{0, "1-10:8/12,8-31:24/29,0-31:0/3",	&exp1[9 * step], 32, 0},
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-02-24  1:55 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-21  8:08 [PATCH v5 0/8] support for bitmap (and hence CPU) list "N" abbreviation Paul Gortmaker
2021-02-21  8:08 ` [PATCH 1/8] lib: test_bitmap: clearly separate ERANGE from EINVAL tests Paul Gortmaker
2021-02-21  8:08 ` [PATCH 2/8] lib: test_bitmap: add tests to trigger ERANGE case Paul Gortmaker
2021-02-21  8:08 ` [PATCH 3/8] lib: test_bitmap: add more start-end:offset/len tests Paul Gortmaker
2021-02-21  8:08 ` [PATCH 4/8] lib: bitmap: fold nbits into region struct Paul Gortmaker
2021-02-21  8:08 ` [PATCH 5/8] lib: bitmap: move ERANGE check from set_region to check_region Paul Gortmaker
2021-02-21  8:08 ` [PATCH 6/8] lib: bitmap: support "N" as an alias for size of bitmap Paul Gortmaker
2021-02-21  8:08 ` [PATCH 7/8] lib: test_bitmap: add tests for "N" alias Paul Gortmaker
2021-02-21  8:08 ` [PATCH 8/8] rcu: deprecate "all" option to rcu_nocbs= Paul Gortmaker
2021-02-22 23:43 ` [PATCH v5 0/8] support for bitmap (and hence CPU) list "N" abbreviation Paul E. McKenney
2021-02-23  1:05 ` Yury Norov
2021-02-24  1:54   ` Paul E. McKenney
  -- strict thread matches above, loose matches on Subject: below --
2021-02-09 22:58 [PATCH v4 " Paul Gortmaker
2021-02-09 22:59 ` [PATCH 7/8] lib: test_bitmap: add tests for "N" alias Paul Gortmaker
2021-01-26 17:11 [PATCH v3 0/8] support for bitmap (and hence CPU) list "N" abbreviation Paul Gortmaker
2021-01-26 17:11 ` [PATCH 7/8] lib: test_bitmap: add tests for "N" alias Paul Gortmaker

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).