Message-ID: <20241028010647.38f4847f@rorschach.local.home>
Date: Mon, 28 Oct 2024 01:06:47 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc: linux-kernel@...r.kernel.org, Michael Jeanson <mjeanson@...icios.com>,
 Masami Hiramatsu <mhiramat@...nel.org>, Peter Zijlstra
 <peterz@...radead.org>, Alexei Starovoitov <ast@...nel.org>, Yonghong Song
 <yhs@...com>, "Paul E . McKenney" <paulmck@...nel.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>, Namhyung Kim <namhyung@...nel.org>,
 Andrii Nakryiko <andrii.nakryiko@...il.com>, bpf@...r.kernel.org, Joel
 Fernandes <joel@...lfernandes.org>, Jordan Rife <jrife@...gle.com>, Linus
 Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [RFC PATCH v3 2/3] tracing: Introduce tracepoint_is_syscall()

On Sun, 27 Oct 2024 08:30:54 -0400
Mathieu Desnoyers <mathieu.desnoyers@...icios.com> wrote:
> > 
> > I wonder if we should call it "sleepable" instead? For this patch set
> > do we really care if it's a system call or not? It's really if the
> > tracepoint is sleepable or not that's the issue. System calls are just
> > one user of it, there may be more in the future, and the changes to BPF
> > will still be needed.  
> 
> Remember that syscall tracepoint probes are allowed to handle page
> faults, but should not generally block, otherwise it would postpone the
> grace periods of all RCU tasks trace users.
> 
> So naming this "sleepable" would be misleading, because probes are
> not allowed general blocking, just to handle page faults.

I'm fine with "faultable" too.

> 
> If we look at the history of this tracepoint feature, we went with
> the following naming over the various versions of the patch series:
> 
> 1) Sleepable tracepoints: until we understood that we just want to
>     allow page fault, not general sleeping, so we needed to change
>     the name,
> 
> 2) Faultable tracepoints: until Linus requested that we aim for
>     something that is specific to system calls, rather than a generic
>     thing.
> 
>     https://lore.kernel.org/lkml/CAHk-=wggDLDeTKbhb5hh--x=-DQd69v41137M72m6NOTmbD-cw@mail.gmail.com/

Reading that thread again, I believe that Linus was talking more about
all the infrastructure going around to make a special "faultable"
tracepoint (I could be wrong, and Linus may correct me here). When in
fact, the only user is system calls. But from the BPF POV, it doesn't
care if it's a system call, it cares that it is faultable, and the
check should be on that. Having BPF check if it's a system call is
requiring that BPF knows the implementation details of system call
tracepoints. But if it knows it is faultable, then it needs to do
something special.

> 
> 3) Syscall tracepoints: This is what we currently have.
> 
> > Other than that, I think this could work.  
> 
> Calling this field "sleepable" would be misleading. Calling it "faultable"
> would be a better fit, but based on Linus' request, I'm tempted to stick
> with "syscall" for now.
> 
> Your concern is to name this in a way that is general and future-proof.
> Linus' point was to make it syscall-specific rather than general. My
> position is that we should wait until we face other use-cases (if we
> even do) before consider changing the naming from "syscall" to something
> more generic.

Yes, but that was for the infrastructure itself. It really doesnt' make
sense that BPF needs to know which type of tracepoint can fault. That's
telling BPF, you need to know the implementation of this type of
tracepoint.

-- Steve

Powered by blists - more mailing lists