linux-kernel - Re: [PATCH 1/3] lib: add "all" and "none" as valid ranges to bitmap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210122043421.GR16838@windriver.com>
Date:   Thu, 21 Jan 2021 23:34:22 -0500
From:   Paul Gortmaker <paul.gortmaker@...driver.com>
To:     Yury Norov <yury.norov@...il.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        "Paul E. McKenney" <paulmck@...nel.org>
Subject: Re: [PATCH 1/3] lib: add "all" and "none" as valid ranges to
 bitmap_parselist()

[Re: [PATCH 1/3] lib: add "all" and "none" as valid ranges to bitmap_parselist()] On 21/01/2021 (Thu 16:07) Yury Norov wrote:

> On Thu, Jan 21, 2021 at 2:34 PM Paul Gortmaker
> <paul.gortmaker@...driver.com> wrote:
> >
> > The use of "all" was originally RCU specific - I'd pushed it down to
> > being used for any CPU lists -- then Yuri suggested pushing it down
> > further to be used by any bitmap, which is done here.
> >
> > As a trivial one line extension, we also accept the inverse "none"
> > as a valid alias.
> >
> > Cc: Yury Norov <yury.norov@...il.com>
> > Cc: Peter Zijlstra <peterz@...radead.org>
> > Cc: "Paul E. McKenney" <paulmck@...nel.org>
> > Signed-off-by: Paul Gortmaker <paul.gortmaker@...driver.com>
> > ---
> >  Documentation/admin-guide/kernel-parameters.rst | 11 +++++++++++
> >  lib/bitmap.c                                    |  9 +++++++++
> >  2 files changed, 20 insertions(+)
> >
> > diff --git a/Documentation/admin-guide/kernel-parameters.rst b/Documentation/admin-guide/kernel-parameters.rst
> > index 682ab28b5c94..5e080080b058 100644
> > --- a/Documentation/admin-guide/kernel-parameters.rst
> > +++ b/Documentation/admin-guide/kernel-parameters.rst
> > @@ -68,7 +68,18 @@ 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
> > +
> > +will provide an full/all-set cpu mask for the associated boot argument.
> > +
> > +        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 boot parameter expecting a CPU list.
> 
> My question from v1 is still there: what about the line like
> "none,all", ok ",all,"

Apologies - I must have overlooked that somehow.  Let me address it now.

> or similar? If it's not legal, it should be mentioned in the comment,

OK, it is not legal.  So if desired, I can do this in the code...

 - * Optionally the self-descriptive "all" or "none" can be used.
 + * Optionally the self-descriptive stand alone "all" or "none" can be used.

...and a similar "stand alone" addition in kernel-parameters.rst above?

> if it is legal,
> the corresponding code should go to bitmap_parse_region(), just like for "N".

Non-standalone is not legal.  The strcmp ensures the "all" or "none" are
stand-alone.  And as can be seen in the testing below, any attempt to
combine them with commas or ranges or repeated instances is -EINVAL.
(And I'll look at adding such tests to bitmap_test.c as requested.)

> My personal preference is the latter option.

I'm a bit confused as to the value in adding code for supporting things
like ",all,none,all,,none" and then having to define some policy, like
"last processed takes precedence" or similar.   A strict stand-alone
"all" or "none" and everything else as -EINVAL as per below seems
logical.   Maybe I'm missing something and you can elaborate?

Thanks
Paul.
--

root@...kbox:/sys/fs/cgroup/cpuset/foo# /bin/echo all,none,all > cpuset.cpus
/bin/echo: write error: Invalid argument
root@...kbox:/sys/fs/cgroup/cpuset/foo# /bin/echo none,all > cpuset.cpus
/bin/echo: write error: Invalid argument
root@...kbox:/sys/fs/cgroup/cpuset/foo# /bin/echo all,all > cpuset.cpus
/bin/echo: write error: Invalid argument
root@...kbox:/sys/fs/cgroup/cpuset/foo# /bin/echo all, > cpuset.cpus
/bin/echo: write error: Invalid argument
root@...kbox:/sys/fs/cgroup/cpuset/foo# /bin/echo all > cpuset.cpus
root@...kbox:/sys/fs/cgroup/cpuset/foo# /bin/echo ,none > cpuset.cpus
/bin/echo: write error: Invalid argument
root@...kbox:/sys/fs/cgroup/cpuset/foo# /bin/echo none > cpuset.cpus
root@...kbox:/sys/fs/cgroup/cpuset/foo# /bin/echo 1,3,5,7,9,11,13,15 > cpuset.cpus
root@...kbox:/sys/fs/cgroup/cpuset/foo# cat cpuset.cpus
1,3,5,7,9,11,13,15
root@...kbox:/sys/fs/cgroup/cpuset/foo# /bin/echo 1,3,5,7,9,11,13,all > cpuset.cpus
/bin/echo: write error: Invalid argument
root@...kbox:/sys/fs/cgroup/cpuset/foo# /bin/echo none,3,5,7,9,11,13,15 > cpuset.cpus
/bin/echo: write error: Invalid argument
root@...kbox:/sys/fs/cgroup/cpuset/foo#