Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755967AbbHZQ7B (ORCPT ); Wed, 26 Aug 2015 12:59:01 -0400 Received: from mga14.intel.com ([192.55.52.115]:53056 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751818AbbHZQ7A (ORCPT ); Wed, 26 Aug 2015 12:59:00 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.17,417,1437462000"; d="scan'208";a="791414261" From: Alexander Shishkin To: Ingo Molnar , Johannes Berg Cc: Linus Torvalds , Thomas Gleixner , adrian.hunter@intel.com, Ingo Molnar , Borislav Petkov , Vince Weaver , Arnaldo Carvalho de Melo , 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 In-Reply-To: <20150826072656.GA19305@gmail.com> References: <1440492739.2192.7.camel@sipsolutions.net> <20150825090252.GB22414@gmail.com> <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> User-Agent: Notmuch/0.20.2 (http://notmuchmail.org) Emacs/24.4.1 (x86_64-pc-linux-gnu) Date: Wed, 26 Aug 2015 19:56:47 +0300 Message-ID: <87r3mqgf8w.fsf@ashishki-desk.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5316 Lines: 169 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). # 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/