Re: [GIT PULL] tracing: Two event pid filtering bug fixes

From: Steven Rostedt
Date: Sat Nov 27 2021 - 15:49:24 EST


On Sat, 27 Nov 2021 12:12:55 -0800
Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:

> On Sat, Nov 27, 2021 at 10:28 AM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> >
> > Two fixes to event pid filtering:
> >
> > - Have created events reflect the current state of pid filtering
> >
> > - Test pid filtering on discard test of recorded logic.
> > (Also clean up the if statement to be cleaner).
>
> I could not parse either of those statements. The second one in
> particular is just a jumble of random words in a random order.

Ouch, rereading that I see what you mean. I guess it was a combination
of Thanksgiving Turkey, then finding out my email isn't going out due
to the updates to mail.kernel.org (which was acting as my smarthost
relay), and my wife wondering where I am (as I wasn't doing the chores
she asked me to), so I was in a bit of a rush to get the pull request
out after it had passed all my tests.

>
> I tried to make it make sense by looking at the commits themselves,
> but who knows. Maybe I made it worse by turning it from
> incomprehensible to actively wrong.

First change:

- If pid filtering is enabled, a flag is set to the file descriptor of
the event for the instance buffer that has pid filtering enabled.
This flag is checked at the time of triggering to know if the event
should be traced or not. The problem was when an event was created
(by module load, kprobe, eprobe, etc), a file descriptor is create
in each instance, but it wasn't updating the pid filtering flag for
that event. Hence, those events would start tracing on every
task and not honor the pid filter.

Second change:

- Even if an event is filtered out, it could have a "trigger"
associated it it (triggers do actions based on the event fields,
like a stack dump into the tracing buffer, or a histogram, or even
disable tracing). Thus, the event is recorded into a temporary
buffer so its fields may be used by the trigger. But after the
trigger has run, it needs to know if that event should be recorded
into the trace buffer or not, so the filtering is executed again (if
the event is to be filtered it is discarded). But this second check
did not take into account pid filtering, and so if an event had a
trigger attached to it, it did not honor the pid filters, and traced
for all processes.

Better?

>
> Please make those explanations more clear in the future, ok?

This was a unique situation. I hope to be better next time.

-- Steve