[PATCH v3 2/8] drivers/perf: hisi: Improve the detection of associated CPUs

[Yicong Yang]

On 2024/11/6 19:51, Will Deacon wrote:
> On Wed, Nov 06, 2024 at 04:33:18PM +0800, Yicong Yang wrote:
>> Hi Will,
>>
>> Further comment on this patch?
> 
> You failed to convince me that copying the online mask is safe, but I
> haven't had time to look into that in more depth myself. Saying "It should
> be safe" isn't really enough -- it _must_ be safe!
> 

I assume the "safe" here means we won't have trouble if CPU in the copied cpumask
offlined since we don't synchronize with the cpuhp here. Accessing cpu_online_mask
itself is safe since it's a piece of static memory.

We'll initialize the hisi_pmu::associated_cpus from cpu_online_mask in the below
cases. For other cases the associated CPUs is initalized in the cpuhp callback
and we won't come here.
1) for a PMU does have associated CPUs like L3C PMU, but the associcated CPUs
   not onlined at probe
2) for a PMU has no association like CPA PMU which locates on a SICL. Before this
   patch this kind of PMU'associated CPUs are initliazed in the cpuhp callbacks

For the above 2 cases it is safe since the driver's not using hisi_pmu::assoicated_cpus
directly but combined with cpu_online_masks. PMU indicates the user the preferred CPU
to open events on by "cpumask" sysfs which shows hisi_pmu::on_cpu. It's initialized/updated in:
1) hisi_pmu_init_associated_cpus() by cpumask_local_spread() which tries to
   found a nearest CPU of the node that the device locates
2) cpuhp callbacks we registered where holds the cpumap upate lock.

Actually we allow hisi_pmu::associated_cpus contains offline CPUs which is also
mentioned in the commit.

Based on above it's safe to use the cpu_online_mask here. But I find one case
doesn't covered by this patch - if boot with maxcpus=1, offline CPU X and offline
CPU0 (both are not assocaited CPUs), hisi_pmu::on_cpu will not be updated. Since
hisi_pmu::associated_cpus won't be updated when onlining an unassociated CPU
and hisi_pmu::on_cpu is selected from the intersection of the associated CPUs
and the online CPUs, the intersection will be empty in this case.

I'll see how to handle in this case and update the patch.

Thanks.