[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2025020538-risk-snoring-1b83@gregkh>
Date: Wed, 5 Feb 2025 17:00:49 +0100
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: "Liang, Kan" <kan.liang@...ux.intel.com>
Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Mark Rutland <mark.rutland@....com>, Jiri Olsa <jolsa@...nel.org>,
Ian Rogers <irogers@...gle.com>,
Adrian Hunter <adrian.hunter@...el.com>
Subject: Re: [PATCH] perf/core: move all of the pmu devices into their own
location
On Tue, Feb 04, 2025 at 01:23:51PM -0500, Liang, Kan wrote:
>
>
> On 2025-02-04 11:28 a.m., Greg Kroah-Hartman wrote:
> > 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@...uxfoundation.org> 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
> >>
> >> 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
> >
> > Note, this file looks to work properly, it's just the comment that is
> > incorrect.
> >
> > Any hints on what perf command or test I should run to verify I did get
> > this all right?
>
> For the perf tool, "perf test" should be run at minimum.
Cool, I never noticed that, I'll use that to start with.
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/Documentation/perf-list.txt#n191
> >
> > I'll fix that.
> >
> >> I think it should bring big impact for the end user, especially when
> >> they still use an older perf tool and script.
> >
> > It seems that the majority of the perf code IS looking in the correct
> > place, just mem-events.c seemed wrong.
> >
>
> There should be two more.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/builtin-stat.c#n100
But that's a cpu device, not a event_bus device, right?
Oh, no, wow, yeah, that is an event_bus device. Now that's confusing, I
imagine the sysfs cpu authors get annoyed at that one...
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/arch/x86/util/iostat.c#n35
Ah, missed that one too, thanks!
greg k-h
Powered by blists - more mailing lists