Re: [PATCH 29/30] Documentation: tracing: add TIF_NEED_RESCHED_LAZY

[Ankur Arora]

Joel Fernandes <joel@xxxxxxxxxxxxxxxxx> writes:

> On Wed, Feb 21, 2024 at 04:43:34PM -0500, Steven Rostedt wrote:
>> On Mon, 12 Feb 2024 21:55:53 -0800
>> Ankur Arora <ankur.a.arora@xxxxxxxxxx> wrote:
>>
>> > Document various combinations of resched flags.
>> >
>> > Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
>> > Cc: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
>> > Cc: Jonathan Corbet <corbet@xxxxxxx>
>> > Originally-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>> > Link: https://lore.kernel.org/lkml/87jzshhexi.ffs@tglx/
>> > Signed-off-by: Ankur Arora <ankur.a.arora@xxxxxxxxxx>
>> > ---
>> >  Documentation/trace/ftrace.rst | 6 +++++-
>> >  1 file changed, 5 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/Documentation/trace/ftrace.rst b/Documentation/trace/ftrace.rst
>> > index 7e7b8ec17934..7f20c0bae009 100644
>> > --- a/Documentation/trace/ftrace.rst
>> > +++ b/Documentation/trace/ftrace.rst
>> > @@ -1036,8 +1036,12 @@ explains which is which.
>> >  		be printed here.
>> >
>> >    need-resched:
>> > -	- 'N' both TIF_NEED_RESCHED and PREEMPT_NEED_RESCHED is set,
>> > +	- 'B' all three, TIF_NEED_RESCHED, TIF_NEED_RESCHED_LAZY and PREEMPT_NEED_RESCHED are set,
>> > +	- 'N' both TIF_NEED_RESCHED and PREEMPT_NEED_RESCHED are set,
>> > +	- 'L' both TIF_NEED_RESCHED_LAZY and PREEMPT_NEED_RESCHED are set,
>> > +	- 'b' both TIF_NEED_RESCHED and TIF_NEED_RESCHED_LAZY are set,
>> >  	- 'n' only TIF_NEED_RESCHED is set,
>> > +	- 'l' only TIF_NEED_RESCHED_LAZY is set,
>> >  	- 'p' only PREEMPT_NEED_RESCHED is set,
>
> One thing I was curious about in current code, under which situation will
> "only PREEMPT_NEED_RESCHED is set" case happen? All the code paths I see set
> the PREEMPT_NEED_RESCHED when then TIF has already been set. That kind of
> makes sense, why enter the scheduler on preempt_enable() unless there was a
> reason to, and TIF should have recorded that reason.

True. And, the only place where we clear PREEMPT_NEED_RESCHED is in
__schedule() after clearing the TIF_NEED_RESCHED* flags.

> So in other words, if 'p' above cannot happen, then it could be removed as a
> potential need-resched stat. If it can happen, I'd love to learn in which
> case?

Yeah, AFAICT the only case we might see it alone is in case of a bug.

> Also considering all users of set_tsk_need_resched() also call the
> set_preempt_need_resched() shortly after, should we add a helper to squash
> the 2 calls and simplify callers?
>
Just to explicitly lay out the reason for these being separate interfaces:
set_tsk_need_resched() can be set from a remote CPU, while
set_preempt_need_resched() (or its folding version) is only meant to be
used on the local CPU.

> There are some "special cases" where only TIF flag need be set (such as setting
> rescheduling from an interrupt or when rescheduling a remote CPU). For those,
> they can directly call the set_tsk_need_resched() like they do now.

The remote case, as you say is always handled in the scheduler. So, maybe
it is worth having a set_need_resched_local_cpu() which takes care of
both of these things but more importantly makes clear the limits of this.

That said, these interfaces have very few users (just RCU, and the s390
page fault handler) and both of these are fairly sophisticated users, so
not sure yet another interface in this area is worth adding.

Thanks

--
ankur