From: Michal Hocko <mhocko@kernel.org>
To: Yunsheng Lin <linyunsheng@huawei.com>
Cc: will@kernel.org, akpm@linux-foundation.org, rppt@linux.ibm.com,
anshuman.khandual@arm.com, adobriyan@gmail.com, cai@lca.pw,
robin.murphy@arm.com, tglx@linutronix.de,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linuxarm@huawei.com
Subject: Re: [PATCH] arm64: numa: check the node id before accessing node_to_cpumask_map
Date: Fri, 30 Aug 2019 13:13:43 +0200 [thread overview]
Message-ID: <20190830111343.GD28313@dhcp22.suse.cz> (raw)
In-Reply-To: <20839f55-dc99-d2bf-7c60-c37f38232044@huawei.com>
On Fri 30-08-19 17:49:46, Yunsheng Lin wrote:
> On 2019/8/30 16:39, Michal Hocko wrote:
> > On Fri 30-08-19 16:08:14, Yunsheng Lin wrote:
[...]
> >> It seems the cpumask_of_node with CONFIG_DEBUG_PER_CPU_MAPS is used
> >> to catch the erorr case and give a warning to user when node id is not
> >> valid.
> >
> > Yeah the config help text
> > config DEBUG_PER_CPU_MAPS
> > bool "Debug access to per_cpu maps"
> > depends on DEBUG_KERNEL
> > depends on SMP
> > help
> > Say Y to verify that the per_cpu map being accessed has
> > been set up. This adds a fair amount of code to kernel memory
> > and decreases performance.
> >
> > Say N if unsure.
> >
> > suggests that this is intentionally hidden behind a config so a normal
> > path shouldn't really duplicate it. If those checks make sense in
> > general then the config option should be dropped I think.
>
> It seems cpumask_of_node with CONFIG_DEBUG_PER_CPU_MAPS on may be used to
> debug some early use of cpumask_of_node problem, see below:
>
> /*
> * Allocate node_to_cpumask_map based on number of available nodes
> * Requires node_possible_map to be valid.
> *
> * Note: cpumask_of_node() is not valid until after this is done.
> * (Use CONFIG_DEBUG_PER_CPU_MAPS to check this.)
> */
> static void __init setup_node_to_cpumask_map(void)
> {
> int node;
>
> /* setup nr_node_ids if not done yet */
> if (nr_node_ids == MAX_NUMNODES)
> setup_nr_node_ids();
>
> /* allocate and clear the mapping */
> for (node = 0; node < nr_node_ids; node++) {
> alloc_bootmem_cpumask_var(&node_to_cpumask_map[node]);
> cpumask_clear(node_to_cpumask_map[node]);
> }
>
> /* cpumask_of_node() will now work */
> pr_debug("Node to cpumask map for %u nodes\n", nr_node_ids);
> }
>
> So I prefer to keep it as two implementations for arm64 and x86, but
> ensure the two implementations be consistent. It can be cleaned up later
> when there is no use at all.
>
> Is it ok with you?
I am not really sure what you are asking here TBH. You want both
CONFIG_DEBUG_PER_CPU_MAPS implementations to use the same (duplicated) code?
If that is the case then no objections from me. I would obviously
preferred a shared code but that might be a larger change indeed and can
be done later.
--
Michal Hocko
SUSE Labs
prev parent reply other threads:[~2019-08-30 11:13 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-30 2:26 [PATCH] arm64: numa: check the node id before accessing node_to_cpumask_map Yunsheng Lin
2019-08-30 5:55 ` Michal Hocko
2019-08-30 6:35 ` Yunsheng Lin
2019-08-30 6:44 ` Michal Hocko
2019-08-30 8:08 ` Yunsheng Lin
2019-08-30 8:39 ` Michal Hocko
2019-08-30 9:49 ` Yunsheng Lin
2019-08-30 11:13 ` Michal Hocko [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=20190830111343.GD28313@dhcp22.suse.cz \
--to=mhocko@kernel.org \
--cc=adobriyan@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=anshuman.khandual@arm.com \
--cc=cai@lca.pw \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxarm@huawei.com \
--cc=linyunsheng@huawei.com \
--cc=robin.murphy@arm.com \
--cc=rppt@linux.ibm.com \
--cc=tglx@linutronix.de \
--cc=will@kernel.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).