[PATCH RFC cpumask] Allow "all", "none", and "last" in cpumask strings

[PATCH RFC cpumask] Allow "all", "none", and "last" in cpumask strings

[Paul E. McKenney]

Hello!

This series allows "all", "none", and "last" to be used in cpumask
strings.  This allows these strings to be less dependent on the underlying
system.  For example, currently a string specifying all but the first
CPU must be "1-7" on an eight-CPU system and "1-15" on a 16-CPU system.
With this series, the single string "1-last" can be used regardless of the
number of CPUs (at least assuming that each system has at least one CPU).

1.	Un-inline cpulist_parse for SMP; prepare for ascii helpers,
	courtesy of Paul Gortmaker.

2.	Make "all" alias global and not just RCU, courtesy of Paul
	Gortmaker.

3.	Add a "none" alias to complement "all", courtesy of Paul
	Gortmaker.

4.	Add "last" alias for cpu list specifications, courtesy of Paul
	Gortmaker.

5.	Use "all" and "last" in "nohz_full" and "rcu_nocbs".

						Thanx, Paul

------------------------------------------------------------------------

 Documentation/admin-guide/kernel-parameters.rst            |   20 +
 Documentation/admin-guide/kernel-parameters.txt            |    4 
 include/linux/cpumask.h                                    |    8 
 kernel/rcu/tree_plugin.h                                   |   13 -
 lib/cpumask.c                                              |  136 ++++++++++++-
 tools/testing/selftests/rcutorture/configs/rcu/TREE04.boot |    2 
 tools/testing/selftests/rcutorture/configs/rcu/TREE08.boot |    2 
 7 files changed, 169 insertions(+), 16 deletions(-)

[PATCH RFC cpumask 1/5] cpumask: Un-inline cpulist_parse for SMP; prepare for ascii helpers

[paulmck]

From: Paul Gortmaker <paul.gortmaker@windriver.com>

In order to support convenience tokens like "all", and "none" and
"last" in CPU lists, we'll have to use string operations and expand
on what is currently a simple wrapper around the underlying bitmap
function call.

Rather than add header dependencies to cpumask.h and code more complex
operations not really appropriate for a header file, we prepare by
simply un-inlining it here and move it to the lib dir alongside the
other more complex cpumask functions.

Since lib/cpumask.c is built conditionally on CONFIG_SMP, and there
are non-SMP callers, we leave the one-line stub behind for that case.
If they want to check "0-0" is a valid range, they can still do it.
In the meantime, we can add the ascii helpers for CONFIG_SMP users.
The use of NR_CPUS vs. CONFIG_SMP is consistent with the existing file.

Aside from an additional exported symbol in the SMP case, no functional
changes are anticipated with this move.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
---
 include/linux/cpumask.h |  8 ++++++++
 lib/cpumask.c           | 13 +++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index 383684e..1f506e2 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -685,11 +685,19 @@ static inline int cpumask_parse(const char *buf, struct cpumask *dstp)
  * @dstp: the cpumask to set.
  *
  * Returns -errno, or 0 for success.
+ *
+ * There are instances of non-SMP callers of this, and the easiest way
+ * to remain 100% runtime compatible is to let them continue to have the
+ * one-line stub, while the SMP version in lib/cpumask.c gets improved.
  */
+#if NR_CPUS == 1
 static inline int cpulist_parse(const char *buf, struct cpumask *dstp)
 {
 	return bitmap_parselist(buf, cpumask_bits(dstp), nr_cpumask_bits);
 }
+#else
+int cpulist_parse(const char *buf, struct cpumask *dstp);
+#endif
 
 /**
  * cpumask_size - size to allocate for a 'struct cpumask' in bytes
diff --git a/lib/cpumask.c b/lib/cpumask.c
index 3592402..6e6e835 100644
--- a/lib/cpumask.c
+++ b/lib/cpumask.c
@@ -95,6 +95,19 @@ int cpumask_next_wrap(int n, const struct cpumask *mask, int start, bool wrap)
 }
 EXPORT_SYMBOL(cpumask_next_wrap);
 
+/**
+ * cpulist_parse - extract a cpumask from a user string of ranges
+ * @buf: the buffer to extract from
+ * @dstp: the cpumask to set.
+ *
+ * Returns -errno, or 0 for success.
+ */
+int cpulist_parse(const char *buf, struct cpumask *dstp)
+{
+	return bitmap_parselist(buf, cpumask_bits(dstp), nr_cpumask_bits);
+}
+EXPORT_SYMBOL(cpulist_parse);
+
 /* These are not inline because of header tangles. */
 #ifdef CONFIG_CPUMASK_OFFSTACK
 /**
-- 
2.9.5

[PATCH RFC cpumask 2/5] cpumask: Make "all" alias global and not just RCU

[paulmck]

From: Paul Gortmaker <paul.gortmaker@windriver.com>

It is probably better that we don't have subsystem specific
abbreviations or aliases for generic CPU list specifications.

Hence we move the "all" from RCU out to lib/ so that it can be
used in any instance where CPU lists are being parsed.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
---
 Documentation/admin-guide/kernel-parameters.rst |  7 +++++++
 Documentation/admin-guide/kernel-parameters.txt |  4 +---
 kernel/rcu/tree_plugin.h                        | 13 ++++---------
 lib/cpumask.c                                   |  6 ++++++
 4 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.rst b/Documentation/admin-guide/kernel-parameters.rst
index 06fb1b4..cdf4e81 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 following convenience aliases are also accepted and used:
+
+        foo_cpus=all
+
+is equivalent to "foo_cpus=0-N" -- where "N" is the numerically last CPU on
+the system, thus avoiding looking up the value in "/sys/devices/system/cpu"
+in advance on each deployed system.
 
 
 This document may not be entirely up to date and comprehensive. The command
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index c722ec1..0f4379b 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 7e291ce..642ebd6 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -1463,20 +1463,15 @@ 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 (cpulist_parse(str, rcu_nocb_mask)) {
+		pr_warn("rcu_nocbs= bad CPU range, all CPUs set\n");
 		cpumask_setall(rcu_nocb_mask);
-	else
-		if (cpulist_parse(str, rcu_nocb_mask)) {
-			pr_warn("rcu_nocbs= bad CPU range, all CPUs set\n");
-			cpumask_setall(rcu_nocb_mask);
-		}
+	}
 	return 1;
 }
 __setup("rcu_nocbs=", rcu_nocb_setup);
diff --git a/lib/cpumask.c b/lib/cpumask.c
index 6e6e835..9f8ff72 100644
--- a/lib/cpumask.c
+++ b/lib/cpumask.c
@@ -2,6 +2,7 @@
 #include <linux/slab.h>
 #include <linux/kernel.h>
 #include <linux/bitops.h>
+#include <linux/string.h>
 #include <linux/cpumask.h>
 #include <linux/export.h>
 #include <linux/memblock.h>
@@ -104,6 +105,11 @@ EXPORT_SYMBOL(cpumask_next_wrap);
  */
 int cpulist_parse(const char *buf, struct cpumask *dstp)
 {
+	if (!strcmp(buf, "all")) {
+		cpumask_setall(dstp);
+		return 0;
+	}
+
 	return bitmap_parselist(buf, cpumask_bits(dstp), nr_cpumask_bits);
 }
 EXPORT_SYMBOL(cpulist_parse);
