linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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: Fri, 25 Jun 2021 16:38:23 +0200	[thread overview]
Message-ID: <YNXqXwq1+o09eHox@kroah.com> (raw)
In-Reply-To: <540d8a38-da12-56c8-8306-8d3d61ae1d6b@linux.intel.com>

On Fri, Jun 25, 2021 at 07:22:08AM -0700, Andi Kleen wrote:
> 
> On 6/24/2021 10:19 PM, Greg KH wrote:
> > On Thu, Jun 24, 2021 at 10:28:36AM -0700, Andi Kleen wrote:
> > > > But a driver does not caer.  And if perf does not care, who cares?
> > > The users who write scripts that specify the perf events on the perf command
> > > line.
> > Great, then as you have deemed the device name part of your documented
> > api, then keep it stable as that is what you decided to do a long time
> > ago when it was created.
> 
> The fully supported driver keeps it stable, but the driver in fallback mode
> (as in running on a yet unknown system) can't because it doesn't have enough
> information. It has to fall back to the numeric identifiers.
> 
> Supporting yet unknown systems is a big advantage, missing full kernel
> support is the number one reason people can't use uncore monitoring today.
> 
> The symlink keeps some degree of compatibility between the two.

But it creates something that is not needed at the moment, and then
userspace will rely on it.  Don't make userspace rely on it today and
all should be fine.

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.
Don't hard-encode device names into userspace tools, that way lies
madness.

> 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?

thanks,

greg k-h

  reply	other threads:[~2021-06-25 14:38 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-24  1:22 [PATCH 0/7] perf: Add Sapphire Rapids server uncore support kan.liang
2021-06-24  1:22 ` [PATCH 1/7] driver core: Add a way to get to bus devices kset kan.liang
2021-06-24  5:41   ` Greg KH
2021-06-24  1:22 ` [PATCH 2/7] perf: Create a symlink for a PMU kan.liang
2021-06-24  5:48   ` Greg KH
2021-06-24 14:24     ` Andi Kleen
2021-06-24 14:29       ` Greg KH
2021-06-24 15:24         ` Andi Kleen
2021-06-24 15:31           ` Greg KH
2021-06-24 17:07             ` Liang, Kan
2021-06-24 17:35               ` Greg KH
2021-06-24 17:49                 ` Andi Kleen
2021-06-25  5:18                   ` Greg KH
2021-06-25  5:17               ` Greg KH
2021-06-24 17:28             ` Andi Kleen
2021-06-25  5:19               ` Greg KH
2021-06-25 14:22                 ` Andi Kleen
2021-06-25 14:38                   ` Greg KH [this message]
2021-06-25 14:49                     ` Andi Kleen
2021-06-25 15:03                       ` Liang, Kan
2021-06-25 15:44                         ` Andi Kleen
2021-06-25 15:57                           ` Liang, Kan
2021-06-25 16:18                             ` Liang, Kan
2021-06-27 11:02                       ` Greg KH
2021-06-27 16:30                         ` Andi Kleen
2021-06-28  6:55                           ` Greg KH
2021-06-28 15:00                             ` Andi Kleen
2021-06-24  1:22 ` [PATCH 3/7] perf/x86/intel/uncore: Create a symlink for an uncore PMU kan.liang
2021-06-24  5:44   ` Greg KH
2021-06-24  1:22 ` [PATCH 4/7] perf/x86/intel/uncore: Add Sapphire Rapids server support kan.liang
2021-06-24  1:22 ` [PATCH 5/7] perf/x86/intel/uncore: Factor out snr_uncore_mmio_map() kan.liang
2021-06-24  1:22 ` [PATCH 6/7] perf/x86/intel/uncore: Support free-running counters on Sapphire Rapids server kan.liang
2021-06-24  1:22 ` [PATCH 7/7] perf/x86/intel/uncore: Fix invalid unit check kan.liang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YNXqXwq1+o09eHox@kroah.com \
    --to=greg@kroah.com \
    --cc=acme@kernel.org \
    --cc=ak@linux.intel.com \
    --cc=eranian@google.com \
    --cc=jolsa@redhat.com \
    --cc=kan.liang@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).