[1/2] bitmap_parse: support 'all' semantics
diff mbox series

Message ID 20210421031326.72816-2-yury.norov@gmail.com
State In Next
Commit 458a0b70b496b278fbf89f08b7ab5c92de715ae9
Headers show
Series
  • bitmap_parselist: support 'all' semantics
Related show

Commit Message

Yury Norov April 21, 2021, 3:13 a.m. UTC
RCU code supports an 'all' group as a special case when parsing
rcu_nocbs parameter. This patch moves the 'all' support to the core
bitmap_parse code, so that all bitmap users can enjoy this extension.

Moving 'all' parsing to a bitmap_parse level, also allows users to
pass patterns together with 'all' in regular group:pattern format

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 Documentation/admin-guide/kernel-parameters.rst | 5 +++++
 lib/bitmap.c                                    | 9 +++++++++
 lib/test_bitmap.c                               | 7 +++++++
 3 files changed, 21 insertions(+)

Comments

Steven Rostedt April 21, 2021, 3:19 p.m. UTC | #1
On Tue, 20 Apr 2021 20:13:25 -0700
Yury Norov <yury.norov@gmail.com> wrote:

> @@ -76,6 +76,11 @@ to change, such as less cores in the CPU list, then N and any ranges using N
>  will also change.  Use the same on a small 4 core system, and "16-N" becomes
>  "16-3" and now the same boot input will be flagged as invalid (start > end).
>  
> +The special case-tolerant group name "all" has a meaning of selecting all CPUs,
> +such that "isolcpus=all" is the equivalent of "isolcpus=0-N".

I'm OK with the concept of this patch set, but really? That is a horrible
example. One should NEVER set isolcpus to all!

-- Steve


> +
> +The semantics of "N" and "all" is supported on a level of bitmaps and holds for
> +all users of bitmap_parse().
>
Paul E. McKenney April 21, 2021, 3:57 p.m. UTC | #2
On Wed, Apr 21, 2021 at 11:19:32AM -0400, Steven Rostedt wrote:
> On Tue, 20 Apr 2021 20:13:25 -0700
> Yury Norov <yury.norov@gmail.com> wrote:
> 
> > @@ -76,6 +76,11 @@ to change, such as less cores in the CPU list, then N and any ranges using N
> >  will also change.  Use the same on a small 4 core system, and "16-N" becomes
> >  "16-3" and now the same boot input will be flagged as invalid (start > end).
> >  
> > +The special case-tolerant group name "all" has a meaning of selecting all CPUs,
> > +such that "isolcpus=all" is the equivalent of "isolcpus=0-N".
> 
> I'm OK with the concept of this patch set, but really? That is a horrible
> example. One should NEVER set isolcpus to all!

How about "isolcpus=all:2/4"?  ;-)

							Thanx, Paul

> -- Steve
> 
> 
> > +
> > +The semantics of "N" and "all" is supported on a level of bitmaps and holds for
> > +all users of bitmap_parse().
> >
Paul Gortmaker April 21, 2021, 11:45 p.m. UTC | #3
[Re: [PATCH 1/2] bitmap_parse: support 'all' semantics] On 21/04/2021 (Wed 11:19) Steven Rostedt wrote:

> On Tue, 20 Apr 2021 20:13:25 -0700
> Yury Norov <yury.norov@gmail.com> wrote:
> 
> > @@ -76,6 +76,11 @@ to change, such as less cores in the CPU list, then N and any ranges using N
> >  will also change.  Use the same on a small 4 core system, and "16-N" becomes
> >  "16-3" and now the same boot input will be flagged as invalid (start > end).
> >  
> > +The special case-tolerant group name "all" has a meaning of selecting all CPUs,
> > +such that "isolcpus=all" is the equivalent of "isolcpus=0-N".
> 
> I'm OK with the concept of this patch set, but really? That is a horrible
> example. One should NEVER set isolcpus to all!

It wouldn't have parsed anyways.  The "isolcpus=" would just think "all" was
an unsupported flag and discard it before it ever got to cpu/bitmap
processing.   See 3662daf02350 "sched/isolation: Allow "isolcpus=" to
skip unknown sub-parameters".

The use case example Paul chose (not me; the smart Paul) is better.

Paul.
--

> 
> -- Steve
> 
> 
> > +
> > +The semantics of "N" and "all" is supported on a level of bitmaps and holds for
> > +all users of bitmap_parse().
> >

Patch
diff mbox series

diff --git a/Documentation/admin-guide/kernel-parameters.rst b/Documentation/admin-guide/kernel-parameters.rst
index 3996b54158bf..7178d9c4de7f 100644
--- a/Documentation/admin-guide/kernel-parameters.rst
+++ b/Documentation/admin-guide/kernel-parameters.rst
@@ -76,6 +76,11 @@  to change, such as less cores in the CPU list, then N and any ranges using N
 will also change.  Use the same on a small 4 core system, and "16-N" becomes
 "16-3" and now the same boot input will be flagged as invalid (start > end).
 
+The special case-tolerant group name "all" has a meaning of selecting all CPUs,
+such that "isolcpus=all" is the equivalent of "isolcpus=0-N".
+
+The semantics of "N" and "all" is supported on a level of bitmaps and holds for
+all users of bitmap_parse().
 
 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/bitmap.c b/lib/bitmap.c
index 74ceb02f45e3..6e29b2aae6ba 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -581,6 +581,14 @@  static const char *bitmap_parse_region(const char *str, struct region *r)
 {
 	unsigned int lastbit = r->nbits - 1;
 
+	if (!strncasecmp(str, "all", 3)) {
+		r->start = 0;
+		r->end = lastbit;
+		str += 3;
+
+		goto check_pattern;
+	}
+
 	str = bitmap_getnum(str, &r->start, lastbit);
 	if (IS_ERR(str))
 		return str;
@@ -595,6 +603,7 @@  static const char *bitmap_parse_region(const char *str, struct region *r)
 	if (IS_ERR(str))
 		return str;
 
+check_pattern:
 	if (end_of_region(*str))
 		goto no_pattern;
 
diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
index 9cd575583180..4ea73f5aed41 100644
--- a/lib/test_bitmap.c
+++ b/lib/test_bitmap.c
@@ -366,6 +366,13 @@  static const struct test_bitmap_parselist parselist_tests[] __initconst = {
 	{0, "0-31:1/3,1-31:1/3,2-31:1/3",	&exp1[8 * step], 32, 0},
 	{0, "1-10:8/12,8-31:24/29,0-31:0/3",	&exp1[9 * step], 32, 0},
 
+	{0,	  "all",		&exp1[8 * step], 32, 0},
+	{0,	  "0, 1, all,  ",	&exp1[8 * step], 32, 0},
+	{0,	  "all:1/2",		&exp1[4 * step], 32, 0},
+	{0,	  "ALL:1/2",		&exp1[4 * step], 32, 0},
+	{-EINVAL, "al", NULL, 8, 0},
+	{-EINVAL, "alll", NULL, 8, 0},
+
 	{-EINVAL, "-1",	NULL, 8, 0},
 	{-EINVAL, "-0",	NULL, 8, 0},
 	{-EINVAL, "10-1", NULL, 8, 0},