-- 
2.9.5

Re: [PATCH RFC cpumask 2/5] cpumask: Make "all" alias global and not just RCU

[Yury Norov]

Hi Paul,

On Tue, Jan 5, 2021 at 4:49 PM <paulmck@kernel.org> wrote:
>
> From: Paul Gortmaker <paul.gortmaker@windriver.com>
>
> It is probably better that we don't have subsystem specific
> abbreviations or aliases for generic CPU list specifications.
>
> Hence we move the "all" from RCU out to lib/ so that it can be
> used in any instance where CPU lists are being parsed.
>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> ---
>  Documentation/admin-guide/kernel-parameters.rst |  7 +++++++
>  Documentation/admin-guide/kernel-parameters.txt |  4 +---
>  kernel/rcu/tree_plugin.h                        | 13 ++++---------
>  lib/cpumask.c                                   |  6 ++++++
>  4 files changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.rst b/Documentation/admin-guide/kernel-parameters.rst
> index 06fb1b4..cdf4e81 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 following convenience aliases are also accepted and used:
> +
> +        foo_cpus=all
> +
> +is equivalent to "foo_cpus=0-N" -- where "N" is the numerically last CPU on
> +the system, thus avoiding looking up the value in "/sys/devices/system/cpu"
> +in advance on each deployed system.
>
>
>  This document may not be entirely up to date and comprehensive. The command
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index c722ec1..0f4379b 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 7e291ce..642ebd6 100644
> --- a/kernel/rcu/tree_plugin.h
> +++ b/kernel/rcu/tree_plugin.h
> @@ -1463,20 +1463,15 @@ 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 (cpulist_parse(str, rcu_nocb_mask)) {
> +               pr_warn("rcu_nocbs= bad CPU range, all CPUs set\n");
>                 cpumask_setall(rcu_nocb_mask);
> -       else
> -               if (cpulist_parse(str, rcu_nocb_mask)) {
> -                       pr_warn("rcu_nocbs= bad CPU range, all CPUs set\n");
> -                       cpumask_setall(rcu_nocb_mask);
> -               }
> +       }
>         return 1;
>  }
>  __setup("rcu_nocbs=", rcu_nocb_setup);
> diff --git a/lib/cpumask.c b/lib/cpumask.c
> index 6e6e835..9f8ff72 100644
> --- a/lib/cpumask.c
> +++ b/lib/cpumask.c
> @@ -2,6 +2,7 @@
>  #include <linux/slab.h>
>  #include <linux/kernel.h>
>  #include <linux/bitops.h>
> +#include <linux/string.h>
>  #include <linux/cpumask.h>
>  #include <linux/export.h>
>  #include <linux/memblock.h>
> @@ -104,6 +105,11 @@ EXPORT_SYMBOL(cpumask_next_wrap);
>   */
>  int cpulist_parse(const char *buf, struct cpumask *dstp)
>  {
> +       if (!strcmp(buf, "all")) {
> +               cpumask_setall(dstp);
> +               return 0;
> +       }

I like the extensions. Can you move this chunk inside  bitmap_parselist()
to let other users like nodelist_parse() use it?

>         return bitmap_parselist(buf, cpumask_bits(dstp), nr_cpumask_bits);
>  }
>  EXPORT_SYMBOL(cpulist_parse);
> --
> 2.9.5
>

[PATCH RFC cpumask 3/5] cpumask: Add a "none" alias to complement "all"

[paulmck]

From: Paul Gortmaker <paul.gortmaker@windriver.com>

With global support for a CPU list alias of "all", it seems to just make
sense to also trivially extend support for an opposite "none" specifier.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
---
 Documentation/admin-guide/kernel-parameters.rst | 6 ++++++
 lib/cpumask.c                                   | 5 +++++
 2 files changed, 11 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.rst b/Documentation/admin-guide/kernel-parameters.rst
index cdf4e81..7dd1224 100644
--- a/Documentation/admin-guide/kernel-parameters.rst
+++ b/Documentation/admin-guide/kernel-parameters.rst
@@ -76,6 +76,12 @@ is equivalent to "foo_cpus=0-N" -- where "N" is the numerically last CPU on
 the system, thus avoiding looking up the value in "/sys/devices/system/cpu"
 in advance on each deployed system.
 
+        foo_cpus=none
+
+will provide an empty/cleared cpu mask for the associated boot argument.
+
+Note that "all" and "none" are not necessarily valid/sensible input values
+for each available parameter expecting a CPU list.
 
 This document may not be entirely up to date and comprehensive. The command
 "modinfo -p ${modulename}" shows a current list of all parameters of a loadable
diff --git a/lib/cpumask.c b/lib/cpumask.c
index 9f8ff72..7fbcab8 100644
--- a/lib/cpumask.c
+++ b/lib/cpumask.c
@@ -110,6 +110,11 @@ int cpulist_parse(const char *buf, struct cpumask *dstp)
 		return 0;
 	}
 
+	if (!strcmp(buf, "none")) {
+		cpumask_clear(dstp);
+		return 0;
+	}
+
 	return bitmap_parselist(buf, cpumask_bits(dstp), nr_cpumask_bits);
 }
 EXPORT_SYMBOL(cpulist_parse);
-- 
2.9.5

Re: [PATCH RFC cpumask 3/5] cpumask: Add a "none" alias to complement "all"

[Yury Norov]

