linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Paul Gortmaker <paul.gortmaker@windriver.com>
To: Yury Norov <yury.norov@gmail.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	lizefan@huawei.com, Ingo Molnar <mingo@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	josh@joshtriplett.org, Peter Zijlstra <peterz@infradead.org>,
	"Paul E. McKenney" <paulmck@kernel.org>,
	fweisbec@gmail.com, Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Subject: Re: [PATCH v3 0/8] support for bitmap (and hence CPU) list "N" abbreviation
Date: Wed, 27 Jan 2021 04:12:38 -0500	[thread overview]
Message-ID: <20210127091238.GH23530@windriver.com> (raw)
In-Reply-To: <CAAH8bW-6GQkx67=iiwozKwG=4b4rJ8sqNB3UrMeS5eregNvkzA@mail.gmail.com>

[Re: [PATCH v3 0/8] support for bitmap (and hence CPU) list "N" abbreviation] On 26/01/2021 (Tue 14:27) Yury Norov wrote:

> On Tue, Jan 26, 2021 at 9:12 AM Paul Gortmaker
> <paul.gortmaker@windriver.com> wrote:
> >
> > 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.
> 
> I think it's worth moving this sentence to the Documentation. Another

Dynamic nature comment added to Documentation

> caveat with
> N is that users' config may surprisingly become invalid, like if user
> says 32-N, and
> on some machine with a smaller bitmap this config fails to boot.

Updated example to indicate that "16-N" becomes invalid if moved from 32
core system to quad core.  I'm not currently able to think of an example
where boot will fail -- vs. a subsystem getting -EINVAL from bitmap code
and printing a subsystem error instead.

> It doesn't mean of course that I'm against 'N'. I think it's very
> useful especially in
> such common cases like "N", "0-N", "1-N".
> 
> Would it make sense to treat the mask "32-N" when N < 32 as N-N,
> and bark something in dmesg?

I don't think so.  For the same reasons you used to convince me -- that N
should be treated as just another number and not have special rules.

If I boot now, with "important_cpu="32-3" on a quad core then I get what
I get for being stupid.   We don't special case that and subsitute in a
"3-3" (which would then be "3") -- and nor should we!

Sticking to the CPU example, we have no idea what the caller's use case
is -- we don't know if NUMA stuff might be present and whether having
the single CPU #3 in that set is better or worse than EINVAL and no CPUs
in the set.   Expand that to bitmaps in general and we have no idea what
the "right" reaction to garbage input is.

The context of the caller could be simply test_bitmap.c itself -- which
would be expecting the EINVAL, and not some kind of "hot patching" of
the region in order to make it valid.

The only sane option is for the bitmap code to return EINVAL and let the
calling subsystem (with the appropriate context/info) make the decision
as to what to do next.  Which is what the series does now.

Paul.
--

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

      reply	other threads:[~2021-01-27  9:16 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-26 17:11 Paul Gortmaker
2021-01-26 17:11 ` [PATCH 1/8] lib: test_bitmap: clearly separate ERANGE from EINVAL tests Paul Gortmaker
2021-01-26 21:04   ` Andy Shevchenko
2021-01-27  7:21     ` Paul Gortmaker
2021-01-26 17:11 ` [PATCH 2/8] lib: test_bitmap: add more start-end:offset/len tests Paul Gortmaker
2021-01-26 21:11   ` Andy Shevchenko
2021-01-27  3:03   ` Yury Norov
2021-01-26 17:11 ` [PATCH 3/8] lib: bitmap: fold nbits into region struct Paul Gortmaker
2021-01-26 21:16   ` Andy Shevchenko
2021-01-26 21:18     ` Andy Shevchenko
2021-01-27  8:02     ` Paul Gortmaker
2021-01-28  0:47       ` Yury Norov
2021-01-28 10:17         ` Andy Shevchenko
2021-01-27  3:08   ` Yury Norov
2021-01-26 17:11 ` [PATCH 4/8] lib: bitmap: move ERANGE check from set_region to check_region Paul Gortmaker
2021-01-26 21:19   ` Andy Shevchenko
2021-01-27  3:12   ` Yury Norov
2021-01-26 17:11 ` [PATCH 5/8] lib: bitmap_getnum: separate arg into region and field Paul Gortmaker
2021-01-26 21:23   ` Andy Shevchenko
2021-01-27  2:58     ` Yury Norov
2021-01-27  8:38       ` Paul Gortmaker
2021-01-26 17:11 ` [PATCH 6/8] lib: bitmap: support "N" as an alias for size of bitmap Paul Gortmaker
2021-01-26 21:37   ` Andy Shevchenko
2021-01-26 21:41     ` Andy Shevchenko
2021-01-27 17:57       ` Yury Norov
2021-01-27  8:20     ` Paul Gortmaker
2021-01-26 17:11 ` [PATCH 7/8] lib: test_bitmap: add tests for "N" alias 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
2021-01-26 22:27 ` [PATCH v3 0/8] support for bitmap (and hence CPU) list "N" abbreviation Yury Norov
2021-01-27  9:12   ` Paul Gortmaker [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210127091238.GH23530@windriver.com \
    --to=paul.gortmaker@windriver.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=fweisbec@gmail.com \
    --cc=josh@joshtriplett.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=lizefan@huawei.com \
    --cc=mingo@kernel.org \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=yury.norov@gmail.com \
    --subject='Re: [PATCH v3 0/8] support for bitmap (and hence CPU) list "N" abbreviation' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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