Re: [External] Re: [PATCH v3 1/1] arch_topology: move parse_acpi_topology() to common code

[Sudeep Holla <sudeep.holla@arm.com>]

On Mon, Sep 22, 2025 at 07:04:05PM +0800, yunhui cui wrote:
> Hi Sudeep,
> 
> On Mon, Sep 22, 2025 at 5:01 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > On Mon, Sep 22, 2025 at 10:18:57AM +0800, yunhui cui wrote:
> > > Hi Sudeep,
> > >
> > > On Fri, Sep 19, 2025 at 10:05 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > [...]
> >
> > > >
> > > > Just thinking if it makes sense keep acpi_cpu_is_threaded generic without
> > > > the need for weak definition.
> > > >
> > > > Additional note: not sure why you haven't moved this under CONFIG_ARM64/RISCV as
> > > > done with other code.
> > > >
> > > > bool __init acpi_cpu_is_threaded(int cpu)
> > > > {
> > > >         int is_threaded = acpi_pptt_cpu_is_thread(cpu);
> > > >
> > > >         /*
> > > >          * if the PPTT doesn't have thread information, check for architecture
> > > >          * specific fallback if available
> > > >          */
> > > >         if (is_threaded < 0)
> > > >                 is_threaded = arch_cpu_is_threaded();
> > > >
> > > >         return !!is_threaded;
> > > > }
> > > >
> > > > Then you can just have the define in
> > > >
> > > > #define arch_cpu_is_threaded() (read_cpuid_mpidr() & MPIDR_MT_BITMASK)
> > > >
> > > > in arch/arm64/include/asm/topology.h
> > > >
> > > > and
> > > >
> > > > +#ifndef arch_cpu_is_threaded
> > > > +#define arch_cpu_is_threaded           (0)
> > > > +#endif
> > > >
> > > > in include/linux/arch_topology.h
> > > >
> > > > Thoughts ?
> > >
> > > If placed in include/linux/arch_topology.h, there is a possibility
> > > that "arch_cpu_is_threaded" will be redefined.
> > >
> >
> > Why is that a problem ? We want arch to override the default definition
> > if and when required.
> 
> Because include/linux/topology.h first includes linux/arch_topology.h,
> and then includes asm/topology.h, a warning will be generated during
> compilation:
> 
> In file included from ./include/linux/topology.h:37,
>                  from ./include/linux/gfp.h:8,
>                  from ./include/linux/xarray.h:16,
>                  from ./include/linux/list_lru.h:14,
>                  from ./include/linux/fs.h:14,
>                  from kernel/events/core.c:11:
> ./arch/arm64/include/asm/topology.h:39: warning:
> "arch_cpu_is_threaded" redefined
>    39 | #define arch_cpu_is_threaded() (read_cpuid_mpidr() & MPIDR_MT_BITMASK)
>       |
> In file included from ./include/linux/topology.h:30:
> ./include/linux/arch_topology.h:94: note: this is the location of the
> previous definition
>    94 | #define arch_cpu_is_threaded()  (0)
> 
> Unless #undef arch_cpu_is_threaded is used in arch/arm64/include/asm/topology.h,
> 

The above way of #undef first should be fine.

> So it's better to place
> #ifndef arch_cpu_is_threaded
> #define arch_cpu_is_threaded (0)
> #endif
> in include/asm-generic/topology.h.
>

I don't think asm-generic/topology.h is the right place and also you are
again better on order of inclusion.

-- 
Regards,
Sudeep