On Tue, Jan 5, 2021 at 4:49 PM <paulmck@kernel.org> wrote:
>
> From: Paul Gortmaker <paul.gortmaker@windriver.com>
>
> With global support for a CPU list alias of "all", it seems to just make
> sense to also trivially extend support for an opposite "none" specifier.
>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> ---
>  Documentation/admin-guide/kernel-parameters.rst | 6 ++++++
>  lib/cpumask.c                                   | 5 +++++
>  2 files changed, 11 insertions(+)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.rst b/Documentation/admin-guide/kernel-parameters.rst
> index cdf4e81..7dd1224 100644
> --- a/Documentation/admin-guide/kernel-parameters.rst
> +++ b/Documentation/admin-guide/kernel-parameters.rst
> @@ -76,6 +76,12 @@ is equivalent to "foo_cpus=0-N" -- where "N" is the numerically last CPU on
>  the system, thus avoiding looking up the value in "/sys/devices/system/cpu"
>  in advance on each deployed system.
>
> +        foo_cpus=none
> +
> +will provide an empty/cleared cpu mask for the associated boot argument.
> +
> +Note that "all" and "none" are not necessarily valid/sensible input values
> +for each available parameter expecting a CPU list.
>
>  This document may not be entirely up to date and comprehensive. The command
>  "modinfo -p ${modulename}" shows a current list of all parameters of a loadable
> diff --git a/lib/cpumask.c b/lib/cpumask.c
> index 9f8ff72..7fbcab8 100644
> --- a/lib/cpumask.c
> +++ b/lib/cpumask.c
> @@ -110,6 +110,11 @@ int cpulist_parse(const char *buf, struct cpumask *dstp)
>                 return 0;
>         }
>
> +       if (!strcmp(buf, "none")) {
> +               cpumask_clear(dstp);
> +               return 0;
> +       }

Same comment as to the patch 2. Also, what if a user wants to stack ranges
like 'all, 1-3, none'? As far as I understand current implementation,
cpu_parselist
will pass new keywords to bitmap_parselist() which will fail to parse it.

If you think of new extensions as special case keywords which should not be
mixed with traditional region descriptors, I'm OK with that. But it should be
explained in documentation. I think it's better to handle 'all' and 'none' in
bitmap_parse_region().

The parselist() supports partially used groups with the notation like:
0-1023:2/256 ==> 0,1,256,257,512,513,768,769
I think it's worth making sure that new keywords work with groups smoothly,
otherwise mention in documentation that they don't.

>         return bitmap_parselist(buf, cpumask_bits(dstp), nr_cpumask_bits);
>  }
>  EXPORT_SYMBOL(cpulist_parse);
> --
> 2.9.5
>

[PATCH RFC cpumask 4/5] cpumask: Add "last" alias for cpu list specifications

[paulmck]

From: Paul Gortmaker <paul.gortmaker@windriver.com>

It seems that a common configuration is to use the 1st couple cores
for housekeeping tasks, and or driving a busy peripheral that generates
a lot of interrupts, or something similar.

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.

Deployment would be easier if we could just simply replace 31 and 47
with "last" and let the system substitute in the actual number at boot;
a number that it knows better than we do.

No need to have custom boot args per node, no need to do a trial boot
in order to snoop /proc/cpuinfo and/or /sys/devices/system/cpu - no
more fencepost errors of using 32 and 48 instead of 31 and 47.

A generic token replacement is used to substitute "last" with the
number of CPUs present before handing off to bitmap processing.  But
it could just as easily be used to replace any placeholder token with
any other token or value only known at/after boot.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
---
 Documentation/admin-guide/kernel-parameters.rst |   7 ++
 lib/cpumask.c                                   | 112 +++++++++++++++++++++++-
 2 files changed, 117 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.rst b/Documentation/admin-guide/kernel-parameters.rst
index 7dd1224..5f441ef 100644
--- a/Documentation/admin-guide/kernel-parameters.rst
+++ b/Documentation/admin-guide/kernel-parameters.rst
@@ -83,6 +83,13 @@ will provide an empty/cleared cpu mask for the associated boot argument.
 Note that "all" and "none" are not necessarily valid/sensible input values
 for each available parameter expecting a CPU list.
 
+        foo_cpus=1,3,5,16-last
+
+will at runtime, replace "last" with the number of the last (highest number)
+present CPU on the system.  Thus a common deployment can be used on multiple
+systems with different total number of cores present, without needing to
+evaluate the total core count in advance on each system.
+
 This document may not be entirely up to date and comprehensive. The command
 "modinfo -p ${modulename}" shows a current list of all parameters of a loadable
 module. Loadable modules, after being loaded into the running kernel, also
diff --git a/lib/cpumask.c b/lib/cpumask.c
index 7fbcab8..97a005f 100644
--- a/lib/cpumask.c
+++ b/lib/cpumask.c
@@ -3,6 +3,7 @@
 #include <linux/kernel.h>
 #include <linux/bitops.h>
 #include <linux/string.h>
+#include <linux/ctype.h>
 #include <linux/cpumask.h>
 #include <linux/export.h>
 #include <linux/memblock.h>
@@ -96,15 +97,97 @@ int cpumask_next_wrap(int n, const struct cpumask *mask, int start, bool wrap)
 }
 EXPORT_SYMBOL(cpumask_next_wrap);
 
+/*
+ * Basically strstr() but given "foo", ignore "foobar", "myfoo", "foofoo"
+ * and "foo2bar" -- i.e. any case where the token is a word fragment.
+ */
+static char *cpumask_find_token(const char *str, const char *token)
+{
+	char *here = strstr(str, token);
+	size_t tlen = strlen(token);
+
+	if (!here)
+		return NULL;
+
+	while (here) {
+		size_t offset = here - str;
+		char prev, next = str[offset + tlen];
+
+		if (offset)
+			prev = str[offset - 1];
+		else
+			prev = '\0';
+
+		if (!(isalnum(prev) || isalnum(next)))
+			break;
+
+		here = strstr(here + tlen, token);
+	}
+
+	return here;
+}
+
+/*
+ * replace old token with new token: Given a convenience or placeholder
+ * token "last" and an associated value not known until boot, of say 1234,
+ * replace instances of "last" with "1234".
+ *
+ * For example src = "1,3,last,7-last,9,lastly,last-2047\0"  results in a
+ *            dest = "1,3,1234,7-1234,9,lastly,1234-2047\0"
+ *
+ * The destination string may be shorter than, equal to, or longer than
+ * the source string -- based on whether the new token strlen is shorter
+ * than, equal to, or longer than the old token strlen.
+ * The caller must allocate dest space accordingly with that in mind.
+ */
+
+static void cpulist_replace_token(char *dest, const char *src,
+			   const char *old_token, const char *new_token)
+{
+	const char *src_start = src;
+	char *dest_start = dest, *here;
+	const size_t olen = strlen(old_token);
+	const size_t nlen = strlen(new_token);
+
+	here = cpumask_find_token(src_start, old_token);
+	if (!here) {
+		strcpy(dest, src);
+		return;
+	}
+
+	while (here) {
+		size_t offset = here - src_start;
+
+		strncpy(dest_start, src_start, offset);
+		dest_start += offset;
+		src_start += offset;
+
+		strcpy(dest_start, new_token);
+		dest_start += nlen;
+		src_start += olen;
+
+		strcpy(dest_start, src_start);	/* remainder of string */
+		here = cpumask_find_token(src_start, old_token);
+	}
+}
+
 /**
  * cpulist_parse - extract a cpumask from a user string of ranges
  * @buf: the buffer to extract from
  * @dstp: the cpumask to set.
  *
  * Returns -errno, or 0 for success.
+ *
+ * Marked __ref because memblock_*() are __meminit and we use them for
+ * any early calls before slab is available.
  */
