lkml.org 
[lkml]   [2020]   [Sep]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 4.19 146/245] perf stat: Force error in fallback on :k events
On Tue, Sep 29, 2020 at 07:03:46PM +0530, Naresh Kamboju wrote:
> On Tue, 29 Sep 2020 at 17:54, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > From: Stephane Eranian <eranian@google.com>
> >
> > [ Upstream commit bec49a9e05db3dbdca696fa07c62c52638fb6371 ]
> >
> > When it is not possible for a non-privilege perf command to monitor at
> > the kernel level (:k), the fallback code forces a :u. That works if the
> > event was previously monitoring both levels. But if the event was
> > already constrained to kernel only, then it does not make sense to
> > restrict it to user only.
> >
> > Given the code works by exclusion, a kernel only event would have:
> >
> > attr->exclude_user = 1
> >
> > The fallback code would add:
> >
> > attr->exclude_kernel = 1
> >
> > In the end the end would not monitor in either the user level or kernel
> > level. In other words, it would count nothing.
> >
> > An event programmed to monitor kernel only cannot be switched to user
> > only without seriously warning the user.
> >
> > This patch forces an error in this case to make it clear the request
> > cannot really be satisfied.
> >
> > Behavior with paranoid 1:
> >
> > $ sudo bash -c "echo 1 > /proc/sys/kernel/perf_event_paranoid"
> > $ perf stat -e cycles:k sleep 1
> >
> > Performance counter stats for 'sleep 1':
> >
> > 1,520,413 cycles:k
> >
> > 1.002361664 seconds time elapsed
> >
> > 0.002480000 seconds user
> > 0.000000000 seconds sys
> >
> > Old behavior with paranoid 2:
> >
> > $ sudo bash -c "echo 2 > /proc/sys/kernel/perf_event_paranoid"
> > $ perf stat -e cycles:k sleep 1
> > Performance counter stats for 'sleep 1':
> >
> > 0 cycles:ku
> >
> > 1.002358127 seconds time elapsed
> >
> > 0.002384000 seconds user
> > 0.000000000 seconds sys
> >
> > New behavior with paranoid 2:
> >
> > $ sudo bash -c "echo 2 > /proc/sys/kernel/perf_event_paranoid"
> > $ perf stat -e cycles:k sleep 1
> > Error:
> > You may not have permission to collect stats.
> >
> > Consider tweaking /proc/sys/kernel/perf_event_paranoid,
> > which controls use of the performance events system by
> > unprivileged users (without CAP_PERFMON or CAP_SYS_ADMIN).
> >
> > The current value is 2:
> >
> > -1: Allow use of (almost) all events by all users
> > Ignore mlock limit after perf_event_mlock_kb without CAP_IPC_LOCK
> > >= 0: Disallow ftrace function tracepoint by users without CAP_PERFMON or CAP_SYS_ADMIN
> > Disallow raw tracepoint access by users without CAP_SYS_PERFMON or CAP_SYS_ADMIN
> > >= 1: Disallow CPU event access by users without CAP_PERFMON or CAP_SYS_ADMIN
> > >= 2: Disallow kernel profiling by users without CAP_PERFMON or CAP_SYS_ADMIN
> >
> > To make this setting permanent, edit /etc/sysctl.conf too, e.g.:
> >
> > kernel.perf_event_paranoid = -1
> >
> > v2 of this patch addresses the review feedback from jolsa@redhat.com.
> >
> > Signed-off-by: Stephane Eranian <eranian@google.com>
> > Reviewed-by: Ian Rogers <irogers@google.com>
> > Acked-by: Jiri Olsa <jolsa@redhat.com>
> > Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > Cc: Jiri Olsa <jolsa@redhat.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Namhyung Kim <namhyung@kernel.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Link: http://lore.kernel.org/lkml/20200414161550.225588-1-irogers@google.com
> > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Signed-off-by: Sasha Levin <sashal@kernel.org>
>
> perf failed on stable rc branch 4.19 on all devices.
>
> Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
>
> build warning and errors,
> -----------------------------------
> In file included from util/evlist.h:15:0,
> from util/evsel.c:30:
> util/evsel.c: In function 'perf_evsel__exit':
> util/util.h:25:28: warning: passing argument 1 of 'free' discards
> 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
> #define zfree(ptr) ({ free(*ptr); *ptr = NULL; })
> ^
> util/evsel.c:1293:2: note: in expansion of macro 'zfree'
> zfree(&evsel->pmu_name);
> ^~~~~
> In file included from
> /srv/oe/build/tmp-lkft-glibc/work/intel_corei7_64-linaro-linux/perf/1.0-r9/perf-1.0/tools/perf/arch/x86/include/perf_regs.h:5:0,
> from util/perf_regs.h:27,
> from util/event.h:11,
> from util/callchain.h:8,
> from util/evsel.c:26:
> perf/1.0-r9/recipe-sysroot/usr/include/stdlib.h:563:13: note: expected
> 'void *' but argument is of type 'const char *'
> extern void free (void *__ptr) __THROW;
> ^~~~
> util/evsel.c: In function 'perf_evsel__fallback':
> util/evsel.c:2802:14: error: 'struct perf_evsel' has no member named
> 'core'; did you mean 'node'?
> if (evsel->core.attr.exclude_user)
> ^~~~
> node

I thought Sasha had dropped all of the offending patches. I'll go drop
this one and push out a new 4.19-rc release.

But note, the latest 4.19.y tree doesn't even build perf for me, so I
can't really check this locally :(

thanks,

greg k-h

\
 
 \ /
  Last update: 2020-09-29 16:28    [W:0.697 / U:9.868 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site