Re: [PATCH v1 0/2] perf: Drop leaked kernel samples

From: Jin, Yao
Date: Fri Jun 15 2018 - 21:18:16 EST

On 6/16/2018 8:56 AM, Kyle Huey wrote:
On Fri, Jun 15, 2018 at 5:50 PM, Jin, Yao <yao.jin@xxxxxxxxxxxxxxx> wrote:
Hi All,

This patch raised many questions, I was prepared. :)

I'd like to try another proposal that it adds a special flag in the returned
perf_sample_data to indicate the perf binary that this sample is a leaked

For example, create a new PERF_SAMPLE_RETURN_LEAKAGE in

In perf_prepare_sample(),

if (event->attr.exclude_kernel && !user_mode(regs))

Now all the samples are kept and the leaked kernel samples are tagged with

In perf binary, it filters out the samples with PERF_SAMPLE_RETURN_LEAKAGE.
It needs perf binary modification but rr doesn't need to be changed.

I don't 0-stuffing some fields because:

1. Keeping the skid info should allow us to look at that if we have
interesting later.

2. Not sure if 0-stuffing some fields has potential conflicts with some

Is this proposal reasonable?

Jin Yao

On 6/16/2018 1:34 AM, Robert O'Callahan wrote:

On Fri, Jun 15, 2018 at 10:16 AM, Kyle Huey <me@xxxxxxxxxxxx> wrote:

If you want a sysctl for your own reasons that's fine. But we don't
want a sysctl. We want to work without any further configuration.

Also toggling a sysctl would require root privileges, but rr does not
currently need to run as root. Thus rr users would have to either
permanently change their system configuration (and every extra
configuration step is a pain), or run rr as root so rr can toggle the
sysctl itself. Running rr as root is highly undesirable.


If the problem you're trying to fix is an inappropriate disclosure of
kernel-space information to user-space, how does filtering the samples
in user space solve anything?

- Kyle

In theory it is. But actually we just use perf tool to look at the sampling result.

And suppose a case, if we want to estimate the skid window, we still need the skid info.

Jin Yao