v2 - split into two patches: the first inlines the cond_resched_rcu_qs
macro suitable for stable backport, the second converts to using
cond_resched_rcu_qs.
Joe Lawrence (2):
workqueue: add quiescent state between work items
workqueue: use cond_resched_rcu_qs macro
kernel/workqueue.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
--
1.7.10.4
Similar to the stop_machine deadlock scenario on !PREEMPT kernels
addressed in b22ce2785d97 "workqueue: cond_resched() after processing
each work item", kworker threads requeueing back-to-back with zero jiffy
delay can stall RCU. The cond_resched call introduced in that fix will
yield only iff there are other higher priority tasks to run, so force a
quiescent RCU state between work items.
Signed-off-by: Joe Lawrence <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
Link: https://lkml.kernel.org/r/[email protected]
Fixes: b22ce2785d97 ("workqueue: cond_resched() after processing each work item")
Cc: <[email protected]>
---
kernel/workqueue.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 5dbe22a..345bec9 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -2043,8 +2043,10 @@ __acquires(&pool->lock)
* kernels, where a requeueing work item waiting for something to
* happen could deadlock with stop_machine as such work item could
* indefinitely requeue itself while all other CPUs are trapped in
- * stop_machine.
+ * stop_machine. At the same time, report a quiescent RCU state so
+ * the same condition doesn't freeze RCU.
*/
+ rcu_note_voluntary_context_switch(current);
cond_resched();
spin_lock_irq(&pool->lock);
--
1.7.10.4
Tidy up and use cond_resched_rcu_qs when calling cond_resched and
reporting potential quiescent state to RCU.
Signed-off-by: Joe Lawrence <[email protected]>
---
kernel/workqueue.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 345bec9..09b685d 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -2046,8 +2046,7 @@ __acquires(&pool->lock)
* stop_machine. At the same time, report a quiescent RCU state so
* the same condition doesn't freeze RCU.
*/
- rcu_note_voluntary_context_switch(current);
- cond_resched();
+ cond_resched_rcu_qs();
spin_lock_irq(&pool->lock);
--
1.7.10.4
On Sun, Oct 05, 2014 at 01:24:22PM -0400, Joe Lawrence wrote:
> Tidy up and use cond_resched_rcu_qs when calling cond_resched and
> reporting potential quiescent state to RCU.
>
> Signed-off-by: Joe Lawrence <[email protected]>
Reviewed-by: Paul E. McKenney <[email protected]>
> ---
> kernel/workqueue.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 345bec9..09b685d 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -2046,8 +2046,7 @@ __acquires(&pool->lock)
> * stop_machine. At the same time, report a quiescent RCU state so
> * the same condition doesn't freeze RCU.
> */
> - rcu_note_voluntary_context_switch(current);
> - cond_resched();
> + cond_resched_rcu_qs();
>
> spin_lock_irq(&pool->lock);
>
> --
> 1.7.10.4
>
On Sun, Oct 05, 2014 at 01:24:21PM -0400, Joe Lawrence wrote:
> Similar to the stop_machine deadlock scenario on !PREEMPT kernels
> addressed in b22ce2785d97 "workqueue: cond_resched() after processing
> each work item", kworker threads requeueing back-to-back with zero jiffy
> delay can stall RCU. The cond_resched call introduced in that fix will
> yield only iff there are other higher priority tasks to run, so force a
> quiescent RCU state between work items.
>
> Signed-off-by: Joe Lawrence <[email protected]>
> Link: https://lkml.kernel.org/r/[email protected]
> Link: https://lkml.kernel.org/r/[email protected]
> Fixes: b22ce2785d97 ("workqueue: cond_resched() after processing each work item")
> Cc: <[email protected]>
Applied to wq/for-3.17-fixes. If 3.17 comes out before this gets
merged, I'll send it as for-3.18.
Thanks.
--
tejun
On Sun, Oct 05, 2014 at 03:21:19PM -0400, Tejun Heo wrote:
> On Sun, Oct 05, 2014 at 01:24:21PM -0400, Joe Lawrence wrote:
> > Similar to the stop_machine deadlock scenario on !PREEMPT kernels
> > addressed in b22ce2785d97 "workqueue: cond_resched() after processing
> > each work item", kworker threads requeueing back-to-back with zero jiffy
> > delay can stall RCU. The cond_resched call introduced in that fix will
> > yield only iff there are other higher priority tasks to run, so force a
> > quiescent RCU state between work items.
> >
> > Signed-off-by: Joe Lawrence <[email protected]>
> > Link: https://lkml.kernel.org/r/[email protected]
> > Link: https://lkml.kernel.org/r/[email protected]
> > Fixes: b22ce2785d97 ("workqueue: cond_resched() after processing each work item")
> > Cc: <[email protected]>
>
> Applied to wq/for-3.17-fixes. If 3.17 comes out before this gets
> merged, I'll send it as for-3.18.
Oops, the rcu calls aren't in mainline yet. I think it'd be best to
route these through the RCU tree. Paul, can you please route these
two patches?
Acked-by: Tejun Heo <[email protected]>
Thanks.
--
tejun
On Sun, Oct 05, 2014 at 01:24:22PM -0400, Joe Lawrence wrote:
> Tidy up and use cond_resched_rcu_qs when calling cond_resched and
> reporting potential quiescent state to RCU.
>
> Signed-off-by: Joe Lawrence <[email protected]>
Please route through the rcu tree.
Acked-by: Tejun Heo <[email protected]>
Thanks!
--
tejun
On Sun, Oct 05, 2014 at 03:47:48PM -0400, Tejun Heo wrote:
> On Sun, Oct 05, 2014 at 03:21:19PM -0400, Tejun Heo wrote:
> > On Sun, Oct 05, 2014 at 01:24:21PM -0400, Joe Lawrence wrote:
> > > Similar to the stop_machine deadlock scenario on !PREEMPT kernels
> > > addressed in b22ce2785d97 "workqueue: cond_resched() after processing
> > > each work item", kworker threads requeueing back-to-back with zero jiffy
> > > delay can stall RCU. The cond_resched call introduced in that fix will
> > > yield only iff there are other higher priority tasks to run, so force a
> > > quiescent RCU state between work items.
> > >
> > > Signed-off-by: Joe Lawrence <[email protected]>
> > > Link: https://lkml.kernel.org/r/[email protected]
> > > Link: https://lkml.kernel.org/r/[email protected]
> > > Fixes: b22ce2785d97 ("workqueue: cond_resched() after processing each work item")
> > > Cc: <[email protected]>
> >
> > Applied to wq/for-3.17-fixes. If 3.17 comes out before this gets
> > merged, I'll send it as for-3.18.
>
> Oops, the rcu calls aren't in mainline yet. I think it'd be best to
> route these through the RCU tree. Paul, can you please route these
> two patches?
>
> Acked-by: Tejun Heo <[email protected]>
Will do!
I will try 3.17, failing that, 3.18.
Thanx, Paul
Mon, Oct 06, 2014 at 06:21:58AM CEST, [email protected] wrote:
>On Sun, Oct 05, 2014 at 03:47:48PM -0400, Tejun Heo wrote:
>> On Sun, Oct 05, 2014 at 03:21:19PM -0400, Tejun Heo wrote:
>> > On Sun, Oct 05, 2014 at 01:24:21PM -0400, Joe Lawrence wrote:
>> > > Similar to the stop_machine deadlock scenario on !PREEMPT kernels
>> > > addressed in b22ce2785d97 "workqueue: cond_resched() after processing
>> > > each work item", kworker threads requeueing back-to-back with zero jiffy
>> > > delay can stall RCU. The cond_resched call introduced in that fix will
>> > > yield only iff there are other higher priority tasks to run, so force a
>> > > quiescent RCU state between work items.
>> > >
>> > > Signed-off-by: Joe Lawrence <[email protected]>
>> > > Link: https://lkml.kernel.org/r/[email protected]
>> > > Link: https://lkml.kernel.org/r/[email protected]
>> > > Fixes: b22ce2785d97 ("workqueue: cond_resched() after processing each work item")
>> > > Cc: <[email protected]>
>> >
>> > Applied to wq/for-3.17-fixes. If 3.17 comes out before this gets
>> > merged, I'll send it as for-3.18.
>>
>> Oops, the rcu calls aren't in mainline yet. I think it'd be best to
>> route these through the RCU tree. Paul, can you please route these
>> two patches?
>>
>> Acked-by: Tejun Heo <[email protected]>
>
>Will do!
>
>I will try 3.17, failing that, 3.18.
Paul, Tehun, how do you propose to fix this on older kernels which do
not have rcu_note_voluntary_context_switch? I'm particullary interested
in 3.10.
Thanks.
On Tue, Oct 07, 2014 at 09:29:42AM +0200, Jiri Pirko wrote:
> Mon, Oct 06, 2014 at 06:21:58AM CEST, [email protected] wrote:
> >On Sun, Oct 05, 2014 at 03:47:48PM -0400, Tejun Heo wrote:
> >> On Sun, Oct 05, 2014 at 03:21:19PM -0400, Tejun Heo wrote:
> >> > On Sun, Oct 05, 2014 at 01:24:21PM -0400, Joe Lawrence wrote:
> >> > > Similar to the stop_machine deadlock scenario on !PREEMPT kernels
> >> > > addressed in b22ce2785d97 "workqueue: cond_resched() after processing
> >> > > each work item", kworker threads requeueing back-to-back with zero jiffy
> >> > > delay can stall RCU. The cond_resched call introduced in that fix will
> >> > > yield only iff there are other higher priority tasks to run, so force a
> >> > > quiescent RCU state between work items.
> >> > >
> >> > > Signed-off-by: Joe Lawrence <[email protected]>
> >> > > Link: https://lkml.kernel.org/r/[email protected]
> >> > > Link: https://lkml.kernel.org/r/[email protected]
> >> > > Fixes: b22ce2785d97 ("workqueue: cond_resched() after processing each work item")
> >> > > Cc: <[email protected]>
> >> >
> >> > Applied to wq/for-3.17-fixes. If 3.17 comes out before this gets
> >> > merged, I'll send it as for-3.18.
> >>
> >> Oops, the rcu calls aren't in mainline yet. I think it'd be best to
> >> route these through the RCU tree. Paul, can you please route these
> >> two patches?
> >>
> >> Acked-by: Tejun Heo <[email protected]>
> >
> >Will do!
> >
> >I will try 3.17, failing that, 3.18.
>
>
> Paul, Tehun, how do you propose to fix this on older kernels which do
> not have rcu_note_voluntary_context_switch? I'm particullary interested
> in 3.10.
Hello, Jiri,
Older kernels can instead use rcu_note_context_switch().
Thanx, Paul
On Tue, 7 Oct 2014 06:43:29 -0700
"Paul E. McKenney" <[email protected]> wrote:
> On Tue, Oct 07, 2014 at 09:29:42AM +0200, Jiri Pirko wrote:
[ ... snip ... ]
> >
> > Paul, Tehun, how do you propose to fix this on older kernels which do
> > not have rcu_note_voluntary_context_switch? I'm particullary interested
> > in 3.10.
>
> Hello, Jiri,
>
> Older kernels can instead use rcu_note_context_switch().
Hi Paul,
Does 4a81e8328d37 ("rcu: Reduce overhead of cond_resched() checks for
RCU") affect a backport to 3.10?
I noticed that rcu_note_context_switch added a call to
rcu_momentary_dyntick_idle in that change, which is only present in
v3.16+.
Would rcu_note_context_switch be effective by itself on a 3.10 kernel?
Thanks,
-- Joe
On Tue, Oct 07, 2014 at 01:45:28PM -0400, Joe Lawrence wrote:
> On Tue, 7 Oct 2014 06:43:29 -0700
> "Paul E. McKenney" <[email protected]> wrote:
>
> > On Tue, Oct 07, 2014 at 09:29:42AM +0200, Jiri Pirko wrote:
> [ ... snip ... ]
> > >
> > > Paul, Tehun, how do you propose to fix this on older kernels which do
> > > not have rcu_note_voluntary_context_switch? I'm particullary interested
> > > in 3.10.
> >
> > Hello, Jiri,
> >
> > Older kernels can instead use rcu_note_context_switch().
>
> Hi Paul,
>
> Does 4a81e8328d37 ("rcu: Reduce overhead of cond_resched() checks for
> RCU") affect a backport to 3.10?
>
> I noticed that rcu_note_context_switch added a call to
> rcu_momentary_dyntick_idle in that change, which is only present in
> v3.16+.
>
> Would rcu_note_context_switch be effective by itself on a 3.10 kernel?
Should be fine. There is more overhead than current mainline, but that
should not be in the noise compared to executing a work-queue item.
Thanx, Paul
Wed, Oct 08, 2014 at 05:24:11AM CEST, [email protected] wrote:
>On Tue, Oct 07, 2014 at 01:45:28PM -0400, Joe Lawrence wrote:
>> On Tue, 7 Oct 2014 06:43:29 -0700
>> "Paul E. McKenney" <[email protected]> wrote:
>>
>> > On Tue, Oct 07, 2014 at 09:29:42AM +0200, Jiri Pirko wrote:
>> [ ... snip ... ]
>> > >
>> > > Paul, Tehun, how do you propose to fix this on older kernels which do
>> > > not have rcu_note_voluntary_context_switch? I'm particullary interested
>> > > in 3.10.
>> >
>> > Hello, Jiri,
>> >
>> > Older kernels can instead use rcu_note_context_switch().
>>
>> Hi Paul,
>>
>> Does 4a81e8328d37 ("rcu: Reduce overhead of cond_resched() checks for
>> RCU") affect a backport to 3.10?
>>
>> I noticed that rcu_note_context_switch added a call to
>> rcu_momentary_dyntick_idle in that change, which is only present in
>> v3.16+.
>>
>> Would rcu_note_context_switch be effective by itself on a 3.10 kernel?
>
>Should be fine. There is more overhead than current mainline, but that
>should not be in the noise compared to executing a work-queue item.
>
> Thanx, Paul
>
I cooked up following patch. Please tell me if it is fine or not. I can
also send it oficially so it can be included into stable trees:
Subject: workqueue: Add quiescent state between work items
Similar to the stop_machine deadlock scenario on !PREEMPT kernels
addressed in b22ce2785d97 "workqueue: cond_resched() after processing
each work item", kworker threads requeueing back-to-back with zero jiffy
delay can stall RCU. The cond_resched call introduced in that fix will
yield only iff there are other higher priority tasks to run, so force a
quiescent RCU state between work items.
Signed-off-by: Jiri Pirko <[email protected]>
---
kernel/workqueue.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index e9719c7..14a7163 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -2196,8 +2196,10 @@ __acquires(&pool->lock)
* kernels, where a requeueing work item waiting for something to
* happen could deadlock with stop_machine as such work item could
* indefinitely requeue itself while all other CPUs are trapped in
- * stop_machine.
+ * stop_machine. At the same time, report a quiescent RCU state so
+ * the same condition doesn't freeze RCU.
*/
+ rcu_note_context_switch(raw_smp_processor_id());
cond_resched();
spin_lock_irq(&pool->lock);
--
1.9.3
On Wed, Oct 08, 2014 at 01:54:28PM +0200, Jiri Pirko wrote:
> Wed, Oct 08, 2014 at 05:24:11AM CEST, [email protected] wrote:
> >On Tue, Oct 07, 2014 at 01:45:28PM -0400, Joe Lawrence wrote:
> >> On Tue, 7 Oct 2014 06:43:29 -0700
> >> "Paul E. McKenney" <[email protected]> wrote:
> >>
> >> > On Tue, Oct 07, 2014 at 09:29:42AM +0200, Jiri Pirko wrote:
> >> [ ... snip ... ]
> >> > >
> >> > > Paul, Tehun, how do you propose to fix this on older kernels which do
> >> > > not have rcu_note_voluntary_context_switch? I'm particullary interested
> >> > > in 3.10.
> >> >
> >> > Hello, Jiri,
> >> >
> >> > Older kernels can instead use rcu_note_context_switch().
> >>
> >> Hi Paul,
> >>
> >> Does 4a81e8328d37 ("rcu: Reduce overhead of cond_resched() checks for
> >> RCU") affect a backport to 3.10?
> >>
> >> I noticed that rcu_note_context_switch added a call to
> >> rcu_momentary_dyntick_idle in that change, which is only present in
> >> v3.16+.
> >>
> >> Would rcu_note_context_switch be effective by itself on a 3.10 kernel?
> >
> >Should be fine. There is more overhead than current mainline, but that
> >should not be in the noise compared to executing a work-queue item.
> >
> > Thanx, Paul
> >
>
> I cooked up following patch. Please tell me if it is fine or not. I can
> also send it oficially so it can be included into stable trees:
Looks good!
Thanx, Paul
> Subject: workqueue: Add quiescent state between work items
>
> Similar to the stop_machine deadlock scenario on !PREEMPT kernels
> addressed in b22ce2785d97 "workqueue: cond_resched() after processing
> each work item", kworker threads requeueing back-to-back with zero jiffy
> delay can stall RCU. The cond_resched call introduced in that fix will
> yield only iff there are other higher priority tasks to run, so force a
> quiescent RCU state between work items.
>
> Signed-off-by: Jiri Pirko <[email protected]>
> ---
> kernel/workqueue.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index e9719c7..14a7163 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -2196,8 +2196,10 @@ __acquires(&pool->lock)
> * kernels, where a requeueing work item waiting for something to
> * happen could deadlock with stop_machine as such work item could
> * indefinitely requeue itself while all other CPUs are trapped in
> - * stop_machine.
> + * stop_machine. At the same time, report a quiescent RCU state so
> + * the same condition doesn't freeze RCU.
> */
> + rcu_note_context_switch(raw_smp_processor_id());
> cond_resched();
>
> spin_lock_irq(&pool->lock);
> --
> 1.9.3
>
>