-int cpulist_parse(const char *buf, struct cpumask *dstp)
+int __ref cpulist_parse(const char *buf, struct cpumask *dstp)
 {
+	int r;
+	char *cpulist, last_cpu[5];	/* NR_CPUS <= 9999 */
+	size_t len = strlen(buf) + 1;	/* don't forget '\0' */
+	bool early = !slab_is_available();
+
 	if (!strcmp(buf, "all")) {
 		cpumask_setall(dstp);
 		return 0;
@@ -115,7 +198,32 @@ int cpulist_parse(const char *buf, struct cpumask *dstp)
 		return 0;
 	}
 
-	return bitmap_parselist(buf, cpumask_bits(dstp), nr_cpumask_bits);
+	/*
+	 * strlen("last") means "len" is OK up to NR_CPUS <= 9999.
+	 */
+	if (early)
+		cpulist = memblock_alloc(len, SMP_CACHE_BYTES);
+	else
+		cpulist = kzalloc(len, GFP_KERNEL);
+
+	if (cpulist == NULL)
+		return -ENOMEM;
+
+	/*
+	 * bitmap_parselist has no concept of "last" CPU, so we have to
+	 * replace "last" with a real number in dest copy of the string.
+	 */
+	sprintf(last_cpu, "%d", cpumask_last(cpu_present_mask));
+	cpulist_replace_token(cpulist, buf, "last", last_cpu);
+
+	r = bitmap_parselist(cpulist, cpumask_bits(dstp), nr_cpumask_bits);
+
+	if (early)
+		memblock_free((phys_addr_t)cpulist, len);
+	else
+		kfree(cpulist);
+
+	return r;
 }
 EXPORT_SYMBOL(cpulist_parse);
 
-- 
2.9.5

Re: [PATCH RFC cpumask 4/5] cpumask: Add "last" alias for cpu list specifications

[Yury Norov]

On Tue, Jan 5, 2021 at 4:49 PM <paulmck@kernel.org> wrote:
>
> From: Paul Gortmaker <paul.gortmaker@windriver.com>
>
> It seems that a common configuration is to use the 1st couple cores
> for housekeeping tasks, and or driving a busy peripheral that generates
> a lot of interrupts, or something similar.
>
> 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.
>
> Deployment would be easier if we could just simply replace 31 and 47
> with "last" and let the system substitute in the actual number at boot;
> a number that it knows better than we do.
>
> No need to have custom boot args per node, no need to do a trial boot
> in order to snoop /proc/cpuinfo and/or /sys/devices/system/cpu - no
> more fencepost errors of using 32 and 48 instead of 31 and 47.
>
> A generic token replacement is used to substitute "last" with the
> number of CPUs present before handing off to bitmap processing.  But
> it could just as easily be used to replace any placeholder token with
> any other token or value only known at/after boot.
>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> ---
>  Documentation/admin-guide/kernel-parameters.rst |   7 ++
>  lib/cpumask.c                                   | 112 +++++++++++++++++++++++-
>  2 files changed, 117 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.rst b/Documentation/admin-guide/kernel-parameters.rst
> index 7dd1224..5f441ef 100644
> --- a/Documentation/admin-guide/kernel-parameters.rst
> +++ b/Documentation/admin-guide/kernel-parameters.rst
> @@ -83,6 +83,13 @@ will provide an empty/cleared cpu mask for the associated boot argument.
>  Note that "all" and "none" are not necessarily valid/sensible input values
>  for each available parameter expecting a CPU list.
>
> +        foo_cpus=1,3,5,16-last
> +
> +will at runtime, replace "last" with the number of the last (highest number)
> +present CPU on the system.  Thus a common deployment can be used on multiple
> +systems with different total number of cores present, without needing to
> +evaluate the total core count in advance on each system.
> +
>  This document may not be entirely up to date and comprehensive. The command
>  "modinfo -p ${modulename}" shows a current list of all parameters of a loadable
>  module. Loadable modules, after being loaded into the running kernel, also
> diff --git a/lib/cpumask.c b/lib/cpumask.c
> index 7fbcab8..97a005f 100644
> --- a/lib/cpumask.c
> +++ b/lib/cpumask.c
> @@ -3,6 +3,7 @@
>  #include <linux/kernel.h>
>  #include <linux/bitops.h>
>  #include <linux/string.h>
> +#include <linux/ctype.h>
>  #include <linux/cpumask.h>
>  #include <linux/export.h>
>  #include <linux/memblock.h>
> @@ -96,15 +97,97 @@ int cpumask_next_wrap(int n, const struct cpumask *mask, int start, bool wrap)
>  }
>  EXPORT_SYMBOL(cpumask_next_wrap);
>
> +/*
> + * Basically strstr() but given "foo", ignore "foobar", "myfoo", "foofoo"
> + * and "foo2bar" -- i.e. any case where the token is a word fragment.
> + */
> +static char *cpumask_find_token(const char *str, const char *token)

This should go to lib/string.c and have a name like strword().

