* [PATCH] rcu/tree: Fix self wakeups for grace period kthread
@ 2019-03-08 9:46 Neeraj Upadhyay
2019-03-12 1:50 ` Steven Rostedt
0 siblings, 1 reply; 6+ messages in thread
From: Neeraj Upadhyay @ 2019-03-08 9:46 UTC (permalink / raw)
To: paulmck, josh, rostedt, mathieu.desnoyers, jiangshanlai
Cc: linux-kernel, Neeraj Upadhyay
Update the code to match the comment that self wakeup of
grace period kthread is allowed from interrupt handler, and
softirq handler, running in the grace period kthread's
context. Present code allows self wakeups from all
interrupt contexts - nmi, softirq and hardirq contexts.
Signed-off-by: Neeraj Upadhyay <neeraju@codeaurora.org>
---
kernel/rcu/tree.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index acd6ccf..57cac6d 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -1585,7 +1585,7 @@ static bool rcu_future_gp_cleanup(struct rcu_node *rnp)
static void rcu_gp_kthread_wake(void)
{
if ((current == rcu_state.gp_kthread &&
- !in_interrupt() && !in_serving_softirq()) ||
+ !in_irq() && !in_serving_softirq()) ||
!READ_ONCE(rcu_state.gp_flags) ||
!rcu_state.gp_kthread)
return;
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of the Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] rcu/tree: Fix self wakeups for grace period kthread
2019-03-08 9:46 [PATCH] rcu/tree: Fix self wakeups for grace period kthread Neeraj Upadhyay
@ 2019-03-12 1:50 ` Steven Rostedt
2019-03-12 11:55 ` Neeraj Upadhyay
0 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2019-03-12 1:50 UTC (permalink / raw)
To: Neeraj Upadhyay
Cc: paulmck, josh, mathieu.desnoyers, jiangshanlai, linux-kernel
On Fri, 8 Mar 2019 15:16:18 +0530
Neeraj Upadhyay <neeraju@codeaurora.org> wrote:
> Update the code to match the comment that self wakeup of
> grace period kthread is allowed from interrupt handler, and
> softirq handler, running in the grace period kthread's
> context. Present code allows self wakeups from all
> interrupt contexts - nmi, softirq and hardirq contexts.
That's not actually the issue. But it appears that we return if we
simply have BH disabled, which I don't think we want, and we don't care
about NMI as NMI should never call this code.
I think your patch is correct, but the change log is not.
-- Steve
>
> Signed-off-by: Neeraj Upadhyay <neeraju@codeaurora.org>
> ---
> kernel/rcu/tree.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index acd6ccf..57cac6d 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -1585,7 +1585,7 @@ static bool rcu_future_gp_cleanup(struct rcu_node *rnp)
> static void rcu_gp_kthread_wake(void)
> {
> if ((current == rcu_state.gp_kthread &&
> - !in_interrupt() && !in_serving_softirq()) ||
> + !in_irq() && !in_serving_softirq()) ||
> !READ_ONCE(rcu_state.gp_flags) ||
> !rcu_state.gp_kthread)
> return;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rcu/tree: Fix self wakeups for grace period kthread
2019-03-12 1:50 ` Steven Rostedt
@ 2019-03-12 11:55 ` Neeraj Upadhyay
2019-03-12 14:09 ` Paul E. McKenney
0 siblings, 1 reply; 6+ messages in thread
From: Neeraj Upadhyay @ 2019-03-12 11:55 UTC (permalink / raw)
To: Steven Rostedt
Cc: paulmck, josh, mathieu.desnoyers, jiangshanlai, linux-kernel
On 3/12/19 7:20 AM, Steven Rostedt wrote:
> On Fri, 8 Mar 2019 15:16:18 +0530
> Neeraj Upadhyay <neeraju@codeaurora.org> wrote:
>
>> Update the code to match the comment that self wakeup of
>> grace period kthread is allowed from interrupt handler, and
>> softirq handler, running in the grace period kthread's
>> context. Present code allows self wakeups from all
>> interrupt contexts - nmi, softirq and hardirq contexts.
>
> That's not actually the issue. But it appears that we return if we
> simply have BH disabled, which I don't think we want, and we don't care
> about NMI as NMI should never call this code.
>
> I think your patch is correct, but the change log is not.
>
> -- Steve
>
Hi Steve, sorry, I don't understand fully, why we want to not return in
BH disabled case. From the commit logs and lkml discussion, there is a
case where GP kthread is interrupted in the wait event path and
rcu_gp_kthread_wake() is called in softirq handler (I am not sure about
interrupt handler case; how rcu_gp_kthread_wake() is called from that path).
https://github.com/torvalds/linux/commit/1d1f898df6586c5ea9aeaf349f13089c6fa37903
Thanks
Neeraj
>
>>
>> Signed-off-by: Neeraj Upadhyay <neeraju@codeaurora.org>
>> ---
>> kernel/rcu/tree.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
>> index acd6ccf..57cac6d 100644
>> --- a/kernel/rcu/tree.c
>> +++ b/kernel/rcu/tree.c
>> @@ -1585,7 +1585,7 @@ static bool rcu_future_gp_cleanup(struct rcu_node *rnp)
>> static void rcu_gp_kthread_wake(void)
>> {
>> if ((current == rcu_state.gp_kthread &&
>> - !in_interrupt() && !in_serving_softirq()) ||
>> + !in_irq() && !in_serving_softirq()) ||
>> !READ_ONCE(rcu_state.gp_flags) ||
>> !rcu_state.gp_kthread)
>> return;
>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of the Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rcu/tree: Fix self wakeups for grace period kthread
2019-03-12 11:55 ` Neeraj Upadhyay
@ 2019-03-12 14:09 ` Paul E. McKenney
2019-03-12 14:56 ` Steven Rostedt
0 siblings, 1 reply; 6+ messages in thread
From: Paul E. McKenney @ 2019-03-12 14:09 UTC (permalink / raw)
To: Neeraj Upadhyay
Cc: Steven Rostedt, josh, mathieu.desnoyers, jiangshanlai, linux-kernel
On Tue, Mar 12, 2019 at 05:25:28PM +0530, Neeraj Upadhyay wrote:
> On 3/12/19 7:20 AM, Steven Rostedt wrote:
> >On Fri, 8 Mar 2019 15:16:18 +0530
> >Neeraj Upadhyay <neeraju@codeaurora.org> wrote:
> >
> >>Update the code to match the comment that self wakeup of
> >>grace period kthread is allowed from interrupt handler, and
> >>softirq handler, running in the grace period kthread's
> >>context. Present code allows self wakeups from all
> >>interrupt contexts - nmi, softirq and hardirq contexts.
> >
> >That's not actually the issue. But it appears that we return if we
> >simply have BH disabled, which I don't think we want, and we don't care
> >about NMI as NMI should never call this code.
> >
> >I think your patch is correct, but the change log is not.
How about this?
The current rcu_gp_kthread_wake() function uses in_interrupt()
and thus does a self-wakeup from all interrupt contexts,
including the pointless case where the GP kthread happens to be
running with bottom halves disabled, along with the impossible
case where the GP kthread is running within an NMI handler (you
are not supposed to invoke rcu_gp_kthread_wake() from within an
NMI handler. This commit therefore replaces the in_interrupt()
with in_irq(), so that the self-wakeups happen only from handlers
for hardware interrupts and softirqs. This also makes the code
match the comment.
Thanx, Paul
> >-- Steve
> >
>
> Hi Steve, sorry, I don't understand fully, why we want to not return
> in BH disabled case. From the commit logs and lkml discussion, there
> is a case where GP kthread is interrupted in the wait event path and
> rcu_gp_kthread_wake() is called in softirq handler (I am not sure
> about interrupt handler case; how rcu_gp_kthread_wake() is called
> from that path).
>
> https://github.com/torvalds/linux/commit/1d1f898df6586c5ea9aeaf349f13089c6fa37903
>
> Thanks
> Neeraj
> >
> >>
> >>Signed-off-by: Neeraj Upadhyay <neeraju@codeaurora.org>
> >>---
> >> kernel/rcu/tree.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >>diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> >>index acd6ccf..57cac6d 100644
> >>--- a/kernel/rcu/tree.c
> >>+++ b/kernel/rcu/tree.c
> >>@@ -1585,7 +1585,7 @@ static bool rcu_future_gp_cleanup(struct rcu_node *rnp)
> >> static void rcu_gp_kthread_wake(void)
> >> {
> >> if ((current == rcu_state.gp_kthread &&
> >>- !in_interrupt() && !in_serving_softirq()) ||
> >>+ !in_irq() && !in_serving_softirq()) ||
> >> !READ_ONCE(rcu_state.gp_flags) ||
> >> !rcu_state.gp_kthread)
> >> return;
> >
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
> member of the Code Aurora Forum, hosted by The Linux Foundation
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rcu/tree: Fix self wakeups for grace period kthread
2019-03-12 14:09 ` Paul E. McKenney
@ 2019-03-12 14:56 ` Steven Rostedt
2019-03-12 15:36 ` Paul E. McKenney
0 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2019-03-12 14:56 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Neeraj Upadhyay, josh, mathieu.desnoyers, jiangshanlai, linux-kernel
On Tue, 12 Mar 2019 07:09:23 -0700
"Paul E. McKenney" <paulmck@linux.ibm.com> wrote:
> On Tue, Mar 12, 2019 at 05:25:28PM +0530, Neeraj Upadhyay wrote:
> > On 3/12/19 7:20 AM, Steven Rostedt wrote:
> > >On Fri, 8 Mar 2019 15:16:18 +0530
> > >Neeraj Upadhyay <neeraju@codeaurora.org> wrote:
> > >
> > >>Update the code to match the comment that self wakeup of
> > >>grace period kthread is allowed from interrupt handler, and
> > >>softirq handler, running in the grace period kthread's
> > >>context. Present code allows self wakeups from all
> > >>interrupt contexts - nmi, softirq and hardirq contexts.
> > >
> > >That's not actually the issue. But it appears that we return if we
> > >simply have BH disabled, which I don't think we want, and we don't care
> > >about NMI as NMI should never call this code.
> > >
> > >I think your patch is correct, but the change log is not.
>
> How about this?
>
> The current rcu_gp_kthread_wake() function uses in_interrupt()
> and thus does a self-wakeup from all interrupt contexts,
> including the pointless case where the GP kthread happens to be
> running with bottom halves disabled, along with the impossible
> case where the GP kthread is running within an NMI handler (you
> are not supposed to invoke rcu_gp_kthread_wake() from within an
> NMI handler. This commit therefore replaces the in_interrupt()
> with in_irq(), so that the self-wakeups happen only from handlers
> for hardware interrupts and softirqs. This also makes the code
> match the comment.
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
>
> Thanx, Paul
>
> > >-- Steve
> > >
> >
> > Hi Steve, sorry, I don't understand fully, why we want to not return
> > in BH disabled case. From the commit logs and lkml discussion, there
> > is a case where GP kthread is interrupted in the wait event path and
> > rcu_gp_kthread_wake() is called in softirq handler (I am not sure
> > about interrupt handler case; how rcu_gp_kthread_wake() is called
> > from that path).
BH disabled case isn't a case where the kthread is preempted. It's just
that the kthread disabled BH, and thus we want to return.
-- Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rcu/tree: Fix self wakeups for grace period kthread
2019-03-12 14:56 ` Steven Rostedt
@ 2019-03-12 15:36 ` Paul E. McKenney
0 siblings, 0 replies; 6+ messages in thread
From: Paul E. McKenney @ 2019-03-12 15:36 UTC (permalink / raw)
To: Steven Rostedt
Cc: Neeraj Upadhyay, josh, mathieu.desnoyers, jiangshanlai, linux-kernel
On Tue, Mar 12, 2019 at 10:56:08AM -0400, Steven Rostedt wrote:
> On Tue, 12 Mar 2019 07:09:23 -0700
> "Paul E. McKenney" <paulmck@linux.ibm.com> wrote:
>
> > On Tue, Mar 12, 2019 at 05:25:28PM +0530, Neeraj Upadhyay wrote:
> > > On 3/12/19 7:20 AM, Steven Rostedt wrote:
> > > >On Fri, 8 Mar 2019 15:16:18 +0530
> > > >Neeraj Upadhyay <neeraju@codeaurora.org> wrote:
> > > >
> > > >>Update the code to match the comment that self wakeup of
> > > >>grace period kthread is allowed from interrupt handler, and
> > > >>softirq handler, running in the grace period kthread's
> > > >>context. Present code allows self wakeups from all
> > > >>interrupt contexts - nmi, softirq and hardirq contexts.
> > > >
> > > >That's not actually the issue. But it appears that we return if we
> > > >simply have BH disabled, which I don't think we want, and we don't care
> > > >about NMI as NMI should never call this code.
> > > >
> > > >I think your patch is correct, but the change log is not.
> >
> > How about this?
> >
> > The current rcu_gp_kthread_wake() function uses in_interrupt()
> > and thus does a self-wakeup from all interrupt contexts,
> > including the pointless case where the GP kthread happens to be
> > running with bottom halves disabled, along with the impossible
> > case where the GP kthread is running within an NMI handler (you
> > are not supposed to invoke rcu_gp_kthread_wake() from within an
> > NMI handler. This commit therefore replaces the in_interrupt()
> > with in_irq(), so that the self-wakeups happen only from handlers
> > for hardware interrupts and softirqs. This also makes the code
> > match the comment.
>
> Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Applied, thank you!
Thanx, Paul
> > > >-- Steve
> > > >
> > >
> > > Hi Steve, sorry, I don't understand fully, why we want to not return
> > > in BH disabled case. From the commit logs and lkml discussion, there
> > > is a case where GP kthread is interrupted in the wait event path and
> > > rcu_gp_kthread_wake() is called in softirq handler (I am not sure
> > > about interrupt handler case; how rcu_gp_kthread_wake() is called
> > > from that path).
>
> BH disabled case isn't a case where the kthread is preempted. It's just
> that the kthread disabled BH, and thus we want to return.
>
> -- Steve
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-03-12 15:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-08 9:46 [PATCH] rcu/tree: Fix self wakeups for grace period kthread Neeraj Upadhyay
2019-03-12 1:50 ` Steven Rostedt
2019-03-12 11:55 ` Neeraj Upadhyay
2019-03-12 14:09 ` Paul E. McKenney
2019-03-12 14:56 ` Steven Rostedt
2019-03-12 15:36 ` Paul E. McKenney
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.