Re: [PATCH RFC cpumask 4/5] cpumask: Add "last" alias for cpu list specifications
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
On Wed, Jan 06, 2021 at 01:16:50PM -0800, Yury Norov wrote:
> On Wed, Jan 6, 2021 at 1:50 AM Peter Zijlstra 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
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
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 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
Re: [PATCH RFC cpumask 4/5] cpumask: Add "last" alias for cpu list specifications
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 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
On Wed, Jan 6, 2021 at 1:50 AM Peter Zijlstra wrote:
>
> On Tue, Jan 05, 2021 at 04:49:55PM -0800, paul...@kernel.org wrote:
> > From: Paul Gortmaker
> >
> > 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
[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, paul...@kernel.org wrote:
> > From: Paul Gortmaker
> >
> > 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
On Tue, Jan 05, 2021 at 04:49:55PM -0800, paul...@kernel.org wrote:
> From: Paul Gortmaker
>
> 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
On Tue, Jan 5, 2021 at 4:49 PM wrote:
>
> From: Paul Gortmaker
>
> 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
> Signed-off-by: Paul E. McKenney
> ---
> 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
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -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
[PATCH RFC cpumask 4/5] cpumask: Add "last" alias for cpu list specifications
From: Paul Gortmaker
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
Signed-off-by: Paul E. McKenney
---
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
#include
#include
+#include
#include
#include
#include
@@ -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;
+