> +{
> +       char *here = strstr(str, token);
> +       size_t tlen = strlen(token);
> +
> +       if (!here)
> +               return NULL;
> +
> +       while (here) {
> +               size_t offset = here - str;
> +               char prev, next = str[offset + tlen];
> +
> +               if (offset)
> +                       prev = str[offset - 1];
> +               else
> +                       prev = '\0';
> +
> +               if (!(isalnum(prev) || isalnum(next)))
> +                       break;
> +
> +               here = strstr(here + tlen, token);
> +       }
> +
> +       return here;
> +}
> +
> +/*
> + * replace old token with new token: Given a convenience or placeholder
> + * token "last" and an associated value not known until boot, of say 1234,
> + * replace instances of "last" with "1234".
> + *
> + * For example src = "1,3,last,7-last,9,lastly,last-2047\0"  results in a
> + *            dest = "1,3,1234,7-1234,9,lastly,1234-2047\0"

This should be definitely done in bitmap_parselist(). There you will
find suitable
helpers to solve this problem in a few lines.

> + * The destination string may be shorter than, equal to, or longer than
> + * the source string -- based on whether the new token strlen is shorter
> + * than, equal to, or longer than the old token strlen.
> + * The caller must allocate dest space accordingly with that in mind.

How could he have a proper value in mind if he doesn't know the number of
digits in the last cpu number in advance, as well as the number of
substitutions?
For me, this hole will be exploited sooner or later.

> + */
> +
> +static void cpulist_replace_token(char *dest, const char *src,
> +                          const char *old_token, const char *new_token)
> +{
> +       const char *src_start = src;
> +       char *dest_start = dest, *here;
> +       const size_t olen = strlen(old_token);
> +       const size_t nlen = strlen(new_token);
> +
> +       here = cpumask_find_token(src_start, old_token);
> +       if (!here) {
> +               strcpy(dest, src);
> +               return;
> +       }
> +
> +       while (here) {
> +               size_t offset = here - src_start;
> +
> +               strncpy(dest_start, src_start, offset);
> +               dest_start += offset;
> +               src_start += offset;
> +
> +               strcpy(dest_start, new_token);
> +               dest_start += nlen;
> +               src_start += olen;
> +
> +               strcpy(dest_start, src_start);  /* remainder of string */

'Shlemiel the painter' style. The complexity is O(N^2).

> +               here = cpumask_find_token(src_start, old_token);
> +       }
> +}
> +
>  /**
>   * cpulist_parse - extract a cpumask from a user string of ranges
>   * @buf: the buffer to extract from
>   * @dstp: the cpumask to set.
>   *
>   * Returns -errno, or 0 for success.
> + *
> + * Marked __ref because memblock_*() are __meminit and we use them for
> + * any early calls before slab is available.
>   */
> -int cpulist_parse(const char *buf, struct cpumask *dstp)
> +int __ref cpulist_parse(const char *buf, struct cpumask *dstp)
>  {
> +       int r;
> +       char *cpulist, last_cpu[5];     /* NR_CPUS <= 9999 */
> +       size_t len = strlen(buf) + 1;   /* don't forget '\0' */
> +       bool early = !slab_is_available();
> +
>         if (!strcmp(buf, "all")) {
>                 cpumask_setall(dstp);
>                 return 0;
> @@ -115,7 +198,32 @@ int cpulist_parse(const char *buf, struct cpumask *dstp)
>                 return 0;
>         }
>
> -       return bitmap_parselist(buf, cpumask_bits(dstp), nr_cpumask_bits);
> +       /*
> +        * strlen("last") means "len" is OK up to NR_CPUS <= 9999.

Most supercomputers run Linux these days.

> +        */
> +       if (early)
> +               cpulist = memblock_alloc(len, SMP_CACHE_BYTES);
> +       else
> +               cpulist = kzalloc(len, GFP_KERNEL);
> +
> +       if (cpulist == NULL)
> +               return -ENOMEM;
> +
> +       /*
> +        * bitmap_parselist has no concept of "last" CPU, so we have to
> +        * replace "last" with a real number in dest copy of the string.

..., so we have to add that concept to bitmap_parselist. NAK.

> +        */
> +       sprintf(last_cpu, "%d", cpumask_last(cpu_present_mask));
> +       cpulist_replace_token(cpulist, buf, "last", last_cpu);
> +
> +       r = bitmap_parselist(cpulist, cpumask_bits(dstp), nr_cpumask_bits);
> +
> +       if (early)
> +               memblock_free((phys_addr_t)cpulist, len);
> +       else
> +               kfree(cpulist);
> +
> +       return r;
>  }
>  EXPORT_SYMBOL(cpulist_parse);
>
> --
> 2.9.5
>

Re: [PATCH RFC cpumask 4/5] cpumask: Add "last" alias for cpu list specifications

[Peter Zijlstra]

On Tue, Jan 05, 2021 at 04:49:55PM -0800, paulmck@kernel.org wrote:
> From: Paul Gortmaker <paul.gortmaker@windriver.com>
> 
> It seems that a common configuration is to use the 1st couple cores
> for housekeeping tasks, and or driving a busy peripheral that generates
> a lot of interrupts, or something similar.
> 
> 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.
> 
> Deployment would be easier if we could just simply replace 31 and 47
> with "last" and let the system substitute in the actual number at boot;
> a number that it knows better than we do.
> 
> No need to have custom boot args per node, no need to do a trial boot
> in order to snoop /proc/cpuinfo and/or /sys/devices/system/cpu - no
> more fencepost errors of using 32 and 48 instead of 31 and 47.
> 
> A generic token replacement is used to substitute "last" with the
> number of CPUs present before handing off to bitmap processing.  But
> it could just as easily be used to replace any placeholder token with
> any other token or value only known at/after boot.

Aside from the comments Yury made, on how all this is better in
bitmap_parselist(), how about doing s/last/N/ here? For me something
like: "4-N" reads much saner than "4-last".

Also, it might make sense to teach all this about core/node topology,
but that's going to be messy. Imagine something like "Core1-CoreN" or
"Nore1-NodeN" to mean the mask all/{Core,Node}0.

And that is another feature that seems to be missing from parselist,
all/except.

Re: [PATCH RFC cpumask 4/5] cpumask: Add "last" alias for cpu list specifications

[Paul Gortmaker]

[Re: [PATCH RFC cpumask 4/5] cpumask: Add "last" alias for cpu list specifications] On 06/01/2021 (Wed 10:49) Peter Zijlstra wrote:

> On Tue, Jan 05, 2021 at 04:49:55PM -0800, paulmck@kernel.org wrote:
> > From: Paul Gortmaker <paul.gortmaker@windriver.com>
> > 
> > It seems that a common configuration is to use the 1st couple cores
> > for housekeeping tasks, and or driving a busy peripheral that generates
> > a lot of interrupts, or something similar.

[...]

> > A generic token replacement is used to substitute "last" with the
> > number of CPUs present before handing off to bitmap processing.  But
> > it could just as easily be used to replace any placeholder token with
> > any other token or value only known at/after boot.
> 
> Aside from the comments Yury made, on how all this is better in
> bitmap_parselist(), how about doing s/last/N/ here? For me something
> like: "4-N" reads much saner than "4-last".

OK, I can see N used as per university math classes... to indicate the
end point of a fixed set of numbers, but I confess to having had to
think about it for a bit (university was a long time ago).  I don't have
any strong opinion one way or another -- "last" vs. "N"...

> Also, it might make sense to teach all this about core/node topology,
> but that's going to be messy. Imagine something like "Core1-CoreN" or
> "Nore1-NodeN" to mean the mask all/{Core,Node}0.
> 
> And that is another feature that seems to be missing from parselist,
> all/except.

Seems reasonable, but I'm going to look at fixing up what I've got as
per Yury's comments before volunteering to muck around with more string
parsing code to add more features...

Thanks,
Paul.
--

Re: [PATCH RFC cpumask 4/5] cpumask: Add "last" alias for cpu list specifications

[Yury Norov]

