Hi Dennis > -----Original Message----- > From: Dennis Zhou [mailto:dennis@xxxxxxxxxx] > Sent: 2019年2月27日 1:04 > To: Christopher Lameter <cl@xxxxxxxxx> > Cc: Peng Fan <peng.fan@xxxxxxx>; tj@xxxxxxxxxx; linux-mm@xxxxxxxxx; > linux-kernel@xxxxxxxxxxxxxxx; van.freenix@xxxxxxxxx > 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@xxxxxxx> Thanks, Peng
