From: Paul Gortmaker <email@example.com> To: Yury Norov <firstname.lastname@example.org> Cc: Linux Kernel Mailing List <email@example.com>, firstname.lastname@example.org, Ingo Molnar <email@example.com>, Thomas Gleixner <firstname.lastname@example.org>, email@example.com, Peter Zijlstra <firstname.lastname@example.org>, "Paul E. McKenney" <email@example.com>, firstname.lastname@example.org, Rasmus Villemoes <email@example.com>, Andy Shevchenko <firstname.lastname@example.org> 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 > <email@example.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://firstname.lastname@example.org/ > > > > [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 <email@example.com> > > Cc: Ingo Molnar <firstname.lastname@example.org> > > Cc: Yury Norov <email@example.com> > > Cc: Thomas Gleixner <firstname.lastname@example.org> > > Cc: Josh Triplett <email@example.com> > > Cc: Peter Zijlstra <firstname.lastname@example.org> > > Cc: "Paul E. McKenney" <email@example.com> > > Cc: Frederic Weisbecker <firstname.lastname@example.org> > > Cc: Rasmus Villemoes <email@example.com> > > Cc: Andy Shevchenko <firstname.lastname@example.org> > > > > --- > > > > 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 > >
prev parent 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 \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --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).