On Wed, Jan 6, 2021 at 1:50 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Tue, Jan 05, 2021 at 04:49:55PM -0800, paulmck@kernel.org wrote:
> > From: Paul Gortmaker <paul.gortmaker@windriver.com>
> >
> > It seems that a common configuration is to use the 1st couple cores
> > for housekeeping tasks, and or driving a busy peripheral that generates
> > a lot of interrupts, or something similar.
> >
> > 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.
> >
> > Deployment would be easier if we could just simply replace 31 and 47
> > with "last" and let the system substitute in the actual number at boot;
> > a number that it knows better than we do.
> >
> > No need to have custom boot args per node, no need to do a trial boot
> > in order to snoop /proc/cpuinfo and/or /sys/devices/system/cpu - no
> > more fencepost errors of using 32 and 48 instead of 31 and 47.
> >
> > A generic token replacement is used to substitute "last" with the
> > number of CPUs present before handing off to bitmap processing.  But
> > it could just as easily be used to replace any placeholder token with
> > any other token or value only known at/after boot.
>
> Aside from the comments Yury made, on how all this is better in
> bitmap_parselist(), how about doing s/last/N/ here? For me something
> like: "4-N" reads much saner than "4-last".
>
> Also, it might make sense to teach all this about core/node topology,
> but that's going to be messy. Imagine something like "Core1-CoreN" or
> "Nore1-NodeN" to mean the mask all/{Core,Node}0.

If you just want to teach bitmap_parselist() to "s/Core0/0-4",  I think
it's doable if we add a hook to a proper subsystem in bitmap_parselist().

> And that is another feature that seems to be missing from parselist,
> all/except.

We already support groups in a range. I think it partially covers the
proposed all/except.

Can you share examples on what you miss?

Re: [PATCH RFC cpumask 4/5] cpumask: Add "last" alias for cpu list specifications

[Peter Zijlstra]

On Wed, Jan 06, 2021 at 01:16:50PM -0800, Yury Norov wrote:
> On Wed, Jan 6, 2021 at 1:50 AM Peter Zijlstra <peterz@infradead.org> wrote:

> > Aside from the comments Yury made, on how all this is better in
> > bitmap_parselist(), how about doing s/last/N/ here? For me something
> > like: "4-N" reads much saner than "4-last".
> >
> > Also, it might make sense to teach all this about core/node topology,
> > but that's going to be messy. Imagine something like "Core1-CoreN" or
> > "Nore1-NodeN" to mean the mask all/{Core,Node}0.
> 
> If you just want to teach bitmap_parselist() to "s/Core0/0-4",  I think
> it's doable if we add a hook to a proper subsystem in bitmap_parselist().
> 
> > And that is another feature that seems to be missing from parselist,
> > all/except.
> 
> We already support groups in a range. I think it partially covers the
> proposed all/except.
> 
> Can you share examples on what you miss?

The obvious one is the "all/Core0" example above, which would be
equivalent to "Core1-CoreN".

Another case that I don't think we can do today is something like, give
me SMT0 of each core.

I don't really see the use of the ranges thing, CPU enumeration just
isn't sane like that. Also, I should really add that randomization pass
to the CPU enumeration :-)

Re: [PATCH RFC cpumask 4/5] cpumask: Add "last" alias for cpu list specifications

[Paul E. McKenney]

On Thu, Jan 07, 2021 at 03:18:47PM +0100, Peter Zijlstra wrote:
> On Wed, Jan 06, 2021 at 01:16:50PM -0800, Yury Norov wrote:
> > On Wed, Jan 6, 2021 at 1:50 AM Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > > Aside from the comments Yury made, on how all this is better in
> > > bitmap_parselist(), how about doing s/last/N/ here? For me something
> > > like: "4-N" reads much saner than "4-last".
> > >
> > > Also, it might make sense to teach all this about core/node topology,
> > > but that's going to be messy. Imagine something like "Core1-CoreN" or
> > > "Nore1-NodeN" to mean the mask all/{Core,Node}0.
> > 
> > If you just want to teach bitmap_parselist() to "s/Core0/0-4",  I think
> > it's doable if we add a hook to a proper subsystem in bitmap_parselist().
> > 
> > > And that is another feature that seems to be missing from parselist,
> > > all/except.
> > 
> > We already support groups in a range. I think it partially covers the
> > proposed all/except.
> > 
> > Can you share examples on what you miss?
> 
> The obvious one is the "all/Core0" example above, which would be
> equivalent to "Core1-CoreN".
> 
> Another case that I don't think we can do today is something like, give
> me SMT0 of each core.
> 
> I don't really see the use of the ranges thing, CPU enumeration just
> isn't sane like that. Also, I should really add that randomization pass
> to the CPU enumeration :-)

Please don't!!!

							Thanx, Paul

Re: [PATCH RFC cpumask 4/5] cpumask: Add "last" alias for cpu list specifications

[Peter Zijlstra]

On Thu, Jan 07, 2021 at 06:47:57AM -0800, Paul E. McKenney wrote:
> > I don't really see the use of the ranges thing, CPU enumeration just
> > isn't sane like that. Also, I should really add that randomization pass
> > to the CPU enumeration :-)
> 
> Please don't!!!

Why not, the BIOS more or less already does that on a per machine basis
anyway. Doing it per boot just makes things more reliably screwy ;-)

Re: [PATCH RFC cpumask 4/5] cpumask: Add "last" alias for cpu list specifications

[Paul E. McKenney]

On Thu, Jan 07, 2021 at 03:59:42PM +0100, Peter Zijlstra wrote:
> On Thu, Jan 07, 2021 at 06:47:57AM -0800, Paul E. McKenney wrote:
> > > I don't really see the use of the ranges thing, CPU enumeration just
> > > isn't sane like that. Also, I should really add that randomization pass
> > > to the CPU enumeration :-)
> > 
> > Please don't!!!
> 
> Why not, the BIOS more or less already does that on a per machine basis
> anyway. Doing it per boot just makes things more reliably screwy ;-)

Fixing BIOS would be much more productive, now wouldn't it?

							Thanx, Paul

Re: [PATCH RFC cpumask 4/5] cpumask: Add "last" alias for cpu list specifications

[Paul E. McKenney]

On Thu, Jan 07, 2021 at 03:18:47PM +0100, Peter Zijlstra wrote:
> On Wed, Jan 06, 2021 at 01:16:50PM -0800, Yury Norov wrote:
> > On Wed, Jan 6, 2021 at 1:50 AM Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > > Aside from the comments Yury made, on how all this is better in
> > > bitmap_parselist(), how about doing s/last/N/ here? For me something
> > > like: "4-N" reads much saner than "4-last".
> > >
> > > Also, it might make sense to teach all this about core/node topology,
> > > but that's going to be messy. Imagine something like "Core1-CoreN" or
> > > "Nore1-NodeN" to mean the mask all/{Core,Node}0.
> > 
> > If you just want to teach bitmap_parselist() to "s/Core0/0-4",  I think
> > it's doable if we add a hook to a proper subsystem in bitmap_parselist().
> > 
> > > And that is another feature that seems to be missing from parselist,
> > > all/except.
> > 
> > We already support groups in a range. I think it partially covers the
> > proposed all/except.
> > 
> > Can you share examples on what you miss?
> 
> The obvious one is the "all/Core0" example above, which would be
> equivalent to "Core1-CoreN".
> 
> Another case that I don't think we can do today is something like, give
> me SMT0 of each core.
> 
> I don't really see the use of the ranges thing, CPU enumeration just
> isn't sane like that.

