Message-ID: <20200929160633.GA2415204@sasha-vm>
Date:   Tue, 29 Sep 2020 12:06:33 -0400
From:   Sasha Levin <sashal@...nel.org>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Naresh Kamboju <naresh.kamboju@...aro.org>,
        linux- stable <stable@...r.kernel.org>,
        Arnaldo Carvalho de Melo <acme@...hat.com>,
        open list <linux-kernel@...r.kernel.org>,
        Stephane Eranian <eranian@...gle.com>,
        Ian Rogers <irogers@...gle.com>, Jiri Olsa <jolsa@...hat.com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Mark Rutland <mark.rutland@....com>,
        Namhyung Kim <namhyung@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH 4.19 146/245] perf stat: Force error in fallback on :k
 events

On Tue, Sep 29, 2020 at 04:27:17PM +0200, Greg Kroah-Hartman wrote:
>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@...uxfoundation.org> wrote:
>> >
>> > From: Stephane Eranian <eranian@...gle.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@...hat.com.
>> >
>> > Signed-off-by: Stephane Eranian <eranian@...gle.com>
>> > Reviewed-by: Ian Rogers <irogers@...gle.com>
>> > Acked-by: Jiri Olsa <jolsa@...hat.com>
>> > Tested-by: Arnaldo Carvalho de Melo <acme@...hat.com>
>> > Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com>
>> > Cc: Jiri Olsa <jolsa@...hat.com>
>> > Cc: Mark Rutland <mark.rutland@....com>
>> > Cc: Namhyung Kim <namhyung@...nel.org>
>> > Cc: Peter Zijlstra <peterz@...radead.org>
>> > Link: http://lore.kernel.org/lkml/20200414161550.225588-1-irogers@google.com
>> > Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>
>> > Signed-off-by: Sasha Levin <sashal@...nel.org>
>>
>> perf failed on stable rc branch 4.19 on all devices.
>>
>> Reported-by: Naresh Kamboju <naresh.kamboju@...aro.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.

I did, looks like this a new report.

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

Same here. Naresh, does perf builds "out of the box" for you, or do you
carry any patches on top?

-- 
Thanks,
Sasha

Powered by blists - more mailing lists