All of lore.kernel.org
 help / color / mirror / Atom feed
From: Adrian Hunter <adrian.hunter@intel.com>
To: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Andy Lutomirski <luto@amacapital.net>,
	Vince Weaver <vincent.weaver@maine.edu>,
	Thomas Gleixner <tglx@linutronix.de>,
	"H. Peter Anvin" <hpa@linux.intel.com>,
	linux-kernel@vger.kernel.org, Jiri Olsa <jolsa@redhat.com>,
	Stephane Eranian <eranian@google.com>,
	Borislav Petkov <bp@suse.de>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Andi Kleen <ak@linux.intel.com>
Subject: Re: [RFC PATCH] perf: Provide status of known PMUs
Date: Thu, 09 Jul 2015 11:44:55 +0300	[thread overview]
Message-ID: <559E3487.1000600@intel.com> (raw)
In-Reply-To: <20150709081011.GA31953@gmail.com>

On 09/07/15 11:10, Ingo Molnar wrote:
> 
> * Adrian Hunter <adrian.hunter@intel.com> wrote:
> 
>> Known PMUs may not be present for various reasons.
>> Provide a way for the user to know what the reason
>> is.
>>
>> A bus attribute is created for each known PMU beneath
>> a group "known_pmus".  The attribute name is the same
>> as the PMU name.  The value is a string consisting of
>> one or, optionally, two parts: a canonical part, and
>> a driver specific part.  If there are two parts, they
>> are separated by " - ".  The canonical part is one of:
>>
>> 	Supported
>> 	Driver error
>> 	Driver not loaded
>> 	Driver not in kernel config
>> 	Not supported by kernel
>> 	Not supported by hardware
>> 	Wrong vendor
>> 	Wrong architecture
>> 	Unknown status
> 
> Very nice!
> 
>> Example:
>>
>> 	$ cat /sys/bus/event_source/known_pmus/intel_pt
>> 	Supported
> 
> So I only have naming nits. 'Supported' is a bit ambiguous, because it could mean 
> that the PMU is supported but the driver is not active. How about 'Enabled'?
> 
> I'd also make the strings more unambiguously structured, something like:
> 
> 	Enabled
> 	Disabled: Driver error
> 	Disabled: Driver not loaded
> 	Disabled: Driver not in kernel config
> 	Disabled: Not supported by the kernel
> 	Disabled: Not supported by the hardware
> 	Disabled: Not supported by the hardware vendor
> 	Disabled: Not supported by the the architecture
> 	Disabled: Unknown status
> 
> (Note the small changes I did to the text in some places.)

OK

> 
> Also note that I'd suggest not enumerating all the error reasons rigidly - just 
> have a single error code, but a free flowing error string that is provided by the 
> low level driver (and maybe strdup()-ed by the core). That way you can provide 
> very specific error descriptions, without having to change the core every time you 
> need a new category. Agreed?

Drivers can optionally provide a string - the optional second part described above.
For example:

	$ cat /sys/bus/event_source/known_pmus/intel_pt
	Disabled: Not supported by the hardware - ToPA output is not supported on this CPU

Having a finite set of categories allows software to interpret the string for
purposes other than displaying it.  Having an optional second part allows
drivers to detail anything else.


  reply	other threads:[~2015-07-09  8:47 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-09  7:48 [RFC PATCH] perf: Provide status of known PMUs Adrian Hunter
2015-07-09  8:10 ` Ingo Molnar
2015-07-09  8:44   ` Adrian Hunter [this message]
2015-07-09  8:50 ` Peter Zijlstra
2015-07-09  9:26   ` Ingo Molnar
2015-07-09 11:59     ` Peter Zijlstra
2015-07-09 12:32       ` Ingo Molnar
2015-07-09 12:42         ` Peter Zijlstra
2015-07-09 14:47           ` Arnaldo Carvalho de Melo
2015-07-10  8:35           ` Ingo Molnar
2015-07-10 18:59             ` Stephane Eranian
2015-07-09  9:30   ` Adrian Hunter
2015-07-09 11:44     ` Peter Zijlstra

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=559E3487.1000600@intel.com \
    --to=adrian.hunter@intel.com \
    --cc=acme@kernel.org \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=bp@suse.de \
    --cc=eranian@google.com \
    --cc=hpa@linux.intel.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=vincent.weaver@maine.edu \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.