Re: [PATCH 2/7] perf: Create a symlink for a PMU

From: Greg KH <greg@kroah.com>
To: Andi Kleen <ak@linux.intel.com>
Cc: kan.liang@linux.intel.com, peterz@infradead.org,
	mingo@redhat.com, linux-kernel@vger.kernel.org,
	eranian@google.com, namhyung@kernel.org, acme@kernel.org,
	jolsa@redhat.com
Subject: Re: [PATCH 2/7] perf: Create a symlink for a PMU
Date: Sun, 27 Jun 2021 13:02:16 +0200	[thread overview]
Message-ID: <YNhauAgaUxMfTa+c@kroah.com> (raw)
In-Reply-To: <e670abe2-67b9-a602-410a-0c4170796ec7@linux.intel.com>

On Fri, Jun 25, 2021 at 07:49:36AM -0700, Andi Kleen wrote:
> 
> > Device names will change, that's always a given, as the kernel can never
> > always make them the same.  That's why userspace needs to scan the bus
> > for all devices and then pick out the one that it wants to look at.
> 
> In perf the tool doesn't normally know what devices (= pmu) the users want
> to look at. It's all specified on the command line depending on what events
> you want to measure. There's no way for the tool to figure that out on its
> own.
> 
> 
> > Don't hard-encode device names into userspace tools, that way lies
> > madness.
> 
> There's no hard coding in the tools (or at least not for the non json event
> list case). It all comes from the command line. But that is where the
> problem comes from.

Then do not break things by renaming the device name, as you all have
now stated that this name is part of the user/kernel api.

But really, I do not see why this is an issue, why isn't userspace just
properly walking the list of devices and picking the one on this
specific system that they want to look at?

> > > Anyways thinking about it if Greg doesn't want symlinks (even though sysfs
> > > already has symlinks elsewhere), maybe we could just create two devices
> > > without symlinks. Kan, do you think that would work?
> > Do not have 2 different structures represent the same hardware device,
> > that too is a shortcut to madness.
> > 
> > What prevents userspace from handling device names changing today?  Why
> > are you forcing userspace to pick a specific device name at all?
> 
> The way the perf tool works is that you have to specify the names on the
> command line:
> 
> perf stat -a -e uncore_cha/event=1/ ...
> 
> With the numeric identifiers it would be
> 
> perf stat -a -e uncore_type_X_Y/event=1/
> 
> The tool handles it all abstractly.

Great, and that device name is something that is unique per machine.
And per boot.  So why are you suddenly thinking that this name has to be
"stable"?

If you think it does have to be stable, that was your choice, so now you
must keep it stable.  Don't try to mess with symlinks and the like
please, as again, that way lies madness and unmaintainability for the
next 20+ years.

> So yes the user tools itself can handle it. But the problem is that it is
> directly exposed to the users, so the users would need to change all their
> scripts when switching between the two cases. That is what we're trying to
> avoid -- provide them a way that works on both.

But these are different systems!  Why would anyone expect that the
device name is the same on different systems?  If you insist on keeping
the name identical for newer kernel versions, then again, that was your
choice and now you have to do that.  Do not try to work around your own
requirement by using a symlink.

greg k-h