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; 16+ 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] 16+ messages in thread
* [PATCH v4 0/8] support for bitmap (and hence CPU) list "N" abbreviation
@ 2021-02-09 22:58 Paul Gortmaker
  2021-02-09 22:59 ` [PATCH 8/8] rcu: deprecate "all" option to rcu_nocbs= Paul Gortmaker
  0 siblings, 1 reply; 16+ messages in thread
From: Paul Gortmaker @ 2021-02-09 22:58 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

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.
---

I've intentionally not gone down the rabbit hole of whether N or Z or
L is the better letter to mark the end of a mathematical set in the
hope that we can stay focused, and get this closed out here in v4.

Aside from that, I believe all other feedback has been responded to
in one way or another.  Note that I didn't add Reviewed/Ack tags to
anything that changed significantly from what was reviewed in v3.

[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.]

[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: move ERANGE check from set_region to check_region
  lib: bitmap: pair nbits value with region struct
  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                                  | 62 +++++++++++++------
 lib/test_bitmap.c                             | 46 ++++++++++++--
 5 files changed, 93 insertions(+), 32 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 16+ messages in thread
* [PATCH v3 0/8] support for bitmap (and hence CPU) list "N" abbreviation
@ 2021-01-26 17:11 Paul Gortmaker
  2021-01-26 17:11 ` [PATCH 8/8] rcu: deprecate "all" option to rcu_nocbs= Paul Gortmaker
  0 siblings, 1 reply; 16+ 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

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 [6/8] 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.
---

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

[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/

[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. ]

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 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_getnum: separate arg into region and field
  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         |  2 +
 .../admin-guide/kernel-parameters.txt         |  4 +-
 kernel/rcu/tree_plugin.h                      |  6 +--
 lib/bitmap.c                                  | 46 ++++++++++--------
 lib/test_bitmap.c                             | 48 ++++++++++++++++---
 5 files changed, 72 insertions(+), 34 deletions(-)

-- 
2.17.1


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

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

Thread overview: 16+ 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 8/8] rcu: deprecate "all" option to rcu_nocbs= 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 8/8] rcu: deprecate "all" option to rcu_nocbs= Paul Gortmaker
2021-01-26 21:36   ` Yury Norov
2021-01-26 22:17     ` Paul E. McKenney

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).