Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752695AbbHZU6a (ORCPT ); Wed, 26 Aug 2015 16:58:30 -0400 Received: from mail.kernel.org ([198.145.29.136]:50424 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751330AbbHZU63 (ORCPT ); Wed, 26 Aug 2015 16:58:29 -0400 Date: Wed, 26 Aug 2015 17:58:23 -0300 From: Arnaldo Carvalho de Melo To: Alexander Shishkin Cc: Ingo Molnar , Johannes Berg , Linus Torvalds , Thomas Gleixner , adrian.hunter@intel.com, Ingo Molnar , Borislav Petkov , Vince Weaver , linux-kernel@vger.kernel.org, Andrew Morton , Peter Zijlstra , "H. Peter Anvin" , Stephane Eranian Subject: Re: [PATCH v2 0/6] perf: Introduce extended syscall error reporting Message-ID: <20150826205823.GB1139@kernel.org> References: <20150825091740.GA23488@gmail.com> <1440495246.2192.13.camel@sipsolutions.net> <20150825100728.GA1820@gmail.com> <1440497981.2192.39.camel@sipsolutions.net> <20150826044948.GC14584@gmail.com> <1440572775.1932.1.camel@sipsolutions.net> <20150826072020.GA19081@gmail.com> <20150826072656.GA19305@gmail.com> <87r3mqgf8w.fsf@ashishki-desk.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87r3mqgf8w.fsf@ashishki-desk.ger.corp.intel.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5774 Lines: 173 Em Wed, Aug 26, 2015 at 07:56:47PM +0300, Alexander Shishkin escreveu: > Ingo Molnar writes: > > > * Ingo Molnar wrote: > > > >> ... but back then I didn't feel like complicating an error recovery ABI for the > >> needs of the 1%, robust error handling is all about simplicity: if it's not > >> simple, tools won't use it. > > > > And note that it needs to be 'simple' in two places for usage to grow naturally: > > > > - the usage site in the kernel > > - the tooling side that recovers the information. > > > > That's why I think that such a form: > > > > return err_str(-EINVAL, "x86/perf: CPU does not support precise sampling"); > > > > is obviously simple on the kernel side as it returns -EINVAL, and is very simple > > on the tooling side as well, if we are allowed to extend prctl(). > > So I hacked stuff a bit [1] to accomodate some of the above > ideas. The below diff shows how these ideas integrate with perf. The > rest is in my github tree. > > - this exterr implementation allows its users to add arbitrary > information to the call site structures and also pretty print them on > the way out; in the example, perf stores perf_event_attr field name > that is the source of trouble; it's a string rather than offsetof(), > because half of our event attribute is a bit field; > - the "way out" doesn't have to be syscall return path (although in the > example it is); > - userspace can fetch the extended error reports via prctl() like you > suggested above; > - error codes are still passed around in the [-EXT_ERRNO..-MAX_ERRNO] > range until they are passed to userspace (which is where ext_err_code() > converts them back to traditional errno.h values). Hey, can we see the builtin-record.c patch please? - Arnaldo > # perf record -e branches -c1 ls > kernel says (0/95): { > "file": "/home/ash/work/linux/arch/x86/kernel/cpu/perf_event.c", > "line": 432, > "code": -95, > "module": "perf/x86", > "message": "BTS sampling not allowed for kernel space" > , "attr_field": "exclude_kernel" > } > > Error: > No hardware sampling interrupt available. > No APIC? If so then you can boot the kernel with the "lapic" boot parameter to force-enable it. > # > > [1] https://github.com/virtuoso/linux-perf/commits/exterr > > >From 1338fe417223cc1d8bc7588d95c6a913993d966f Mon Sep 17 00:00:00 2001 > From: Alexander Shishkin > Date: Wed, 26 Aug 2015 18:36:14 +0300 > Subject: [PATCH] perf: Use extended syscall error reporting somewhat > > Signed-off-by: Alexander Shishkin > --- > arch/x86/kernel/cpu/perf_event.c | 5 ++++- > include/linux/perf_event.h | 14 ++++++++++++++ > kernel/events/core.c | 17 ++++++++++++++++- > 3 files changed, 34 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c > index f56cf074d0..5e8f2edb2c 100644 > --- a/arch/x86/kernel/cpu/perf_event.c > +++ b/arch/x86/kernel/cpu/perf_event.c > @@ -12,6 +12,8 @@ > * For licencing details see kernel-base/COPYING > */ > > +#define EXTERR_MODNAME "perf/x86" > + > #include > #include > #include > @@ -426,7 +428,8 @@ int x86_setup_perfctr(struct perf_event *event) > > /* BTS is currently only allowed for user-mode. */ > if (!attr->exclude_kernel) > - return -EOPNOTSUPP; > + return perf_err(-EOPNOTSUPP, exclude_kernel, > + "BTS sampling not allowed for kernel space"); > > /* disallow bts if conflicting events are present */ > if (x86_add_exclusive(x86_lbr_exclusive_lbr)) > diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h > index 2027809433..eb63074012 100644 > --- a/include/linux/perf_event.h > +++ b/include/linux/perf_event.h > @@ -16,6 +16,20 @@ > > #include > > +#include > + > +struct perf_ext_err_site { > + struct ext_err_site site; > + const char *attr_field; > +}; > + > +#define perf_err(__c, __a, __m) \ > + ({ /* make sure it's a real field before stringifying it */ \ > + struct perf_event_attr __x; (void)__x.__a; \ > + ext_err(perf, __c, __m, \ > + .attr_field = __stringify(__a)); \ > + }) > + > /* > * Kernel-internal data types and definitions: > */ > diff --git a/kernel/events/core.c b/kernel/events/core.c > index ae16867670..5523c623c4 100644 > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -9,6 +9,8 @@ > * For licensing details see kernel-base/COPYING > */ > > +#define EXTERR_MODNAME "perf" > + > #include > #include > #include > @@ -44,11 +46,24 @@ > #include > #include > #include > +#include > > #include "internal.h" > > #include > > +static char *perf_exterr_format(void *site) > +{ > + struct perf_ext_err_site *psite = site; > + char *output; > + > + output = kasprintf(GFP_KERNEL, ",\t\"attr_field\": \"%s\"\n", > + psite->attr_field); > + return output; > +} > + > +DECLARE_EXTERR_DOMAIN(perf, perf_exterr_format); > + > static struct workqueue_struct *perf_wq; > > typedef int (*remote_function_f)(void *); > @@ -8352,7 +8367,7 @@ err_group_fd: > fdput(group); > err_fd: > put_unused_fd(event_fd); > - return err; > + return ext_err_errno(err); > } > > /** > -- > 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/