Re: [GIT PULL] csky changes for v5.12-rc1

From: Linus Torvalds <torvalds@linux-foundation.org>
To: Guo Ren <guoren@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Arnd Bergmann <arnd@arndb.de>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-arch <linux-arch@vger.kernel.org>,
	linux-csky@vger.kernel.org
Subject: Re: [GIT PULL] csky changes for v5.12-rc1
Date: Sun, 28 Feb 2021 12:36:29 -0800	[thread overview]
Message-ID: <CAHk-=wjM+kCsKqNdb=c0hKsv=J7-3Q1zmM15vp6_=8S5XfGMtA@mail.gmail.com> (raw)
In-Reply-To: <20210228034300.1090149-1-guoren@kernel.org>

So this is entirely unrelated to the csky pull request, and is more of
a generic "the perf CPU hotplug thing seems a complete mess".

The csky thing is just the latest - of many - that have been bitten by
the mess, and the one that added yet another hotplug state, and
finally made me go "Let's at least talk about this"

For csky, the problem is this:

On Sat, Feb 27, 2021 at 7:43 PM <guoren@kernel.org> wrote:
>
> arch/csky patches for 5.12-rc1
>
>  - Fixup perf probe failed

and in this case it is 398cb92495cc ("csky: Fixup perf probe failed")
in my current -git tree.

But it's also

    cf6acb8bdb1d ("s390/cpumf: Add support for complete counter set extraction")
    dcb5cdf60a1f ("powerpc/perf/hv-gpci: Add cpu hotplug support")
    1a8f0886a600 ("powerpc/perf/hv-24x7: Add cpu hotplug support")
    6b7ce8927b5a ("irqchip: RISC-V per-HART local interrupt controller driver")
    e9b880581d55 ("coresight: cti: Add CPU Hotplug handling to CTI driver")
    e0685fa228fd ("arm64: Retrieve stolen time as paravirtualized guest")
    6282edb72bed ("clocksource/drivers/exynos_mct: Increase priority
over ARM arch timer")
    78f4e932f776 ("x86/microcode, cpuhotplug: Add a microcode loader
CPU hotplug callback")
    72c69dcddce1 ("powerpc/perf: Trace imc events detection and cpuhotplug")
    5861381d4866 ("PM / arch: x86: Rework the
MSR_IA32_ENERGY_PERF_BIAS handling")
    69c32972d593 ("drivers/perf: Add Cavium ThunderX2 SoC UNCORE PMU driver")
    ...

and that's not even the complete list.

Does it really make sense to have this kind of silly enumeration of
many (MANY!) different arch CPU hotplug state indexes, where most of
them are relevant only to that particular architecture..

No, I don't think this is a _problem_, but it's kind of ugly, wouldn't
you agree?

Wouldn't it be better to just reserve N different states for the
architecture hotplug state, and then let each architecture decide how
they want to order them?

Or better yet, make at least some of them architecture-neutral.
Because now there are drivers that clearly are very tied to one
architecture - or SoCs (look at various timer things) - do they really
want or need their own architecture- or SoC-specific hotplug state?
IOW, do we really need all of these:

        CPUHP_AP_ARM_ARCH_TIMER_STARTING,
        CPUHP_AP_ARM_GLOBAL_TIMER_STARTING,
        CPUHP_AP_JCORE_TIMER_STARTING,
        CPUHP_AP_QCOM_TIMER_STARTING,
        CPUHP_AP_TEGRA_TIMER_STARTING,
        CPUHP_AP_ARMADA_TIMER_STARTING,
        CPUHP_AP_MARCO_TIMER_STARTING,
        CPUHP_AP_MIPS_GIC_TIMER_STARTING,
        CPUHP_AP_ARC_TIMER_STARTING,
        CPUHP_AP_RISCV_TIMER_STARTING,
        CPUHP_AP_CLINT_TIMER_STARTING,
        CPUHP_AP_CSKY_TIMER_STARTING,
        CPUHP_AP_HYPERV_TIMER_STARTING,
        CPUHP_AP_KVM_ARM_TIMER_STARTING,
        CPUHP_AP_DUMMY_TIMER_STARTING,

as separate hotplug events?

Whatever. I don't really care deeply, but this just smells a bit to me.

Comments?

                   Linus