Re: [PATCH 3/3] arm_pmu: rework ACPI probing

[Will Deacon <will@kernel.org>]

On Fri, Sep 30, 2022 at 12:18:44PM +0100, Mark Rutland wrote:
> The current ACPI PMU probing logic tries to associate PMUs with CPUs
> when the CPU is first brought online, in order to handle late hotplug,
> though PMUs are only registered during early boot, and so for late
> hotplugged CPUs this can only associate the CPU with an existing PMU.
> 
> We tried to be clever and the have the arm_pmu_acpi_cpu_starting()
> callback allocate a struct arm_pmu when no matching instance is found,
> in order to avoid duplication of logic. However, as above this doesn't
> do anything useful for late hotplugged CPUs, and this requires us to
> allocate memory in an atomic context, which is especially problematic
> for PREEMPT_RT, as reported by Valentin and Pierre.
> 
> This patch reworks the probing to detect PMUs for all online CPUs in the
> arm_pmu_acpi_probe() function, which is more aligned with how DT probing
> works. The arm_pmu_acpi_cpu_starting() callback only tries to associate
> CPUs with an existing arm_pmu instance, avoiding the problem of
> allocating in atomic context.
> 
> Note that as we didn't previously register PMUs for late-hotplugged
> CPUs, this change doesn't result in a loss of existing functionality,
> though we will now warn when we cannot associate a CPU with a PMU.
> 
> This change allows us to pull the hotplug callback registration into the
> arm_pmu_acpi_probe() function, as we no longer need the callbacks to be
> invoked shortly after probing the boot CPUs, and can register it without
> invoking the calls.
> 
> For the moment the arm_pmu_acpi_init() initcall remains to register the
> SPE PMU, though in future this should probably be moved elsewhere (e.g.
> the arm64 ACPI init code), since this doesn't need to be tied to the
> regular CPU PMU code.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Reported-by: Valentin Schneider <valentin.schneider@arm.com>
> Link: https://lore.kernel.org/r/20210810134127.1394269-2-valentin.schneider@arm.com/
> Reported-by: Pierre Gondois <pierre.gondois@arm.com>
> Link: https://lore.kernel.org/linux-arm-kernel/20220912155105.1443303-1-pierre.gondois@arm.com/
> Cc: Pierre Gondois <pierre.gondois@arm.com>
> Cc: Valentin Schneider <vschneid@redhat.com>
> Cc: Will Deacon <will@kernel.org>
> ---
>  drivers/perf/arm_pmu.c       | 17 ++-----
>  drivers/perf/arm_pmu_acpi.c  | 95 +++++++++++++++++++-----------------
>  include/linux/perf/arm_pmu.h |  1 -
>  3 files changed, 52 insertions(+), 61 deletions(-)

[...]

> @@ -320,13 +320,26 @@ int arm_pmu_acpi_probe(armpmu_init_fn init_fn)
>  	 * For the moment, as with the platform/DT case, we need at least one
>  	 * of a PMU's CPUs to be online at probe time.
>  	 */
> -	for_each_possible_cpu(cpu) {
> +	for_each_online_cpu(cpu) {
>  		struct arm_pmu *pmu = per_cpu(probed_pmus, cpu);
> +		unsigned long cpuid;
>  		char *base_name;
>  
> -		if (!pmu || pmu->name)
> +		/* If we've already probed this CPU, we have nothing to do */
> +		if (pmu)
>  			continue;
>  
> +		pmu = armpmu_alloc();
> +		if (!pmu) {
> +			pr_warn("Unable to allocate PMU for CPU%d\n",
> +				cpu);
> +		}
> +
> +		cpuid = per_cpu(cpu_data, cpu).reg_midr;
> +		pmu->acpi_cpuid = cpuid;

I've queued this, but if armpmu_alloc() fails we now deference NULL here
whereas we should probably propagate the error.

Please can you send a fix on top of for-next/acpi?

Thanks,

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel