[PATCH tip/core/rcu 3/4] rcu-tasks: Make ksoftirqd provide RCU Tasks quiescent states

[PATCH tip/core/rcu 3/4] rcu-tasks: Make ksoftirqd provide RCU Tasks quiescent states

[Paul E. McKenney]

Heavy networking load can cause a CPU to execute continuously and
indefinitely within ksoftirqd, in which case there will be no voluntary
task switches and thus no RCU-tasks quiescent states.  This commit
therefore causes the exiting rcu_softirq_qs() to provide an RCU-tasks
quiescent state.

This of course means that __do_softirq() and its callers cannot be
invoked from within a tracing trampoline.

Reported-by: Toke Høiland-Jørgensen <toke@redhat.com>
Tested-by: Toke Høiland-Jørgensen <toke@redhat.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
---
 kernel/rcu/tree.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 8e78b2430c16..f4daa4e60b14 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -242,6 +242,7 @@ void rcu_softirq_qs(void)
 {
 	rcu_qs();
 	rcu_preempt_deferred_qs(current);
+	rcu_tasks_qs(current, false);
 }
 
 /*
-- 
2.31.1.189.g2e36527f23

Re: [PATCH tip/core/rcu 3/4] rcu-tasks: Make ksoftirqd provide RCU Tasks quiescent states

[Masami Hiramatsu]

Hi Paul,

On Wed, 12 May 2021 11:27:46 -0700
"Paul E. McKenney" <paulmck@kernel.org> wrote:

> Heavy networking load can cause a CPU to execute continuously and
> indefinitely within ksoftirqd, in which case there will be no voluntary
> task switches and thus no RCU-tasks quiescent states.  This commit
> therefore causes the exiting rcu_softirq_qs() to provide an RCU-tasks
> quiescent state.
> 
> This of course means that __do_softirq() and its callers cannot be
> invoked from within a tracing trampoline.

I would like to confirm that you mean "tracing trampoline" here is
the code on the trampoline buffer, not the handler code which is
invoked from the trampoline buffer but it is protected by preempt_disable(),
am I understand correctly?

Thank you,

> 
> Reported-by: Toke Høiland-Jørgensen <toke@redhat.com>
> Tested-by: Toke Høiland-Jørgensen <toke@redhat.com>
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  kernel/rcu/tree.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 8e78b2430c16..f4daa4e60b14 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -242,6 +242,7 @@ void rcu_softirq_qs(void)
>  {
>  	rcu_qs();
>  	rcu_preempt_deferred_qs(current);
> +	rcu_tasks_qs(current, false);
>  }
>  
>  /*
> -- 
> 2.31.1.189.g2e36527f23
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH tip/core/rcu 3/4] rcu-tasks: Make ksoftirqd provide RCU Tasks quiescent states

[Paul E. McKenney]

On Thu, May 13, 2021 at 03:54:17PM +0900, Masami Hiramatsu wrote:
> Hi Paul,
> 
> On Wed, 12 May 2021 11:27:46 -0700
> "Paul E. McKenney" <paulmck@kernel.org> wrote:
> 
> > Heavy networking load can cause a CPU to execute continuously and
> > indefinitely within ksoftirqd, in which case there will be no voluntary
> > task switches and thus no RCU-tasks quiescent states.  This commit
> > therefore causes the exiting rcu_softirq_qs() to provide an RCU-tasks
> > quiescent state.
> > 
> > This of course means that __do_softirq() and its callers cannot be
> > invoked from within a tracing trampoline.
> 
> I would like to confirm that you mean "tracing trampoline" here is
> the code on the trampoline buffer, not the handler code which is
> invoked from the trampoline buffer but it is protected by preempt_disable(),
> am I understand correctly?

Maybe?  ;-)

If the handler code is invoked from the trampoline buffer, but
returns somewhere else, then it is OK for the handler code to invoke
__do_softirq() or its callers.

In addition, if the handler code is invoked from the trampoline buffer is
guaranteed never to be running in the context of the ksoftirqd kthread,
then it is also OK for the handler code to invoke __do_softirq() or
its callers.

Otherwise, if the handler code might return back into the trampoline
buffer and if that code might be running in the context of the ksoftirqd
kthread, invoking __do_softirq() or one of its callers could result in
the trampoline buffer no longer being there when it was returned to.

							Thanx, Paul

> Thank you,
> 
> > 
> > Reported-by: Toke Høiland-Jørgensen <toke@redhat.com>
> > Tested-by: Toke Høiland-Jørgensen <toke@redhat.com>
> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> >  kernel/rcu/tree.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index 8e78b2430c16..f4daa4e60b14 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -242,6 +242,7 @@ void rcu_softirq_qs(void)
> >  {
> >  	rcu_qs();
> >  	rcu_preempt_deferred_qs(current);
> > +	rcu_tasks_qs(current, false);
> >  }
> >  
> >  /*
> > -- 
> > 2.31.1.189.g2e36527f23
> > 
> 
> 
> -- 
> Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH tip/core/rcu 3/4] rcu-tasks: Make ksoftirqd provide RCU Tasks quiescent states

[Masami Hiramatsu]

On Thu, 13 May 2021 07:21:10 -0700
"Paul E. McKenney" <paulmck@kernel.org> wrote:

> On Thu, May 13, 2021 at 03:54:17PM +0900, Masami Hiramatsu wrote:
> > Hi Paul,
> > 
> > On Wed, 12 May 2021 11:27:46 -0700
> > "Paul E. McKenney" <paulmck@kernel.org> wrote:
> > 
> > > Heavy networking load can cause a CPU to execute continuously and
> > > indefinitely within ksoftirqd, in which case there will be no voluntary
> > > task switches and thus no RCU-tasks quiescent states.  This commit
> > > therefore causes the exiting rcu_softirq_qs() to provide an RCU-tasks
> > > quiescent state.
> > > 
> > > This of course means that __do_softirq() and its callers cannot be
> > > invoked from within a tracing trampoline.
> > 
> > I would like to confirm that you mean "tracing trampoline" here is
> > the code on the trampoline buffer, not the handler code which is
> > invoked from the trampoline buffer but it is protected by preempt_disable(),
> > am I understand correctly?
> 
> Maybe?  ;-)
> 
> If the handler code is invoked from the trampoline buffer, but
> returns somewhere else, then it is OK for the handler code to invoke
> __do_softirq() or its callers.
> 
> In addition, if the handler code is invoked from the trampoline buffer is
> guaranteed never to be running in the context of the ksoftirqd kthread,
> then it is also OK for the handler code to invoke __do_softirq() or
> its callers.
> 
> Otherwise, if the handler code might return back into the trampoline
> buffer and if that code might be running in the context of the ksoftirqd
> kthread, invoking __do_softirq() or one of its callers could result in
> the trampoline buffer no longer being there when it was returned to.

Hmm, the optprobe may be involved in this case. It always return to
the trampoline and handler does not disable irqs (only disable preempt).
BTW, what will call the __do_softirq()? Is hardirq safe?

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH tip/core/rcu 3/4] rcu-tasks: Make ksoftirqd provide RCU Tasks quiescent states

[Paul E. McKenney]

On Fri, May 14, 2021 at 02:49:12AM +0900, Masami Hiramatsu wrote:
> On Thu, 13 May 2021 07:21:10 -0700
> "Paul E. McKenney" <paulmck@kernel.org> wrote:
> 
> > On Thu, May 13, 2021 at 03:54:17PM +0900, Masami Hiramatsu wrote:
> > > Hi Paul,
> > > 
> > > On Wed, 12 May 2021 11:27:46 -0700
> > > "Paul E. McKenney" <paulmck@kernel.org> wrote:
> > > 
> > > > Heavy networking load can cause a CPU to execute continuously and
> > > > indefinitely within ksoftirqd, in which case there will be no voluntary
> > > > task switches and thus no RCU-tasks quiescent states.  This commit
> > > > therefore causes the exiting rcu_softirq_qs() to provide an RCU-tasks
> > > > quiescent state.
> > > > 
> > > > This of course means that __do_softirq() and its callers cannot be
> > > > invoked from within a tracing trampoline.
> > > 
> > > I would like to confirm that you mean "tracing trampoline" here is
> > > the code on the trampoline buffer, not the handler code which is
> > > invoked from the trampoline buffer but it is protected by preempt_disable(),
> > > am I understand correctly?
> > 
> > Maybe?  ;-)
> > 
> > If the handler code is invoked from the trampoline buffer, but
> > returns somewhere else, then it is OK for the handler code to invoke
> > __do_softirq() or its callers.
> > 
> > In addition, if the handler code is invoked from the trampoline buffer is
> > guaranteed never to be running in the context of the ksoftirqd kthread,
> > then it is also OK for the handler code to invoke __do_softirq() or
> > its callers.
> > 
> > Otherwise, if the handler code might return back into the trampoline
> > buffer and if that code might be running in the context of the ksoftirqd
> > kthread, invoking __do_softirq() or one of its callers could result in
> > the trampoline buffer no longer being there when it was returned to.
> 
> Hmm, the optprobe may be involved in this case. It always return to
> the trampoline and handler does not disable irqs (only disable preempt).
> BTW, what will call the __do_softirq()? Is hardirq safe?

As long as your code does not explicitly call __do_softirq() or one of
its callers, you should be OK.

Let's suppose that your code takes a hardirq from ksoftirqd context.
In that case, the return-from-irq path will notice the ksoftirqd
context and refrain from calling __do_softirqd().  Life is good.
(See the invoke_softirq() function for more detail.)

On the other hand, if your code takes a hardirq from some non-ksoftirqd
context, and if this hardirq decides to handle softirqs on exit
from the hardirq, the "__this_cpu_read(ksoftirqd) == current" within
__do_softirq() will fail, so that rcu_softirq_qs() will not be called.
Life is still good.

Either way, as long as your handler does not explicitly invoke
__do_softirq(), life is good.

The bad case is when you instrument a function that is invoked in the
context of a ksoftirqd kthread, and the corresponding handler (or
some function that the handler explicitly calls) directly invokes
__do_softirq() or one of its caller.

Is that more helpful?

							Thanx, Paul

Re: [PATCH tip/core/rcu 3/4] rcu-tasks: Make ksoftirqd provide RCU Tasks quiescent states

[Masami Hiramatsu]

Hi Paul,

On Thu, 13 May 2021 12:15:39 -0700
"Paul E. McKenney" <paulmck@kernel.org> wrote:

> On Fri, May 14, 2021 at 02:49:12AM +0900, Masami Hiramatsu wrote:
> > On Thu, 13 May 2021 07:21:10 -0700
> > "Paul E. McKenney" <paulmck@kernel.org> wrote:
> > 
> > > On Thu, May 13, 2021 at 03:54:17PM +0900, Masami Hiramatsu wrote:
> > > > Hi Paul,
> > > > 
> > > > On Wed, 12 May 2021 11:27:46 -0700
> > > > "Paul E. McKenney" <paulmck@kernel.org> wrote:
> > > > 
> > > > > Heavy networking load can cause a CPU to execute continuously and
> > > > > indefinitely within ksoftirqd, in which case there will be no voluntary
> > > > > task switches and thus no RCU-tasks quiescent states.  This commit
> > > > > therefore causes the exiting rcu_softirq_qs() to provide an RCU-tasks
> > > > > quiescent state.
> > > > > 
> > > > > This of course means that __do_softirq() and its callers cannot be
> > > > > invoked from within a tracing trampoline.
> > > > 
> > > > I would like to confirm that you mean "tracing trampoline" here is
> > > > the code on the trampoline buffer, not the handler code which is
> > > > invoked from the trampoline buffer but it is protected by preempt_disable(),
> > > > am I understand correctly?
> > > 
> > > Maybe?  ;-)
> > > 
> > > If the handler code is invoked from the trampoline buffer, but
> > > returns somewhere else, then it is OK for the handler code to invoke
> > > __do_softirq() or its callers.
> > > 
> > > In addition, if the handler code is invoked from the trampoline buffer is
> > > guaranteed never to be running in the context of the ksoftirqd kthread,
> > > then it is also OK for the handler code to invoke __do_softirq() or
> > > its callers.
> > > 
> > > Otherwise, if the handler code might return back into the trampoline
> > > buffer and if that code might be running in the context of the ksoftirqd
> > > kthread, invoking __do_softirq() or one of its callers could result in
> > > the trampoline buffer no longer being there when it was returned to.
> > 
> > Hmm, the optprobe may be involved in this case. It always return to
> > the trampoline and handler does not disable irqs (only disable preempt).
> > BTW, what will call the __do_softirq()? Is hardirq safe?
> 
> As long as your code does not explicitly call __do_softirq() or one of
> its callers, you should be OK.
> 
> Let's suppose that your code takes a hardirq from ksoftirqd context.
> In that case, the return-from-irq path will notice the ksoftirqd
> context and refrain from calling __do_softirqd().  Life is good.
> (See the invoke_softirq() function for more detail.)
> 
> On the other hand, if your code takes a hardirq from some non-ksoftirqd
> context, and if this hardirq decides to handle softirqs on exit
> from the hardirq, the "__this_cpu_read(ksoftirqd) == current" within
> __do_softirq() will fail, so that rcu_softirq_qs() will not be called.
> Life is still good.

Ah, OK. This is good.

> 
> Either way, as long as your handler does not explicitly invoke
> __do_softirq(), life is good.

There should be no such code, I hope. 

> 
> The bad case is when you instrument a function that is invoked in the
> context of a ksoftirqd kthread, and the corresponding handler (or
> some function that the handler explicitly calls) directly invokes
> __do_softirq() or one of its caller.
> 
> Is that more helpful?

OK, I got it. So it would be better to be commented later.
But anyway I can't imagine that there is any reason to call
__do_softirq() inside kprobe handler :)

Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>

Thank you,


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH tip/core/rcu 3/4] rcu-tasks: Make ksoftirqd provide RCU Tasks quiescent states

[Paul E. McKenney]

On Fri, May 14, 2021 at 03:04:31PM +0900, Masami Hiramatsu wrote:
> Hi Paul,
> 
> On Thu, 13 May 2021 12:15:39 -0700
> "Paul E. McKenney" <paulmck@kernel.org> wrote:
> 
> > On Fri, May 14, 2021 at 02:49:12AM +0900, Masami Hiramatsu wrote:
> > > On Thu, 13 May 2021 07:21:10 -0700
> > > "Paul E. McKenney" <paulmck@kernel.org> wrote:
> > > 
> > > > On Thu, May 13, 2021 at 03:54:17PM +0900, Masami Hiramatsu wrote:
> > > > > Hi Paul,
> > > > > 
> > > > > On Wed, 12 May 2021 11:27:46 -0700
> > > > > "Paul E. McKenney" <paulmck@kernel.org> wrote:
> > > > > 
> > > > > > Heavy networking load can cause a CPU to execute continuously and
> > > > > > indefinitely within ksoftirqd, in which case there will be no voluntary
> > > > > > task switches and thus no RCU-tasks quiescent states.  This commit
> > > > > > therefore causes the exiting rcu_softirq_qs() to provide an RCU-tasks
> > > > > > quiescent state.
> > > > > > 
> > > > > > This of course means that __do_softirq() and its callers cannot be
> > > > > > invoked from within a tracing trampoline.
> > > > > 
> > > > > I would like to confirm that you mean "tracing trampoline" here is
> > > > > the code on the trampoline buffer, not the handler code which is
> > > > > invoked from the trampoline buffer but it is protected by preempt_disable(),
> > > > > am I understand correctly?
> > > > 
> > > > Maybe?  ;-)
> > > > 
> > > > If the handler code is invoked from the trampoline buffer, but
> > > > returns somewhere else, then it is OK for the handler code to invoke
> > > > __do_softirq() or its callers.
> > > > 
> > > > In addition, if the handler code is invoked from the trampoline buffer is
> > > > guaranteed never to be running in the context of the ksoftirqd kthread,
> > > > then it is also OK for the handler code to invoke __do_softirq() or
> > > > its callers.
> > > > 
> > > > Otherwise, if the handler code might return back into the trampoline
> > > > buffer and if that code might be running in the context of the ksoftirqd
> > > > kthread, invoking __do_softirq() or one of its callers could result in
> > > > the trampoline buffer no longer being there when it was returned to.
> > > 
> > > Hmm, the optprobe may be involved in this case. It always return to
> > > the trampoline and handler does not disable irqs (only disable preempt).
> > > BTW, what will call the __do_softirq()? Is hardirq safe?
> > 
> > As long as your code does not explicitly call __do_softirq() or one of
> > its callers, you should be OK.
> > 
> > Let's suppose that your code takes a hardirq from ksoftirqd context.
> > In that case, the return-from-irq path will notice the ksoftirqd
> > context and refrain from calling __do_softirqd().  Life is good.
> > (See the invoke_softirq() function for more detail.)
> > 
> > On the other hand, if your code takes a hardirq from some non-ksoftirqd
> > context, and if this hardirq decides to handle softirqs on exit
> > from the hardirq, the "__this_cpu_read(ksoftirqd) == current" within
> > __do_softirq() will fail, so that rcu_softirq_qs() will not be called.
> > Life is still good.
> 
> Ah, OK. This is good.
> 
> > 
> > Either way, as long as your handler does not explicitly invoke
> > __do_softirq(), life is good.
> 
> There should be no such code, I hope. 
> 
> > 
> > The bad case is when you instrument a function that is invoked in the
> > context of a ksoftirqd kthread, and the corresponding handler (or
> > some function that the handler explicitly calls) directly invokes
> > __do_softirq() or one of its caller.
> > 
> > Is that more helpful?
> 
> OK, I got it. So it would be better to be commented later.
> But anyway I can't imagine that there is any reason to call
> __do_softirq() inside kprobe handler :)
> 
> Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>

I will apply on the next rebase, thank you!

							Thanx, Paul

[PATCH tip/core/rcu 3/4] rcu-tasks: Make ksoftirqd provide RCU Tasks quiescent states

[Paul E. McKenney]

Heavy networking load can cause a CPU to execute continuously and
indefinitely within ksoftirqd, in which case there will be no voluntary
task switches and thus no RCU-tasks quiescent states.  This commit
therefore causes the exiting rcu_softirq_qs() to provide an RCU-tasks
quiescent state.

This of course means that __do_softirq() and its callers cannot be
invoked from within a tracing trampoline.

Reported-by: Toke Høiland-Jørgensen <toke@redhat.com>
Tested-by: Toke Høiland-Jørgensen <toke@redhat.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
---
 kernel/rcu/tree.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 8e78b2430c16..f4daa4e60b14 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -242,6 +242,7 @@ void rcu_softirq_qs(void)
 {
 	rcu_qs();
 	rcu_preempt_deferred_qs(current);
+	rcu_tasks_qs(current, false);
 }
 
 /*
-- 
2.31.1.189.g2e36527f23