2024-04-10 10:04:05

by Yicong Yang

[permalink] [raw]
Subject: [PATCH 1/2] perf: arm_pmu: Only show online CPUs in device's "cpus" attribute

From: Yicong Yang <[email protected]>

When there're CPUs offline after system booting, perf will failed:
[root@localhost ~]# /home/yang/perf stat -a -e armv8_pmuv3_0/cycles/
Error:
The sys_perf_event_open() syscall returned with 19 (No such device) for event (cpu-clock).
/bin/dmesg | grep -i perf may provide additional information.

This is due to PMU's "cpus" is not updated and still contains offline
CPUs and perf will try to open perf event on the offlined CPUs.

Make "cpus" attribute only shows online CPUs and introduced a new
"supported_cpus" where users can get the range of the CPUs this
PMU supported monitoring.

Signed-off-by: Yicong Yang <[email protected]>
---
drivers/perf/arm_pmu.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index 8458fe2cebb4..acbb0e1d0414 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -558,13 +558,35 @@ static ssize_t cpus_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct arm_pmu *armpmu = to_arm_pmu(dev_get_drvdata(dev));
- return cpumap_print_to_pagebuf(true, buf, &armpmu->supported_cpus);
+ cpumask_var_t mask;
+ ssize_t n;
+
+ /* If allocation failed then show the supported_cpus */
+ if (!alloc_cpumask_var(&mask, GFP_KERNEL))
+ return cpumap_print_to_pagebuf(true, buf, &armpmu->supported_cpus);
+
+ cpumask_and(mask, &armpmu->supported_cpus, cpu_online_mask);
+ n = cpumap_print_to_pagebuf(true, buf, mask);
+ free_cpumask_var(mask);
+
+ return n;
}

static DEVICE_ATTR_RO(cpus);

+static ssize_t supported_cpus_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct arm_pmu *armpmu = to_arm_pmu(dev_get_drvdata(dev));
+
+ return cpumap_print_to_pagebuf(true, buf, &armpmu->supported_cpus);
+}
+
+static DEVICE_ATTR_RO(supported_cpus);
+
static struct attribute *armpmu_common_attrs[] = {
&dev_attr_cpus.attr,
+ &dev_attr_supported_cpus.attr,
NULL,
};

--
2.24.0



2024-04-10 15:35:30

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH 1/2] perf: arm_pmu: Only show online CPUs in device's "cpus" attribute

On Wed, Apr 10, 2024 at 05:58:32PM +0800, Yicong Yang wrote:
> From: Yicong Yang <[email protected]>
>
> When there're CPUs offline after system booting, perf will failed:
> [root@localhost ~]# /home/yang/perf stat -a -e armv8_pmuv3_0/cycles/
> Error:
> The sys_perf_event_open() syscall returned with 19 (No such device) for event (cpu-clock).
> /bin/dmesg | grep -i perf may provide additional information.
>
> This is due to PMU's "cpus" is not updated and still contains offline
> CPUs and perf will try to open perf event on the offlined CPUs.
>
> Make "cpus" attribute only shows online CPUs and introduced a new
> "supported_cpus" where users can get the range of the CPUs this
> PMU supported monitoring.
>
> Signed-off-by: Yicong Yang <[email protected]>
> ---
> drivers/perf/arm_pmu.c | 24 +++++++++++++++++++++++-
> 1 file changed, 23 insertions(+), 1 deletion(-)

Hmm. Is the complexity in the driver really worth it here? CPUs can be
onlined and offlined after the perf_event_open() syscall has been
executed, so this feels like something userspace should be aware of and
handle on a best-effort basis anyway.

Does x86 get away with this because CPU0 is never offlined?

Will

2024-04-11 09:17:13

by Yicong Yang

[permalink] [raw]
Subject: Re: [PATCH 1/2] perf: arm_pmu: Only show online CPUs in device's "cpus" attribute

