linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
	David Ahern <dsahern@gmail.com>,
	linux-kernel@vger.kernel.org,
	Robert Richter <robert.richter@amd.com>
Subject: Re: [PATCH 3/3 v2] perf tool: give user better message if precise is not supported
Date: Fri, 14 Sep 2012 22:11:38 +0200	[thread overview]
Message-ID: <20120914201138.GA17024@gmail.com> (raw)
In-Reply-To: <20120914200518.GA16872@gmail.com>


* Ingo Molnar <mingo@kernel.org> wrote:

> * Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > On Fri, 2012-09-14 at 11:00 -0700, Arnaldo Carvalho de Melo wrote:
> > > > Understood and there have been suggestions on how to definitely state
> > > > what the kernel side did not like. I like Peter's last suggestion --
> > > > something along the lines of clearing attr on a failure except the
> > > > offending setting.
> > > 
> > > I think ws need to use a new bit 
> > 
> > Quite so, for all the reasons you list. But you like the 
> > general idea? I wasn't sure I did, but it was the only thing I 
> > could come up with that would sort of do what we need it to.
> > 
> > The fact that you destroy the user input is awkward, I don't 
> > think there's another syscall that behaves in this fashion.
> 
> Destroying/clearing stuff looks really hacky.
> 
> Why not use a single error status field, set via a long list 
> of enum error constants, a 'perf errnos'?
> 
> The only real problem with the kernel's syscall error code is 
> that it's not wide enough for historic reasons, so we cannot 
> just create our own errnos. But we can create our errors in 
> the attr just fine and make them finegrained enough so that 
> tooling can figure out what happened exactly when it gets a 
> syscall error.
> 
> Yes, that's old-fashioned technology, but it works. With time 
> we could put some structure into the list of error IDs, to 
> make it easily extensible yet grouped in some fashion, etc.

And we could start this small: just covering the problem at hand 
or so, with proper tooling side support - and then extend it as 
needed. I.e. have something in place that we could ask people 
adding new kernel side error paths to use - and with time old 
code could be enriched with better error codes as well.

perf could in fact internally standardize on a 32-bit wide error 
code, and translate it to -EINVAL at the syscall return level. 
That way we could keep all the simple error paths as they are 
today, with a very little syscall level logic (triggered in 
error cases only, so no overhead in the regular case).

I.e.:

  return -EPERF_CPU_PRECISE_EV_NOTSUPP;

would result in that error code being available to new, improved 
tooling that knows about the new attr field. Old tooling would 
still work as well, it would only see the -EINVAL of the 
syscall.

Or something like this - simple and using existing patterns and 
practices. Anything else will likely fail due to non-use.

Thanks,

	Ingo

  reply	other threads:[~2012-09-14 20:11 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-13 20:59 [PATCH 0/3 v3] perf: precise mode and exclude_guest David Ahern
2012-09-13 20:59 ` [PATCH 1/3] perf tool: precise mode requires exclude_guest David Ahern
2012-10-20  0:55   ` [tip:perf/urgent] perf tool: Precise " tip-bot for David Ahern
2012-09-13 20:59 ` [PATCH 2/3] perf: require exclude_guest to use PEBS - kernel side enforcement David Ahern
2012-10-20  0:56   ` [tip:perf/urgent] perf: Require " tip-bot for Peter Zijlstra
2012-09-13 20:59 ` [PATCH 3/3 v2] perf tool: give user better message if precise is not supported David Ahern
2012-09-14  5:43   ` Ingo Molnar
2012-09-14 11:13     ` David Ahern
2012-09-14 11:36       ` Ingo Molnar
2012-09-14 11:43         ` David Ahern
2012-09-14 18:00           ` Arnaldo Carvalho de Melo
2012-09-14 18:07             ` Peter Zijlstra
2012-09-14 20:05               ` Ingo Molnar
2012-09-14 20:11                 ` Ingo Molnar [this message]
2012-09-14 21:26                   ` Peter Zijlstra
2012-09-17  7:11                     ` Ingo Molnar
2012-10-25  8:01   ` [tip:perf/core] perf tools: Give " tip-bot for David Ahern
2012-09-26  1:24 ` [PATCH 0/3 v3] perf: precise mode and exclude_guest David Ahern
2012-10-09 15:08 ` David Ahern
  -- strict thread matches above, loose matches on Subject: below --
2012-09-12 15:16 [PATCH 0/3 v2] " David Ahern
2012-09-12 15:16 ` [PATCH 3/3 v2] perf tool: give user better message if precise is not supported David Ahern

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=20120914201138.GA17024@gmail.com \
    --to=mingo@kernel.org \
    --cc=acme@ghostprotocols.net \
    --cc=dsahern@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=robert.richter@amd.com \
    /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).