Re: [PATCH 1/3] lib: add "all" and "none" as valid ranges to bitmap_parselist()

[Yury Norov]

On Thu, Jan 21, 2021 at 2:34 PM Paul Gortmaker
<paul.gortmaker@xxxxxxxxxxxxx> 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@xxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: "Paul E. McKenney" <paulmck@xxxxxxxxxx>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@xxxxxxxxxxxxx>
> ---
>  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,"
or similar? If it's not legal, it should be mentioned in the comment,
if it is legal,
the corresponding code should go to bitmap_parse_region(), just like for "N".

My personal preference is the latter option.

>  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 75006c4036e9..a1010646fbe5 100644
> --- a/lib/bitmap.c
> +++ b/lib/bitmap.c
> @@ -627,6 +627,7 @@ static const char *bitmap_parse_region(const char *str, struct region *r)
>   * From each group will be used only defined amount of bits.
>   * Syntax: range:used_size/group_size
>   * Example: 0-1023:2/256 ==> 0,1,256,257,512,513,768,769
> + * Optionally the self-descriptive "all" or "none" can be used.
>   *
>   * Returns: 0 on success, -errno on invalid input strings. Error values:
>   *
> @@ -640,8 +641,16 @@ int bitmap_parselist(const char *buf, unsigned long *maskp, int nmaskbits)
>         struct region r;
>         long ret;
>
> +       if (!strcmp(buf, "all")) {
> +               bitmap_fill(maskp, nmaskbits);
> +               return 0;
> +       }
> +
>         bitmap_zero(maskp, nmaskbits);
>
> +       if (!strcmp(buf, "none"))
> +               return 0;
> +
>         while (buf) {
>                 buf = bitmap_find_region(buf);
>                 if (buf == NULL)
> --
> 2.17.1
>