Re: [PATCH] perf/core: move all of the pmu devices into their own location

[Greg Kroah-Hartman]

On Tue, Feb 04, 2025 at 09:06:18AM -0500, Liang, Kan wrote:
> 
> 
> On 2025-02-04 5:16 a.m., Greg Kroah-Hartman wrote:
> > On Tue, Feb 04, 2025 at 09:41:03AM +0200, Alexander Shishkin wrote:
> >> Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> writes:
> >>
> >>> In sysfs, for some reason, all pmu devices seem to show up in the "root"
> >>> of /sys/devices/ making for a confusing mess as these devices are not
> >>> really at the root of the system at all.
> >>>
> >>> Create a fake root devices, "pmu_bus" and place them all under there if
> >>> they do not already have a parent device set, cleaning up sysfs to look
> >>> more sane.
> >>
> >> Yeah, so what happens to the userspace that uses them via /sys/devices/*
> >> directly? Even I have scripts that do that.
> > 
> > You should never be doing that, as you have no idea what type of devices
> > are in that location in the tree.  You should be doing what the
> > documentation says to do, and look in /sys/bus/event_source/devices/
> > instead.  That didn't change here.
> >
> 
> Not just the script, the /sys/devices/ is also used in the current perf
> tool. For example,
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/util/mem-events.c#n192

Ugh, it's ignoring the documentation that perf provides for how to find
these devices.  And it's really really risky to do that, you are
"assuming" that a subdir called "events" is never in any other type of
device.

I'll submit a patch to fix that.

> And the comments and document in the perf tool.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/util/pmu.c#n39
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/Documentation/perf-list.txt#n191

I'll fix that document and the tools as well.

> I think it should bring big impact for the end user, especially when
> they still use an older perf tool and script.

perf is tied to the kernel tree, so we should be ok here.  We can also
take the perf tool fix for all stable kernels as it will be backwards
compatible.

The idea that somehow "events" are better than any other type of device
in the system and belong all individually in the root of the device tree
is a long-standing bug that should be fixed.

thanks,

greg k-h