From: Peng Fan <peng.fan@nxp.com>
To: Dennis Zhou <dennis@kernel.org>, Christopher Lameter <cl@linux.com>
Cc: "tj@kernel.org" <tj@kernel.org>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"van.freenix@gmail.com" <van.freenix@gmail.com>
Subject: RE: [PATCH 1/2] percpu: km: remove SMP check
Date: Wed, 27 Feb 2019 13:02:16 +0000 [thread overview]
Message-ID: <AM0PR04MB44814B3BA09388DFF3681E1388740@AM0PR04MB4481.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <20190226170339.GB47262@dennisz-mbp.dhcp.thefacebook.com>
Hi Dennis
> -----Original Message-----
> From: Dennis Zhou [mailto:dennis@kernel.org]
> Sent: 2019年2月27日 1:04
> To: Christopher Lameter <cl@linux.com>
> Cc: Peng Fan <peng.fan@nxp.com>; tj@kernel.org; linux-mm@kvack.org;
> linux-kernel@vger.kernel.org; van.freenix@gmail.com
> Subject: Re: [PATCH 1/2] percpu: km: remove SMP check
>
> On Tue, Feb 26, 2019 at 03:16:44PM +0000, Christopher Lameter wrote:
> > On Mon, 25 Feb 2019, Dennis Zhou wrote:
> >
> > > > @@ -27,7 +27,7 @@
> > > > * chunk size is not aligned. percpu-km code will whine about it.
> > > > */
> > > >
> > > > -#if defined(CONFIG_SMP) &&
> > > > defined(CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK)
> > > > +#if defined(CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK)
> > > > #error "contiguous percpu allocation is incompatible with paged first
> chunk"
> > > > #endif
> > > >
> > > > --
> > > > 2.16.4
> > > >
> > >
> > > Hi,
> > >
> > > I think keeping CONFIG_SMP makes this easier to remember
> > > dependencies rather than having to dig into the config. So this is a NACK
> from me.
> >
> > But it simplifies the code and makes it easier to read.
> >
> >
>
> I think the check isn't quite right after looking at it a little longer.
> Looking at x86, I believe you can compile it with !SMP and
> CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK will still be set. This should
> still work because x86 has an MMU.
You are right, x86 could boots up with NEED_PER_CPU_PAGE_FIRST_CHUNK
=y and SMP=n. Tested with qemu, info as below:
/ # zcat /proc/config.gz | grep NEED_PER_CPU_KM
CONFIG_NEED_PER_CPU_KM=y
/ # zcat /proc/config.gz | grep SMP
CONFIG_BROKEN_ON_SMP=y
# CONFIG_SMP is not set
CONFIG_GENERIC_SMP_IDLE_THREAD=y
/ # zcat /proc/config.gz | grep NEED_PER_CPU_PAGE_FIRST_CHUNK
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
/ # cat /proc/cpuinfo
processor : 0
vendor_id : AuthenticAMD
cpu family : 6
model : 6
model name : QEMU Virtual CPU version 2.5+
stepping : 3
cpu MHz : 3192.613
cache size : 512 KB
fpu : yes
fpu_exception : yes
cpuid level : 13
wp : yes
flags : fpu de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 syscall nx lm nopl cpuid pni cx16 hypervisor lahf_lm svm 3dnowprefetl
bugs : fxsave_leak sysret_ss_attrs spectre_v1 spectre_v2 spec_store_bypass
bogomips : 6385.22
TLB size : 1024 4K pages
clflush size : 64
cache_alignment : 64
address sizes : 42 bits physical, 48 bits virtual
power management:
But from the comments in this file:
"
* - CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK must not be defined. It's
* not compatible with PER_CPU_KM. EMBED_FIRST_CHUNK should work
* fine.
"
I did not read into details why it is not allowed, but x86 could still work with KM
and NEED_PER_CPU_PAGE_FIRST_CHUNK.
>
> I think more correctly it would be something like below, but I don't have the
> time to fully verify it right now.
>
> Thanks,
> Dennis
>
> ---
> diff --git a/mm/percpu-km.c b/mm/percpu-km.c index
> 0f643dc2dc65..69ccad7d9807 100644
> --- a/mm/percpu-km.c
> +++ b/mm/percpu-km.c
> @@ -27,7 +27,7 @@
> * chunk size is not aligned. percpu-km code will whine about it.
> */
>
> -#if defined(CONFIG_SMP) &&
> defined(CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK)
> +#if !defined(CONFIG_MMU) &&
> +defined(CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK)
> #error "contiguous percpu allocation is incompatible with paged first chunk"
> #endif
>
Acked-by: Peng Fan <peng.fan@nxp.com>
Thanks,
Peng
next prev parent reply other threads:[~2019-02-27 13:02 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-24 13:13 [PATCH 1/2] percpu: km: remove SMP check Peng Fan
2019-02-24 13:13 ` [PATCH 2/2] percpu: km: no need to consider pcpu_group_offsets[0] Peng Fan
2019-02-25 15:16 ` dennis
2019-02-26 0:03 ` Peng Fan
2019-02-26 15:15 ` Christopher Lameter
2019-02-26 16:31 ` Dennis Zhou
2019-02-25 15:13 ` [PATCH 1/2] percpu: km: remove SMP check Dennis Zhou
2019-02-25 23:58 ` Peng Fan
2019-02-26 15:16 ` Christopher Lameter
2019-02-26 17:03 ` Dennis Zhou
2019-02-27 13:02 ` Peng Fan [this message]
2019-02-27 16:41 ` Dennis Zhou
2019-03-03 8:49 ` Peng Fan
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=AM0PR04MB44814B3BA09388DFF3681E1388740@AM0PR04MB4481.eurprd04.prod.outlook.com \
--to=peng.fan@nxp.com \
--cc=cl@linux.com \
--cc=dennis@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=tj@kernel.org \
--cc=van.freenix@gmail.com \
/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).