Ranges are useful on many systems.  Users of systems with insane CPU
enumeration are of course free to provide comma-separated lists of
numbers for their cpumask boot parameters, avoiding use of minus signs.

							Thanx, Paul

[PATCH RFC cpumask 5/5] rcutorture: Use "all" and "last" in "nohz_full" and "rcu_nocbs"

[paulmck]

From: "Paul E. McKenney" <paulmck@kernel.org>

This commit uses the shiny new "all" and "last" cpumask options to
decouple the "nohz_full" and "rcu_nocbs" kernel boot parameters in the
TREE04.boot and TREE08.boot files from the CONFIG_NR_CPUS options in
the TREE04 and TREE08 files.

Reported-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
---
 tools/testing/selftests/rcutorture/configs/rcu/TREE04.boot | 2 +-
 tools/testing/selftests/rcutorture/configs/rcu/TREE08.boot | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/rcutorture/configs/rcu/TREE04.boot b/tools/testing/selftests/rcutorture/configs/rcu/TREE04.boot
index 5adc675..25a765d 100644
--- a/tools/testing/selftests/rcutorture/configs/rcu/TREE04.boot
+++ b/tools/testing/selftests/rcutorture/configs/rcu/TREE04.boot
@@ -1 +1 @@
-rcutree.rcu_fanout_leaf=4 nohz_full=1-7
+rcutree.rcu_fanout_leaf=4 nohz_full=1-last
diff --git a/tools/testing/selftests/rcutorture/configs/rcu/TREE08.boot b/tools/testing/selftests/rcutorture/configs/rcu/TREE08.boot
index 22478fd..94d3844 100644
--- a/tools/testing/selftests/rcutorture/configs/rcu/TREE08.boot
+++ b/tools/testing/selftests/rcutorture/configs/rcu/TREE08.boot
@@ -1,3 +1,3 @@
 rcupdate.rcu_self_test=1
 rcutree.rcu_fanout_exact=1
-rcu_nocbs=0-7
+rcu_nocbs=all
-- 
2.9.5

Re: [PATCH RFC cpumask] Allow "all", "none", and "last" in cpumask strings

[Yury Norov]

On Tue, Jan 5, 2021 at 4:48 PM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> Hello!
>
> This series allows "all", "none", and "last" to be used in cpumask
> strings.  This allows these strings to be less dependent on the underlying
> system.  For example, currently a string specifying all but the first
> CPU must be "1-7" on an eight-CPU system and "1-15" on a 16-CPU system.
> With this series, the single string "1-last" can be used regardless of the
> number of CPUs (at least assuming that each system has at least one CPU).

'none' may be implemented as an empty string or string with separators only,
but I have nothing against explicit 'none'. See other comments inline.

Thanks,
Yury.

> 1.      Un-inline cpulist_parse for SMP; prepare for ascii helpers,
>         courtesy of Paul Gortmaker.
>
> 2.      Make "all" alias global and not just RCU, courtesy of Paul
>         Gortmaker.
>
> 3.      Add a "none" alias to complement "all", courtesy of Paul
>         Gortmaker.
>
> 4.      Add "last" alias for cpu list specifications, courtesy of Paul
>         Gortmaker.
>
> 5.      Use "all" and "last" in "nohz_full" and "rcu_nocbs".
>
>                                                 Thanx, Paul
>
> ------------------------------------------------------------------------
>
>  Documentation/admin-guide/kernel-parameters.rst            |   20 +
>  Documentation/admin-guide/kernel-parameters.txt            |    4
>  include/linux/cpumask.h                                    |    8
>  kernel/rcu/tree_plugin.h                                   |   13 -
>  lib/cpumask.c                                              |  136 ++++++++++++-
>  tools/testing/selftests/rcutorture/configs/rcu/TREE04.boot |    2
>  tools/testing/selftests/rcutorture/configs/rcu/TREE08.boot |    2
>  7 files changed, 169 insertions(+), 16 deletions(-)

Re: [PATCH RFC cpumask] Allow "all", "none", and "last" in cpumask strings

[Yury Norov]

On Wed, Jan 6, 2021 at 12:49 AM Yury Norov <yury.norov@gmail.com> wrote:
>
> On Tue, Jan 5, 2021 at 4:48 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > Hello!
> >
> > This series allows "all", "none", and "last" to be used in cpumask
> > strings.  This allows these strings to be less dependent on the underlying
> > system.  For example, currently a string specifying all but the first
> > CPU must be "1-7" on an eight-CPU system and "1-15" on a 16-CPU system.
> > With this series, the single string "1-last" can be used regardless of the
> > number of CPUs (at least assuming that each system has at least one CPU).
>
> 'none' may be implemented as an empty string or string with separators only,
> but I have nothing against explicit 'none'. See other comments inline.
>
> Thanks,
> Yury.
>
> > 1.      Un-inline cpulist_parse for SMP; prepare for ascii helpers,
> >         courtesy of Paul Gortmaker.
> >
> > 2.      Make "all" alias global and not just RCU, courtesy of Paul
> >         Gortmaker.
> >
> > 3.      Add a "none" alias to complement "all", courtesy of Paul
> >         Gortmaker.
> >
> > 4.      Add "last" alias for cpu list specifications, courtesy of Paul
> >         Gortmaker.
> >
> > 5.      Use "all" and "last" in "nohz_full" and "rcu_nocbs".
> >
> >                                                 Thanx, Paul

Hi Paul,

Today I found this series in linux-next despite downsides discovered during
the review. This series introduces absolutely unneeded cap on the number of
cpus in the system (9999), and also adds unsafe and non-optimal code.

In addition to that, I observe this warning on powerpc:
  CC      lib/cpumask.o
lib/cpumask.c: In function ‘cpulist_parse’:
lib/cpumask.c:222:17: warning: cast from pointer to integer of
different size [-Wpointer-to-int-cast]
  222 |   memblock_free((phys_addr_t)cpulist, len);
      |                 ^

Can you please revert this series unless all the problems will be fixed?

Thanks,
Yury

Re: [PATCH RFC cpumask] Allow "all", "none", and "last" in cpumask strings

[Paul E. McKenney]

