[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAP-5=fWG7JOmQhTa+OccOxSLhik6Du-qy9d90q-zhDTu0P=q3A@mail.gmail.com>
Date: Wed, 12 Feb 2025 15:17:35 -0800
From: Ian Rogers <irogers@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>, Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>, Kan Liang <kan.liang@...ux.intel.com>,
Hao Ge <gehao@...inos.cn>, James Clark <james.clark@...aro.org>,
Howard Chu <howardchu95@...il.com>, Dominique Martinet <asmadeus@...ewreck.org>,
Levi Yun <yeoreum.yun@....com>, Xu Yang <xu.yang_2@....com>,
Tengda Wu <wutengda@...weicloud.com>, Yang Jihong <yangjihong1@...wei.com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 00/10] Move uid filtering to BPF filters
On Wed, Feb 12, 2025 at 2:56 PM Namhyung Kim <namhyung@...nel.org> wrote:
>
> On Wed, Feb 12, 2025 at 12:00:42PM -0800, Ian Rogers wrote:
> > On Wed, Feb 12, 2025 at 10:46 AM Namhyung Kim <namhyung@...nel.org> wrote:
> > > It's not completely broken and works sometimes.
> >
> > No this is the definition of completely broken. If it only works
> > sometimes then you can't use it, we can't put a test on it, there is
> > no point in it. Even when it doesn't fail on perf_event_open, does it
> > work for processes that start after /proc is scanned? No, it is
> > completely broken.
>
> Ok, we have a different definition for it. Let's ignore the imaginary
> users of the broken features. Have you added a test for this change?
>
> Anyway I've tested your change and found some issues:
>
> 1. It silently exited when BPF-skel is not built. Better to put some
> messages at least.
>
> $ sudo ./perf record -u $(id -u) -- sleep 1
>
> 2. Even with BPF-skel, perf record doesn't work well. It did something
> but failed to get sample data for some reason.
>
> $ sudo ./perf record -u $(id -u) -- sleep 1
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.045 MB perf.data ]
>
> Oh, I think you now need to pass -a because it now works in
> system-wide mode and drops samples for other users.
This is a pre-existing problem, no?
Thanks,
Ian
> 3. With BPF-skel, non-root users will see this.
>
> $ ./perf record -u $(id -u) -- sleep 1
> cannot get fd for 'filters' map
> failed to set filter "BPF" on event cycles:P with 13 (Permission denied)
>
> I think it's confusing and better to tell user that you need to run
> 'perf record --setup-filter pin' as root first. But maybe due to the
> issue #2, you still need to run it as root.
>
> Thanks,
> Namhyung
>
Powered by blists - more mailing lists