From: Valentin Schneider <valentin.schneider@arm.com>
To: Jiasheng Jiang <jiasheng@iscas.ac.cn>,
peterz@infradead.org, namit@vmware.com, mingo@kernel.org,
dave.hansen@linux.intel.com
Cc: linux-kernel@vger.kernel.org, Jiasheng Jiang <jiasheng@iscas.ac.cn>
Subject: Re: [PATCH v3] cpumask: Fix implicit type conversion
Date: Thu, 28 Oct 2021 11:48:56 +0100 [thread overview]
Message-ID: <871r45whk7.mognet@arm.com> (raw)
In-Reply-To: <1635404811-2992370-1-git-send-email-jiasheng@iscas.ac.cn>
On 28/10/21 07:06, Jiasheng Jiang wrote:
> The description of the macro in `include/linux/cpumask.h` says the
> variable 'cpu' can be int, whose value ranges from (-2^31) to
> (2^31 - 1).
> However in the for_each_cpu(), 'nr_cpu_ids' and the return value of
> cpumask_next() is unsigned int, whose value ranges from 0 to
> (2^32 - 1).
> If return value of cpumask_next() is (2^31), the restrict
> 'cpu < nr_cpu_ids' can also be statisfied, but the actual value
> of 'cpu' is (-2^31).
> Take amd_pmu_cpu_starting() in `arch/x86/events/amd/core.c` as an
> example. When value of 'cpu' is (-2^31), then in the per_cpu(),
> there will apear __per_cpu_offset[-2^31], which is array out of
> bounds error.
> Moreover, the num of cpu to be the negative doesn't make sense and
> may easily causes trouble.
> It is universally accepted that the implicit type conversion is
> terrible.
> Also, having the good programming custom will set an example for
> others.
> Thus, it might be better to fix the macro description of 'cpu' and
> deal with all the existing issues.
>
AFAIA the upper bounds for NR_CPUS are around 2^12 (arm64) and 2^13 (x86);
I don't think we're anywhere near supporting such massive systems.
I got curious and had a look at the size of .data..percpu on a defconfig
arm64 kernel - I get about ~40KB. So purely on the percpu data side of
things, we're talking about 100TB of RAM...
Trying to improve the code is laudable, but I don't see much incentive in
the churn ATM.
> Fixes: c743f0a ("sched/fair, cpumask: Export for_each_cpu_wrap()")
> Fixes: 8bd93a2 ("rcu: Accelerate grace period if last non-dynticked CPU")
> Fixes: 984f2f3 ("cpumask: introduce new API, without changing anything, v3")
Where's the v1->v2->v3 changelog? This is merely fiddling with doc headers,
what's being fixed here?
prev parent reply other threads:[~2021-10-28 10:49 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-28 7:06 [PATCH v3] cpumask: Fix implicit type conversion Jiasheng Jiang
2021-10-28 10:48 ` Valentin Schneider [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=871r45whk7.mognet@arm.com \
--to=valentin.schneider@arm.com \
--cc=dave.hansen@linux.intel.com \
--cc=jiasheng@iscas.ac.cn \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=namit@vmware.com \
--cc=peterz@infradead.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.