On Wed, Jan 20, 2021 at 11:11:48PM -0800, Yury Norov wrote:
> On Wed, Jan 6, 2021 at 12:49 AM Yury Norov <yury.norov@gmail.com> wrote:
> >
> > On Tue, Jan 5, 2021 at 4:48 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > >
> > > Hello!
> > >
> > > This series allows "all", "none", and "last" to be used in cpumask
> > > strings.  This allows these strings to be less dependent on the underlying
> > > system.  For example, currently a string specifying all but the first
> > > CPU must be "1-7" on an eight-CPU system and "1-15" on a 16-CPU system.
> > > With this series, the single string "1-last" can be used regardless of the
> > > number of CPUs (at least assuming that each system has at least one CPU).
> >
> > 'none' may be implemented as an empty string or string with separators only,
> > but I have nothing against explicit 'none'. See other comments inline.
> >
> > Thanks,
> > Yury.
> >
> > > 1.      Un-inline cpulist_parse for SMP; prepare for ascii helpers,
> > >         courtesy of Paul Gortmaker.
> > >
> > > 2.      Make "all" alias global and not just RCU, courtesy of Paul
> > >         Gortmaker.
> > >
> > > 3.      Add a "none" alias to complement "all", courtesy of Paul
> > >         Gortmaker.
> > >
> > > 4.      Add "last" alias for cpu list specifications, courtesy of Paul
> > >         Gortmaker.
> > >
> > > 5.      Use "all" and "last" in "nohz_full" and "rcu_nocbs".
> > >
> > >                                                 Thanx, Paul
> 
> Hi Paul,
> 
> Today I found this series in linux-next despite downsides discovered during
> the review. This series introduces absolutely unneeded cap on the number of
> cpus in the system (9999), and also adds unsafe and non-optimal code.
> 
> In addition to that, I observe this warning on powerpc:
>   CC      lib/cpumask.o
> lib/cpumask.c: In function ‘cpulist_parse’:
> lib/cpumask.c:222:17: warning: cast from pointer to integer of
> different size [-Wpointer-to-int-cast]
>   222 |   memblock_free((phys_addr_t)cpulist, len);
>       |                 ^
> 
> Can you please revert this series unless all the problems will be fixed?

Thank you for your further review and comments!

I had been keeping the old series as a placeholder for its anticipated
replacement, but given the compiler warning you note above and given
that it is getting uncomfortably close to the time when I send my pull
request, I will remove it from the -rcu rcu/next branch sourced by -next.

							Thanx, Paul

Re: [PATCH RFC cpumask] Allow "all", "none", and "last" in cpumask strings

[Paul E. McKenney]

On Thu, Jan 21, 2021 at 08:57:25AM -0800, Paul E. McKenney wrote:
> On Wed, Jan 20, 2021 at 11:11:48PM -0800, Yury Norov wrote:
> > On Wed, Jan 6, 2021 at 12:49 AM Yury Norov <yury.norov@gmail.com> wrote:
> > >
> > > On Tue, Jan 5, 2021 at 4:48 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > > >
> > > > Hello!
> > > >
> > > > This series allows "all", "none", and "last" to be used in cpumask
> > > > strings.  This allows these strings to be less dependent on the underlying
> > > > system.  For example, currently a string specifying all but the first
> > > > CPU must be "1-7" on an eight-CPU system and "1-15" on a 16-CPU system.
> > > > With this series, the single string "1-last" can be used regardless of the
> > > > number of CPUs (at least assuming that each system has at least one CPU).
> > >
> > > 'none' may be implemented as an empty string or string with separators only,
> > > but I have nothing against explicit 'none'. See other comments inline.
> > >
> > > Thanks,
> > > Yury.
> > >
> > > > 1.      Un-inline cpulist_parse for SMP; prepare for ascii helpers,
> > > >         courtesy of Paul Gortmaker.
> > > >
> > > > 2.      Make "all" alias global and not just RCU, courtesy of Paul
> > > >         Gortmaker.
> > > >
> > > > 3.      Add a "none" alias to complement "all", courtesy of Paul
> > > >         Gortmaker.
> > > >
> > > > 4.      Add "last" alias for cpu list specifications, courtesy of Paul
> > > >         Gortmaker.
> > > >
> > > > 5.      Use "all" and "last" in "nohz_full" and "rcu_nocbs".
> > > >
> > > >                                                 Thanx, Paul
> > 
> > Hi Paul,
> > 
> > Today I found this series in linux-next despite downsides discovered during
> > the review. This series introduces absolutely unneeded cap on the number of
> > cpus in the system (9999), and also adds unsafe and non-optimal code.
> > 
> > In addition to that, I observe this warning on powerpc:
> >   CC      lib/cpumask.o
> > lib/cpumask.c: In function ‘cpulist_parse’:
> > lib/cpumask.c:222:17: warning: cast from pointer to integer of
> > different size [-Wpointer-to-int-cast]
> >   222 |   memblock_free((phys_addr_t)cpulist, len);
> >       |                 ^
> > 
> > Can you please revert this series unless all the problems will be fixed?
> 
> Thank you for your further review and comments!
> 
> I had been keeping the old series as a placeholder for its anticipated
> replacement, but given the compiler warning you note above and given
> that it is getting uncomfortably close to the time when I send my pull
> request, I will remove it from the -rcu rcu/next branch sourced by -next.

Except that the resulting rebasing and testing took too long, so there will
be one more -next with these commits.

								Thanx, Paul

Re: [PATCH RFC cpumask] Allow "all", "none", and "last" in cpumask strings

[Paul Gortmaker]

[Re: [PATCH RFC cpumask] Allow "all", "none", and "last" in cpumask strings] On 20/01/2021 (Wed 23:11) Yury Norov wrote:

> Hi Paul,
> 
> Today I found this series in linux-next despite downsides discovered during
> the review. This series introduces absolutely unneeded cap on the number of
> cpus in the system (9999), and also adds unsafe and non-optimal code.
> 
> In addition to that, I observe this warning on powerpc:
>   CC      lib/cpumask.o
> lib/cpumask.c: In function ‘cpulist_parse’:
> lib/cpumask.c:222:17: warning: cast from pointer to integer of
> different size [-Wpointer-to-int-cast]
>   222 |   memblock_free((phys_addr_t)cpulist, len);
>       |                 ^
> 
> Can you please revert this series unless all the problems will be fixed?

That was my fault - I should have explicitly asked PaulM to yank it once
I didn't get to creating v2 immediately.  Sorry.

Your suggested changes made things much more simple and smaller - thanks!

I believe v2 does address all the problems - please have a look when you
have some time.  It should be easier to review, given the smaller size.

https://lore.kernel.org/lkml/20210121223355.59780-1-paul.gortmaker@windriver.com/

Thanks again,
Paul.
--

> 
> Thanks,
> Yury