Re: [PATCH 1/8] perf: Allow to block process in syscall tracepoints

From: Peter Zijlstra
Date: Fri Dec 07 2018 - 03:58:55 EST


On Thu, Dec 06, 2018 at 01:19:46PM -0500, Steven Rostedt wrote:
> On Thu, 6 Dec 2018 09:34:00 +0100
> Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>
> > >
> > > I don't understand this.. why are we using schedule_timeout() and all
> > > that?
> >
> > Urgh.. in fact, the more I look at this the more I hate it.
> >
> > We want to block in __perf_output_begin(), but we cannot because both
> > tracepoints and perf will have preemptability disabled down there.
> >
> > So what we do is fail the event, fake the lost count and go all the way
> > up that callstack, detect the failure and then poll-wait and retry.
> >
> > And only do this for a few special events... *yuck*
>
> Since this is a special case, we should add a new option to the perf
> system call that, 1 states that it wants the traced process to block
> (and must have PTRACE permission to do so) and 2, after it reads from
> the buffer, it needs to check a bit that says "this process is blocked,
> please wake it up" and then do another perf call to kick the process to
> continue.
>
> I really dislike the polling too. But because this is not a default
> case, and is a new feature, we can add more infrastructure to make it
> work properly, instead of trying to hack the current method into
> something that does something poorly.

So why are we doing this? What makes the syscall tracepoints so much
more special than many of the others that we need to overhaul our
fundamental design principles for them?

These patches give no justification *what*so*ever* for why we're doing
ugly arse things like this. And why does this, whatever this is, need to
be done in perf?

IOW, what problem are we solving ?