On 2024/4/10 23:34, Will Deacon wrote:
> On Wed, Apr 10, 2024 at 05:58:32PM +0800, Yicong Yang wrote:
>> From: Yicong Yang <[email protected]>
>>
>> When there're CPUs offline after system booting, perf will failed:
>> [root@localhost ~]# /home/yang/perf stat -a -e armv8_pmuv3_0/cycles/
>> Error:
>> The sys_perf_event_open() syscall returned with 19 (No such device) for event (cpu-clock).
>> /bin/dmesg | grep -i perf may provide additional information.
>>
>> This is due to PMU's "cpus" is not updated and still contains offline
>> CPUs and perf will try to open perf event on the offlined CPUs.
>>
>> Make "cpus" attribute only shows online CPUs and introduced a new
>> "supported_cpus" where users can get the range of the CPUs this
>> PMU supported monitoring.
>>
>> Signed-off-by: Yicong Yang <[email protected]>
>> ---
>> drivers/perf/arm_pmu.c | 24 +++++++++++++++++++++++-
>> 1 file changed, 23 insertions(+), 1 deletion(-)
>
> Hmm. Is the complexity in the driver really worth it here? CPUs can be
> onlined and offlined after the perf_event_open() syscall has been
> executed,

Yes. So we have cpuhp callbacks to handle the cpu online/offline
and migrate the perf context.

> so this feels like something userspace should be aware of and
> handle on a best-effort basis anyway.
>

Looks like it's a convention for a PMU device to provide a "cpus" attribute (for core
PMUs) or "cpumask" attribute (for uncore PMUs) to indicates the CPUs on which the
events can be opened. If no such attributes provided, all online CPUs indicated. Perf
will check this and if user doesn't specify a certian range of CPUs the events will
be opened on all the CPUs PMU indicated.

> Does x86 get away with this because CPU0 is never offlined?
>

Checked on my x86 server there's no "cpus" or "cpumask" provided so perf will try
to open the events on all the online CPUs if no CPU range specified. But for their
hybrid platform there do have a "cpus" attribute[1] and it'll be updated when CPU
offline[2].

The arm-cspmu also provides a "cpumask" to indicate supported online CPUs and an
"associated_cpus" to indicated the CPUs related to the PMU.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/events/intel/core.c?h=v6.9-rc1#n5931
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/events/intel/core.c?h=v6.9-rc1#n4949

Thanks.



2024-04-18 16:35:08

by Dongli Zhang

[permalink] [raw]
Subject: Re: [PATCH 1/2] perf: arm_pmu: Only show online CPUs in device's "cpus" attribute



On 4/11/24 01:55, Yicong Yang wrote:
> On 2024/4/10 23:34, Will Deacon wrote:
>> On Wed, Apr 10, 2024 at 05:58:32PM +0800, Yicong Yang wrote:
>>> From: Yicong Yang <[email protected]>
>>>
>>> When there're CPUs offline after system booting, perf will failed:
>>> [root@localhost ~]# /home/yang/perf stat -a -e armv8_pmuv3_0/cycles/
>>> Error:
>>> The sys_perf_event_open() syscall returned with 19 (No such device) for event (cpu-clock).
>>> /bin/dmesg | grep -i perf may provide additional information.
>>>
>>> This is due to PMU's "cpus" is not updated and still contains offline
>>> CPUs and perf will try to open perf event on the offlined CPUs.
>>>
>>> Make "cpus" attribute only shows online CPUs and introduced a new
>>> "supported_cpus" where users can get the range of the CPUs this
>>> PMU supported monitoring.
>>>
>>> Signed-off-by: Yicong Yang <[email protected]>
>>> ---
>>> drivers/perf/arm_pmu.c | 24 +++++++++++++++++++++++-
>>> 1 file changed, 23 insertions(+), 1 deletion(-)
>>
>> Hmm. Is the complexity in the driver really worth it here? CPUs can be
>> onlined and offlined after the perf_event_open() syscall has been
>> executed,
>
> Yes. So we have cpuhp callbacks to handle the cpu online/offline
> and migrate the perf context.
>
>> so this feels like something userspace should be aware of and
>> handle on a best-effort basis anyway.
>>
>
> Looks like it's a convention for a PMU device to provide a "cpus" attribute (for core
> PMUs) or "cpumask" attribute (for uncore PMUs) to indicates the CPUs on which the
> events can be opened. If no such attributes provided, all online CPUs indicated. Perf
> will check this and if user doesn't specify a certian range of CPUs the events will
> be opened on all the CPUs PMU indicated.
>
>> Does x86 get away with this because CPU0 is never offlined?
>>
>
> Checked on my x86 server there's no "cpus" or "cpumask" provided so perf will try
> to open the events on all the online CPUs if no CPU range specified. But for their
> hybrid platform there do have a "cpus" attribute[1] and it'll be updated when CPU
> offline[2].
>
> The arm-cspmu also provides a "cpumask" to indicate supported online CPUs and an
> "associated_cpus" to indicated the CPUs related to the PMU.
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/events/intel/core.c?h=v6.9-rc1#n5931
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/events/intel/core.c?h=v6.9-rc1#n4949
>
> Thanks.
>
>


