All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Berg <johannes@sipsolutions.net>
To: Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Ingo Molnar <mingo@redhat.com>
Cc: linux-kernel@vger.kernel.org, adrian.hunter@intel.com,
	Arnaldo Carvalho de Melo <acme@infradead.org>,
	Vince Weaver <vince@deater.net>,
	Stephane Eranian <eranian@google.com>
Subject: Re: [PATCH v2 0/6] perf: Introduce extended syscall error reporting
Date: Tue, 25 Aug 2015 10:52:19 +0200	[thread overview]
Message-ID: <1440492739.2192.7.camel@sipsolutions.net> (raw)
In-Reply-To: <1440426780-27227-1-git-send-email-alexander.shishkin@linux.intel.com>

On Mon, 2015-08-24 at 17:32 +0300, Alexander Shishkin wrote:

> This time around, I employed a linker trick to convert the structures
> containing extended error information into integers, which are then
> made to look just like normal error codes so that IS_ERR_VALUE() and
> friends would still work correctly on them. So no extra pointers in
> the struct perf_event or anywhere else; the extended error codes are
> passed around like normal error codes. They only need to be converted
> in syscalls' topmost return statements. This is done in 1/6.
> 

For the record, as we discussed separately, I'd love to see this move
to more general infrastructure. In wireless (nl80211), for example, we
have a few hundred (!) callsites returning -EINVAL, mostly based on
malformed netlink attributes, and it can be very difficult to figure
out what went wrong; debugging mostly employs a variation of Hugh's
trick.

It would be absolutely necessary to support modules, however that could
get tricky: unless we pass a THIS_MODULE through in some way to
netlink_ack() (in the nl80211 case) we'd have to store the actual index
inside the err_site and assign it on module load somehow. Passing the
module pointer seems better, but has even more wrinkles like what to do
when the error came from a nested call (e.g. nla_parse()) that's in a
different modules ...


Unrelated to all that - maybe perf_errno_to_site() should have some
range checking?

johannes

  parent reply	other threads:[~2015-08-25  8:52 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-24 14:32 [PATCH v2 0/6] perf: Introduce extended syscall error reporting Alexander Shishkin
2015-08-24 14:32 ` [PATCH v2 1/6] " Alexander Shishkin
2015-08-31 18:47   ` Andy Shevchenko
2015-09-01  6:38     ` Alexander Shishkin
2015-08-24 14:32 ` [PATCH v2 2/6] perf: Add file name and line number to perf extended error reports Alexander Shishkin
2015-08-24 14:32 ` [PATCH v2 3/6] perf: Annotate some of the error codes with perf_err() Alexander Shishkin
2015-08-24 14:32 ` [PATCH v2 4/6] perf/x86: " Alexander Shishkin
2015-08-24 14:32 ` [PATCH v2 5/6] perf/x86/intel/pt: Use extended error reporting in event initialization Alexander Shishkin
2015-08-24 14:33 ` [PATCH v2 6/6] perf/x86/intel/bts: " Alexander Shishkin
2015-08-25  8:22 ` [PATCH v2 0/6] perf: Introduce extended syscall error reporting Ingo Molnar
2015-08-25  8:52 ` Johannes Berg [this message]
2015-08-25  9:02   ` Ingo Molnar
2015-08-25  9:17     ` Ingo Molnar
2015-08-25  9:34       ` Johannes Berg
2015-08-25 10:07         ` Ingo Molnar
2015-08-25 10:19           ` Johannes Berg
2015-08-26  4:49             ` Ingo Molnar
     [not found]               ` <CA+55aFw--OFczoY=v17+e2-Q3O0GXnMKRuwzpYpB2qKBpZo=fw@mail.gmail.com>
2015-08-26  7:02                 ` Ingo Molnar
2015-08-26  7:06                 ` Johannes Berg
2015-08-26  7:20                   ` Ingo Molnar
2015-08-26  7:26                     ` Ingo Molnar
2015-08-26 16:56                       ` Alexander Shishkin
2015-08-26 20:58                         ` Arnaldo Carvalho de Melo
2015-09-11 16:11                           ` Alexander Shishkin
2015-08-26 18:41                       ` Andrew Morton
2015-08-26 20:05                         ` Peter Zijlstra
2015-08-26 20:22                           ` Andrew Morton
2015-08-26 20:50                             ` Vince Weaver
2015-08-26 20:56                               ` Andrew Morton
2015-08-26 21:14                                 ` Vince Weaver
2015-08-28 10:07                             ` Ingo Molnar
2015-08-26 21:04                         ` Arnaldo Carvalho de Melo
2015-08-26  7:36                     ` Johannes Berg
2015-08-26 11:37       ` Alexander Shishkin

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=1440492739.2192.7.camel@sipsolutions.net \
    --to=johannes@sipsolutions.net \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@infradead.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=eranian@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=vince@deater.net \
    /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.