From: Hao Ge <gehao@kylinos.cn>
Recently discovered this entry while checking kallsyms on ARM64:
ffff800083e509c0 D _shared_alloc_tag
If ARCH_NEEDS_WEAK_PER_CPU is not defined((it is only defined for
s390 and alpha architectures),there's no need to statically define
the percpu variable _shared_alloc_tag.
Therefore,we need to implement isolation for this purpose.
However,currently ARCH_NEEDS_WEAK_PER_CPU is a #define and
is enclosed within the #if defined(MODULE) conditional block.
When building the core kernel code for s390 or alpha architectures,
ARCH_NEEDS_WEAK_PER_CPU remains undefined (as it is gated
by #if defined(MODULE)).However,when building modules for these
architectures,the macro is explicitly defined.
Therefore,we need to make ARCH_NEEDS_WEAK_PER_CPU a Kconfig option.
And replace all instances of ARCH_NEEDS_WEAK_PER_CPU in the kernel
code with MODULE_NEEDS_WEAK_PER_CPU,MODULE_NEEDS_WEAK_PER_CPU might
be a more accurate description,because it was only needed for modules.
Then,when defining the percpu variable _shared_alloc_tag,wrap it
with the CONFIG_ARCH_NEEDS_WEAK_PER_CPU condition.
Therefore, this patch does the following things:
Add the ARCH_NEEDS_WEAK_PER_CPU option to the mm Kconfig file
and enable it for the s390 and alpha architectures.
And replace all instances of ARCH_NEEDS_WEAK_PER_CPU
in the kernel code with MODULE_NEEDS_WEAK_PER_CPU.
Suggested-by: Suren Baghdasaryan <surenb@google.com>
Signed-off-by: Hao Ge <gehao@kylinos.cn>
---
arch/alpha/Kconfig | 1 +
arch/alpha/include/asm/percpu.h | 2 +-
arch/s390/Kconfig | 1 +
arch/s390/include/asm/percpu.h | 2 +-
include/linux/alloc_tag.h | 6 +++---
include/linux/percpu-defs.h | 4 ++--
mm/Kconfig | 4 ++++
7 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig
index 109a4cddcd13..ce9bc842e384 100644
--- a/arch/alpha/Kconfig
+++ b/arch/alpha/Kconfig
@@ -7,6 +7,7 @@ config ALPHA
select ARCH_HAS_DMA_OPS if PCI
select ARCH_MIGHT_HAVE_PC_PARPORT
select ARCH_MIGHT_HAVE_PC_SERIO
+ select ARCH_NEEDS_WEAK_PER_CPU
select ARCH_NO_PREEMPT
select ARCH_NO_SG_CHAIN
select ARCH_USE_CMPXCHG_LOCKREF
diff --git a/arch/alpha/include/asm/percpu.h b/arch/alpha/include/asm/percpu.h
index 6923249f2d49..b164d3720e9e 100644
--- a/arch/alpha/include/asm/percpu.h
+++ b/arch/alpha/include/asm/percpu.h
@@ -11,7 +11,7 @@
* Always use weak definitions for percpu variables in modules.
*/
#if defined(MODULE) && defined(CONFIG_SMP)
-#define ARCH_NEEDS_WEAK_PER_CPU
+#define MODULE_NEEDS_WEAK_PER_CPU
#endif
#include <asm-generic/percpu.h>
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 0c16dc443e2f..716031d22346 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -132,6 +132,7 @@ config S390
select ARCH_INLINE_WRITE_UNLOCK_IRQ
select ARCH_INLINE_WRITE_UNLOCK_IRQRESTORE
select ARCH_MHP_MEMMAP_ON_MEMORY_ENABLE
+ select ARCH_NEEDS_WEAK_PER_CPU
select ARCH_STACKWALK
select ARCH_SUPPORTS_ATOMIC_RMW
select ARCH_SUPPORTS_DEBUG_PAGEALLOC
diff --git a/arch/s390/include/asm/percpu.h b/arch/s390/include/asm/percpu.h
index 84f6b8357b45..0afc7ce9c26f 100644
--- a/arch/s390/include/asm/percpu.h
+++ b/arch/s390/include/asm/percpu.h
@@ -18,7 +18,7 @@
* generate external references.
*/
#if defined(MODULE)
-#define ARCH_NEEDS_WEAK_PER_CPU
+#define MODULE_NEEDS_WEAK_PER_CPU
#endif
/*
diff --git a/include/linux/alloc_tag.h b/include/linux/alloc_tag.h
index 8f7931eb7d16..f349cca0ebed 100644
--- a/include/linux/alloc_tag.h
+++ b/include/linux/alloc_tag.h
@@ -88,7 +88,7 @@ static inline struct alloc_tag *ct_to_alloc_tag(struct codetag *ct)
return container_of(ct, struct alloc_tag, ct);
}
-#ifdef ARCH_NEEDS_WEAK_PER_CPU
+#ifdef MODULE_NEEDS_WEAK_PER_CPU
/*
* When percpu variables are required to be defined as weak, static percpu
* variables can't be used inside a function (see comments for DECLARE_PER_CPU_SECTION).
@@ -102,7 +102,7 @@ DECLARE_PER_CPU(struct alloc_tag_counters, _shared_alloc_tag);
.ct = CODE_TAG_INIT, \
.counters = &_shared_alloc_tag };
-#else /* ARCH_NEEDS_WEAK_PER_CPU */
+#else /* MODULE_NEEDS_WEAK_PER_CPU */
#ifdef MODULE
@@ -123,7 +123,7 @@ DECLARE_PER_CPU(struct alloc_tag_counters, _shared_alloc_tag);
#endif /* MODULE */
-#endif /* ARCH_NEEDS_WEAK_PER_CPU */
+#endif /* MODULE_NEEDS_WEAK_PER_CPU */
DECLARE_STATIC_KEY_MAYBE(CONFIG_MEM_ALLOC_PROFILING_ENABLED_BY_DEFAULT,
mem_alloc_profiling_key);
diff --git a/include/linux/percpu-defs.h b/include/linux/percpu-defs.h
index 0aeb0e276a3e..b4ecfc3a7b2b 100644
--- a/include/linux/percpu-defs.h
+++ b/include/linux/percpu-defs.h
@@ -64,13 +64,13 @@
* 2. Static percpu variables cannot be defined inside a function.
*
* Archs which need weak percpu definitions should define
- * ARCH_NEEDS_WEAK_PER_CPU in asm/percpu.h when necessary.
+ * MODULE_NEEDS_WEAK_PER_CPU in asm/percpu.h when necessary.
*
* To ensure that the generic code observes the above two
* restrictions, if CONFIG_DEBUG_FORCE_WEAK_PER_CPU is set weak
* definition is used for all cases.
*/
-#if defined(ARCH_NEEDS_WEAK_PER_CPU) || defined(CONFIG_DEBUG_FORCE_WEAK_PER_CPU)
+#if defined(MODULE_NEEDS_WEAK_PER_CPU) || defined(CONFIG_DEBUG_FORCE_WEAK_PER_CPU)
/*
* __pcpu_scope_* dummy variable is used to enforce scope. It
* receives the static modifier when it's used in front of
diff --git a/mm/Kconfig b/mm/Kconfig
index e113f713b493..2f55cc95cfcb 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -929,6 +929,10 @@ config ARCH_SUPPORTS_PUD_PFNMAP
def_bool y
depends on ARCH_SUPPORTS_HUGE_PFNMAP && HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
+# s390 and alpha should be enabled,see comments for DECLARE_PER_CPU_SECTION
+config ARCH_NEEDS_WEAK_PER_CPU
+ bool
+
#
# UP and nommu archs use km based percpu allocator
#
--
2.25.1On 16.06.25 04:29, Hao Ge wrote: > From: Hao Ge <gehao@kylinos.cn> subject is misleading: we are not optimizing anything in this patch, do we? It should probably be called "mm/percpu: rename ARCH_NEEDS_WEAK_PER_CPU to MODULE_NEED_WEAK_PER_CPU" or sth. like that. > > Recently discovered this entry while checking kallsyms on ARM64: > ffff800083e509c0 D _shared_alloc_tag > > If ARCH_NEEDS_WEAK_PER_CPU is not defined((it is only defined for "... is not defined (it is only defined for > s390 and alpha architectures),there's no need to statically define > the percpu variable _shared_alloc_tag. > > Therefore,we need to implement isolation for this purpose. In general, throw in a space after "." and "," to make this easier to parse. > > However,currently ARCH_NEEDS_WEAK_PER_CPU is a #define and > is enclosed within the #if defined(MODULE) conditional block. > > When building the core kernel code for s390 or alpha architectures, > ARCH_NEEDS_WEAK_PER_CPU remains undefined (as it is gated > by #if defined(MODULE)).However,when building modules for these > architectures,the macro is explicitly defined. > > Therefore,we need to make ARCH_NEEDS_WEAK_PER_CPU a Kconfig option. > And replace all instances of ARCH_NEEDS_WEAK_PER_CPU in the kernel > code with MODULE_NEEDS_WEAK_PER_CPU,MODULE_NEEDS_WEAK_PER_CPU might > be a more accurate description,because it was only needed for modules. > Then,when defining the percpu variable _shared_alloc_tag,wrap it > with the CONFIG_ARCH_NEEDS_WEAK_PER_CPU condition. > > Therefore, this patch does the following things: "this patch" should be avoided. "Therefore, let's ..." > Add the ARCH_NEEDS_WEAK_PER_CPU option to the mm Kconfig file > and enable it for the s390 and alpha architectures. > And replace all instances of ARCH_NEEDS_WEAK_PER_CPU > in the kernel code with MODULE_NEEDS_WEAK_PER_CPU. Most of the description here should likely go to patch #2. See below. > > Suggested-by: Suren Baghdasaryan <surenb@google.com> > Signed-off-by: Hao Ge <gehao@kylinos.cn> > --- > arch/alpha/Kconfig | 1 + > arch/alpha/include/asm/percpu.h | 2 +- > arch/s390/Kconfig | 1 + > arch/s390/include/asm/percpu.h | 2 +- > include/linux/alloc_tag.h | 6 +++--- > include/linux/percpu-defs.h | 4 ++-- > mm/Kconfig | 4 ++++ > 7 files changed, 13 insertions(+), 7 deletions(-) > > diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig > index 109a4cddcd13..ce9bc842e384 100644 > --- a/arch/alpha/Kconfig > +++ b/arch/alpha/Kconfig > @@ -7,6 +7,7 @@ config ALPHA > select ARCH_HAS_DMA_OPS if PCI > select ARCH_MIGHT_HAVE_PC_PARPORT > select ARCH_MIGHT_HAVE_PC_SERIO > + select ARCH_NEEDS_WEAK_PER_CPU > select ARCH_NO_PREEMPT > select ARCH_NO_SG_CHAIN > select ARCH_USE_CMPXCHG_LOCKREF > diff --git a/arch/alpha/include/asm/percpu.h b/arch/alpha/include/asm/percpu.h > index 6923249f2d49..b164d3720e9e 100644 > --- a/arch/alpha/include/asm/percpu.h > +++ b/arch/alpha/include/asm/percpu.h > @@ -11,7 +11,7 @@ > * Always use weak definitions for percpu variables in modules. > */ > #if defined(MODULE) && defined(CONFIG_SMP) > -#define ARCH_NEEDS_WEAK_PER_CPU > +#define MODULE_NEEDS_WEAK_PER_CPU > #endif > > #include <asm-generic/percpu.h> > diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig > index 0c16dc443e2f..716031d22346 100644 > --- a/arch/s390/Kconfig > +++ b/arch/s390/Kconfig > @@ -132,6 +132,7 @@ config S390 > select ARCH_INLINE_WRITE_UNLOCK_IRQ > select ARCH_INLINE_WRITE_UNLOCK_IRQRESTORE > select ARCH_MHP_MEMMAP_ON_MEMORY_ENABLE > + select ARCH_NEEDS_WEAK_PER_CPU > select ARCH_STACKWALK > select ARCH_SUPPORTS_ATOMIC_RMW > select ARCH_SUPPORTS_DEBUG_PAGEALLOC > diff --git a/arch/s390/include/asm/percpu.h b/arch/s390/include/asm/percpu.h > index 84f6b8357b45..0afc7ce9c26f 100644 > --- a/arch/s390/include/asm/percpu.h > +++ b/arch/s390/include/asm/percpu.h > @@ -18,7 +18,7 @@ > * generate external references. > */ > #if defined(MODULE) > -#define ARCH_NEEDS_WEAK_PER_CPU > +#define MODULE_NEEDS_WEAK_PER_CPU > #endif > > /* > diff --git a/include/linux/alloc_tag.h b/include/linux/alloc_tag.h > index 8f7931eb7d16..f349cca0ebed 100644 > --- a/include/linux/alloc_tag.h > +++ b/include/linux/alloc_tag.h > @@ -88,7 +88,7 @@ static inline struct alloc_tag *ct_to_alloc_tag(struct codetag *ct) > return container_of(ct, struct alloc_tag, ct); > } > > -#ifdef ARCH_NEEDS_WEAK_PER_CPU > +#ifdef MODULE_NEEDS_WEAK_PER_CPU > /* > * When percpu variables are required to be defined as weak, static percpu > * variables can't be used inside a function (see comments for DECLARE_PER_CPU_SECTION). > @@ -102,7 +102,7 @@ DECLARE_PER_CPU(struct alloc_tag_counters, _shared_alloc_tag); > .ct = CODE_TAG_INIT, \ > .counters = &_shared_alloc_tag }; > > -#else /* ARCH_NEEDS_WEAK_PER_CPU */ > +#else /* MODULE_NEEDS_WEAK_PER_CPU */ > > #ifdef MODULE > > @@ -123,7 +123,7 @@ DECLARE_PER_CPU(struct alloc_tag_counters, _shared_alloc_tag); > > #endif /* MODULE */ > > -#endif /* ARCH_NEEDS_WEAK_PER_CPU */ > +#endif /* MODULE_NEEDS_WEAK_PER_CPU */ > > DECLARE_STATIC_KEY_MAYBE(CONFIG_MEM_ALLOC_PROFILING_ENABLED_BY_DEFAULT, > mem_alloc_profiling_key); > diff --git a/include/linux/percpu-defs.h b/include/linux/percpu-defs.h > index 0aeb0e276a3e..b4ecfc3a7b2b 100644 > --- a/include/linux/percpu-defs.h > +++ b/include/linux/percpu-defs.h > @@ -64,13 +64,13 @@ > * 2. Static percpu variables cannot be defined inside a function. > * > * Archs which need weak percpu definitions should define > - * ARCH_NEEDS_WEAK_PER_CPU in asm/percpu.h when necessary. > + * MODULE_NEEDS_WEAK_PER_CPU in asm/percpu.h when necessary. > * > * To ensure that the generic code observes the above two > * restrictions, if CONFIG_DEBUG_FORCE_WEAK_PER_CPU is set weak > * definition is used for all cases. > */ > -#if defined(ARCH_NEEDS_WEAK_PER_CPU) || defined(CONFIG_DEBUG_FORCE_WEAK_PER_CPU) > +#if defined(MODULE_NEEDS_WEAK_PER_CPU) || defined(CONFIG_DEBUG_FORCE_WEAK_PER_CPU) > /* > * __pcpu_scope_* dummy variable is used to enforce scope. It > * receives the static modifier when it's used in front of > diff --git a/mm/Kconfig b/mm/Kconfig > index e113f713b493..2f55cc95cfcb 100644 > --- a/mm/Kconfig > +++ b/mm/Kconfig > @@ -929,6 +929,10 @@ config ARCH_SUPPORTS_PUD_PFNMAP > def_bool y > depends on ARCH_SUPPORTS_HUGE_PFNMAP && HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD > > +# s390 and alpha should be enabled,see comments for DECLARE_PER_CPU_SECTION > +config ARCH_NEEDS_WEAK_PER_CPU > + bool I agree with Willy that this commit is not ideal -- in particular the "s390 and alpha should be enabled" can get stale easily. If you want to add a comment, rather give guidance why any architecture would want to set this, like: /* Architectures that XXX should set this. */ XXX to be defined. But the bigger problem with this patch is that XXX cannot even be properly defined, because this patch does not make use of CONFIG_ARCH_NEEDS_WEAK_PER_CPU at all. ... that is done in the second patch. So what you could do is move the actual introduction of CONFIG_ARCH_NEEDS_WEAK_PER_CPU to patch #2, where it is actually used, and limit this patch to the rename. Similarly, teak the patch description to reflect only that. -- Cheers, David / dhildenb
On Mon, Jun 16, 2025 at 09:59:09AM +0200, David Hildenbrand wrote: > On 16.06.25 04:29, Hao Ge wrote: > > From: Hao Ge <gehao@kylinos.cn> > > subject is misleading: we are not optimizing anything in this patch, do we? > > It should probably be called > > "mm/percpu: rename ARCH_NEEDS_WEAK_PER_CPU to MODULE_NEED_WEAK_PER_CPU" or > sth. like that. > > > > Add the ARCH_NEEDS_WEAK_PER_CPU option to the mm Kconfig file > > and enable it for the s390 and alpha architectures. > > And replace all instances of ARCH_NEEDS_WEAK_PER_CPU > > in the kernel code with MODULE_NEEDS_WEAK_PER_CPU. > > Most of the description here should likely go to patch #2. See below. ... > So what you could do is move the actual introduction of > CONFIG_ARCH_NEEDS_WEAK_PER_CPU to patch #2, where it is actually used, and > limit this patch to the rename. > > Similarly, teak the patch description to reflect only that. Right, if the patch only renames ARCH_NEEDS_WEAK_PER_CPU to MODULE_NEEDS_WEAK_PER_CPU the description can be as simple as mm/percpu: rename ARCH_NEEDS_WEAK_PER_CPU to MODULE_NEEDS_WEAK_PER_CPU as a preparation for introduction of CONFIG_ARCH_NEEDS_WEAK_PER_CPU. No functional changes. > -- > Cheers, > > David / dhildenb > -- Sincerely yours, Mike.
On Mon, Jun 16, 2025 at 1:55 AM Mike Rapoport <rppt@kernel.org> wrote:
>
> On Mon, Jun 16, 2025 at 09:59:09AM +0200, David Hildenbrand wrote:
> > On 16.06.25 04:29, Hao Ge wrote:
> > > From: Hao Ge <gehao@kylinos.cn>
> >
> > subject is misleading: we are not optimizing anything in this patch, do we?
> >
> > It should probably be called
> >
> > "mm/percpu: rename ARCH_NEEDS_WEAK_PER_CPU to MODULE_NEED_WEAK_PER_CPU" or
> > sth. like that.
> >
> >
> > > Add the ARCH_NEEDS_WEAK_PER_CPU option to the mm Kconfig file
> > > and enable it for the s390 and alpha architectures.
> > > And replace all instances of ARCH_NEEDS_WEAK_PER_CPU
> > > in the kernel code with MODULE_NEEDS_WEAK_PER_CPU.
> >
> > Most of the description here should likely go to patch #2. See below.
>
> ...
>
> > So what you could do is move the actual introduction of
> > CONFIG_ARCH_NEEDS_WEAK_PER_CPU to patch #2, where it is actually used, and
> > limit this patch to the rename.
> >
> > Similarly, teak the patch description to reflect only that.
>
> Right, if the patch only renames ARCH_NEEDS_WEAK_PER_CPU to
> MODULE_NEEDS_WEAK_PER_CPU the description can be as simple as
>
> mm/percpu: rename ARCH_NEEDS_WEAK_PER_CPU to MODULE_NEEDS_WEAK_PER_CPU
>
> as a preparation for introduction of CONFIG_ARCH_NEEDS_WEAK_PER_CPU.
> No functional changes.
Yeah, the title is misleading and the description is too complicated.
Mike's suggested title sounds better to me and for description I would
say something like:
ARCH_NEEDS_WEAK_PER_CPU is currently defined only for modules and
therefore fails to represent requirements of the architecture. This
prevents us using it for conditions which are applicable when building
both modules and the kernel. To handle such conditions, make it a
Kconfig option and add MODULE_NEEDS_WEAK_PER_CPU for the cases when
the condition applies only to modules.
And now that I'm looking at the change I realize that we probably
don't even need a separate MODULE_NEEDS_WEAK_PER_CPU. It will be used
only in one place and can be replaced with:
#if defined(CONFIG_ARCH_NEEDS_WEAK_PER_CPU) && defined(MODULE)
The code inside arch/{alpha|s390}/include/asm/percpu.h that defines
MODULE_NEEDS_WEAK_PER_CPU can be completely removed and in
arch/alpha/Kconfig you can have:
select ARCH_NEEDS_WEAK_PER_CPU if CONFIG_SMP
to preserve CONFIG_SMP dependency.
That seems to me like a nicer cleanup.
>
> > --
> > Cheers,
> >
> > David / dhildenb
> >
>
> --
> Sincerely yours,
> Mike.
On Mon, Jun 16, 2025 at 10:29:17AM +0800, Hao Ge wrote: > +++ b/mm/Kconfig > @@ -929,6 +929,10 @@ config ARCH_SUPPORTS_PUD_PFNMAP > def_bool y > depends on ARCH_SUPPORTS_HUGE_PFNMAP && HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD > > +# s390 and alpha should be enabled,see comments for DECLARE_PER_CPU_SECTION This comment is inappropriate and should be removed. > +config ARCH_NEEDS_WEAK_PER_CPU > + bool > + > # > # UP and nommu archs use km based percpu allocator > # > -- > 2.25.1 > >
On Mon, Jun 16, 2025 at 04:13:55AM +0100, Matthew Wilcox wrote: > On Mon, Jun 16, 2025 at 10:29:17AM +0800, Hao Ge wrote: > > +++ b/mm/Kconfig > > @@ -929,6 +929,10 @@ config ARCH_SUPPORTS_PUD_PFNMAP > > def_bool y > > depends on ARCH_SUPPORTS_HUGE_PFNMAP && HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD > > > > +# s390 and alpha should be enabled,see comments for DECLARE_PER_CPU_SECTION > > This comment is inappropriate and should be removed. Inappropriate? That's not a word I'm accustomed to hearing as an engineering justification. It's referring the reader to context they might want to know about, and other comments they might want to read. Looks like a good comment to me, if perhaps a bit terse.
© 2016 - 2025 Red Hat, Inc.