The arm_dsu has the concepts of 'cpumask' as well. It also has 'associated_cpus'.

When the current cpumask offline, the cpuhp handler will migrate the cpumask to
other associated_cpus.

# cat /sys/devices/arm_dsu_26/associated_cpus
4-5
[root@lse-aarch64-bm-ol8 opc]# cat /sys/devices/arm_dsu_26/cpumask
4

812 static int dsu_pmu_cpu_online(unsigned int cpu, struct hlist_node *node)
813 {
814 struct dsu_pmu *dsu_pmu = hlist_entry_safe(node, struct dsu_pmu,
815 cpuhp_node);
816
817 if (!cpumask_test_cpu(cpu, &dsu_pmu->associated_cpus))
818 return 0;
819
820 /* If the PMU is already managed, there is nothing to do */
821 if (!cpumask_empty(&dsu_pmu->active_cpu))
822 return 0;
823
824 dsu_pmu_init_pmu(dsu_pmu);
825 dsu_pmu_set_active_cpu(cpu, dsu_pmu);
826
827 return 0;
828 }
829
830 static int dsu_pmu_cpu_teardown(unsigned int cpu, struct hlist_node *node)
831 {
832 int dst;
833 struct dsu_pmu *dsu_pmu = hlist_entry_safe(node, struct dsu_pmu,
834 cpuhp_node);
835
836 if (!cpumask_test_and_clear_cpu(cpu, &dsu_pmu->active_cpu))
837 return 0;
838
839 dst = dsu_pmu_get_online_cpu_any_but(dsu_pmu, cpu);
840 /* If there are no active CPUs in the DSU, leave IRQ disabled */
841 if (dst >= nr_cpu_ids)
842 return 0;
843
844 perf_pmu_migrate_context(&dsu_pmu->pmu, cpu, dst);
845 dsu_pmu_set_active_cpu(dst, dsu_pmu);
846
847 return 0;
848 }


However, I think the userspace perf tool looks more friendly (just return <not
supported>) in this case when I offline all CPUs from cpumask of a DSU. Perhaps
because it is NULL now.

# perf stat -e arm_dsu_26/l3d_cache_wb/
^C
Performance counter stats for 'system wide':

<not supported> arm_dsu_26/l3d_cache_wb/

0.553294766 seconds time elapsed


# cat /sys/devices/arm_dsu_26/associated_cpus
4-5
# cat /sys/devices/arm_dsu_26/cpumask
4
# echo 0 > /sys/devices/system/cpu/cpu4/online
# cat /sys/devices/arm_dsu_26/cpumask
5
# echo 0 > /sys/devices/system/cpu/cpu5/online
# cat /sys/devices/arm_dsu_26/cpumask

#

Dongli Zhang