On Mon, Jul 11, 2016 at 10:05:39AM -0500, Jeremy Linton wrote:

Hi,

On 07/07/2016 11:21 AM, Mark Rutland wrote:

quoted

Hi Jeremy,

Apologies for the late reply on this.

On Tue, Jun 21, 2016 at 12:11:46PM -0500, Jeremy Linton wrote:

quoted

With heterogeneous PMUs its helpful to know which PMUs are bound
to each CPU. Provide that information with a cpumask sysfs entry
similar to other PMUs.

Have you tested trying to stat on a particular PMU? e.g.

$ perf stat -e armv8_cortex_a53/cpu_cycles/ ls

I found that the presence of a cpumask file would cause (at least some
versions) of perf-stat to hang, and was holding off adding a cpumask
until we had a solution to that.

Nice!

I guess that is more proof that any tiny change can break things..
Another "fix" is to make the cpumap_print_to_pagebuf's first
parameter false which apparently keeps perf from understanding the
cpu mask and forces the user to use numactl, which is ugly because
numactrl doesn't understand the mask in that format either.

If we have a cpumask, it should be in keeping with the usual style.

I guess fixing perf is probably the best bet here...

Indeed.

The major issue is that it's not clear to me how to avoid breaking
existing binaries when adding the cpumask. I suspect that we have to
expose it under a different name. :/

quoted

See [1,2] for more details on that.

quoted

Signed-off-by: Jeremy Linton <redacted>
---
 arch/arm64/kernel/perf_event.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

This should be generic across the arm-pmu code, and so should live under
drivers/perf/.

Which is where i had it initially, but (IIRC) getting to work there
required some core pmu changes that seemed a little ugly.

That was my experience from local refactoring, too.

I guess I didn't like the idea of reallocating the per pmu attr group
or tweaking the pmu core code.. So I just moved it into perf_event
where it fit more naturally.

I don't think we need to allocate the attr group per pmu (we get the PMU
pointer from the dev, so the attr group can be shared).

I think we can share the logic, the cpumask attr, and the cpumask
attr_group in drivers/perf/arm_pmu.c, exposing a common
arm_pmu_cpumask_attr_group pointer via the arm_pmu header.

Then all each driver needs to wire up is a single pointer in its
attr_groups array, which isn't so bad.

Thanks,
Mark.

quoted

Thanks,
Mark.

[1] http://lkml.kernel.org/r/1467907474-3290-1-git-send-email-mark.rutland at arm.com
[2] http://lkml.kernel.org/r/1467907474-3290-2-git-send-email-mark.rutland at arm.com

quoted

diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 356fa6c..dae73ea 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c

@@ -533,6 +533,26 @@ static struct attribute_group armv8_pmuv3_events_attr_group = {

 PMU_FORMAT_ATTR(event, "config:0-9");

+static ssize_t
+cpumask_show(struct device *dev, struct device_attribute *attr, char *page)
+{
+	struct pmu *pmu = dev_get_drvdata(dev);
+	struct arm_pmu *cpu_pmu = container_of(pmu, struct arm_pmu, pmu);
+
+	return cpumap_print_to_pagebuf(true, page, &cpu_pmu->supported_cpus);
+}
+static DEVICE_ATTR_RO(cpumask);
+
+static struct attribute *armv8_pmuv3_attrs[] = {
+	 &dev_attr_cpumask.attr,
+	 NULL,
+};
+
+static struct attribute_group armv8_pmuv3_attr_group = {
+	.attrs = armv8_pmuv3_attrs,
+};
+
+
 static struct attribute *armv8_pmuv3_format_attrs[] = {
 	&format_attr_event.attr,
 	NULL,

@@ -544,6 +564,7 @@ static struct attribute_group armv8_pmuv3_format_attr_group = {
 };

 static const struct attribute_group *armv8_pmuv3_attr_groups[] = {
+	&armv8_pmuv3_attr_group,
 	&armv8_pmuv3_events_attr_group,
 	&armv8_pmuv3_format_attr_group,
 	NULL,
--

2.5.5