Re: [PATCH] perf, ibs: Check syscall attribute flags

From: Peter Zijlstra
Date: Fri Sep 07 2012 - 12:56:28 EST


On Fri, 2012-09-07 at 18:41 +0200, Robert Richter wrote:
> From 1d037614edef576da441936bd8c917d31f57b179 Mon Sep 17 00:00:00 2001
> From: Robert Richter <robert.richter@xxxxxxx>
> Date: Wed, 25 Jul 2012 19:12:45 +0200
> Subject: [PATCH] perf, ibs: Check syscall attribute flags
>
> Current implementation simply ignores attribute flags. Thus, there is
> no notification to userland of unsupported features. Check syscall's
> attribute flags to let userland know if a feature is supported by the
> kernel. This is also needed to distinguish between future kernels what
> might support a feature.
>
> Cc: <stable@xxxxxxxxxxxxxxx> v3.5..
> Signed-off-by: Robert Richter <robert.richter@xxxxxxx>
> ---
> arch/x86/kernel/cpu/perf_event_amd_ibs.c | 11 +++++++++++
> include/linux/perf_event.h | 2 ++
> 2 files changed, 13 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/perf_event_amd_ibs.c b/arch/x86/kernel/cpu/perf_event_amd_ibs.c
> index 7bfb5be..0456061 100644
> --- a/arch/x86/kernel/cpu/perf_event_amd_ibs.c
> +++ b/arch/x86/kernel/cpu/perf_event_amd_ibs.c
> @@ -215,6 +215,14 @@ static int perf_ibs_init(struct perf_event *event)
> struct perf_ibs *perf_ibs;
> u64 max_cnt, config;
> int ret;
> + struct perf_event_attr notsupp = {

static const ?

> + .exclude_user = 1,
> + .exclude_kernel = 1,
> + .exclude_hv = 1,
> + .exclude_idle = 1,
> + .exclude_host = 1,
> + .exclude_guest = 1,

Ideally we'd grow support for those using SVM entry/exit hooks though.

> + };
>
> perf_ibs = get_ibs_pmu(event->attr.type);
> if (perf_ibs) {
> @@ -229,6 +237,9 @@ static int perf_ibs_init(struct perf_event *event)
> if (event->pmu != &perf_ibs->pmu)
> return -ENOENT;
>
> + if (perf_flags(&event->attr) & perf_flags(&notsupp))
> + return -EINVAL;
> +
> if (config & ~perf_ibs->config_mask)
> return -EINVAL;
>
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 28f9cee..c36a04f 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -304,6 +304,8 @@ struct perf_event_attr {
> __u32 __reserved_2;
> };
>
> +#define perf_flags(attr) (*(&(attr)->read_format + 1))


Another anonymous union shouldn't hurt..

---
include/linux/perf_event.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index cc5e2cd..5df37a0 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -236,7 +236,9 @@ struct perf_event_attr {
__u64 sample_type;
__u64 read_format;

- __u64 disabled : 1, /* off by default */
+ union {
+ __u64 flags;
+ __u64 disabled : 1, /* off by default */
inherit : 1, /* children inherit it */
pinned : 1, /* must always be on PMU */
exclusive : 1, /* only group on PMU */
@@ -272,6 +274,7 @@ struct perf_event_attr {
exclude_callchain_user : 1, /* exclude user callchains */

__reserved_1 : 41;
+ };

union {
__u32 wakeup_events; /* wakeup every n events */

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/