Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1558892 > unrolled thread
| Started by | "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> |
|---|---|
| First post | 2017-01-14 10:20 +0100 |
| Last post | 2017-01-18 08:20 +0100 |
| Articles | 9 — 4 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH tip/core/rcu 11/20] sched,rcu: Make cond_resched() provide RCU quiescent state "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-01-14 10:20 +0100
Re: [PATCH tip/core/rcu 11/20] sched,rcu: Make cond_resched() provide RCU quiescent state Josh Triplett <josh@joshtriplett.org> - 2017-01-16 09:10 +0100
Re: [PATCH tip/core/rcu 11/20] sched,rcu: Make cond_resched() provide RCU quiescent state "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-01-16 12:40 +0100
Re: [PATCH tip/core/rcu 11/20] sched,rcu: Make cond_resched() provide RCU quiescent state Peter Zijlstra <peterz@infradead.org> - 2017-01-16 18:20 +0100
Re: [PATCH tip/core/rcu 11/20] sched,rcu: Make cond_resched() provide RCU quiescent state "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-01-17 05:00 +0100
Re: [PATCH tip/core/rcu 11/20] sched,rcu: Make cond_resched() provide RCU quiescent state Michal Hocko <mhocko@kernel.org> - 2017-01-17 12:10 +0100
Re: [PATCH tip/core/rcu 11/20] sched,rcu: Make cond_resched() provide RCU quiescent state "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-01-17 13:10 +0100
Re: [PATCH tip/core/rcu 11/20] sched,rcu: Make cond_resched() provide RCU quiescent state Michal Hocko <mhocko@kernel.org> - 2017-01-17 13:20 +0100
Re: [PATCH tip/core/rcu 11/20] sched,rcu: Make cond_resched() provide RCU quiescent state "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-01-18 08:20 +0100
| From | "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> |
|---|---|
| Date | 2017-01-14 10:20 +0100 |
| Subject | [PATCH tip/core/rcu 11/20] sched,rcu: Make cond_resched() provide RCU quiescent state |
| Message-ID | <sZsFc-49R-17@gated-at.bofh.it> |
There is some confusion as to which of cond_resched() or
cond_resched_rcu_qs() should be added to long in-kernel loops.
This commit therefore eliminates the decision by adding RCU
quiescent states to cond_resched().
Warning: This is a prototype. For example, it does not correctly
handle Tasks RCU. Which is OK for the moment, given that no one
actually uses Tasks RCU yet.
Reported-by: Michal Hocko <mhocko@kernel.org>
Not-yet-signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
---
include/linux/sched.h | 3 ++-
kernel/sched/core.c | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 4d1905245c7a..1531c48f56e2 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -3352,10 +3352,11 @@ static inline int signal_pending_state(long state, struct task_struct *p)
* cond_resched_lock() will drop the spinlock before scheduling,
* cond_resched_softirq() will enable bhs before scheduling.
*/
+void rcu_all_qs(void);
#ifndef CONFIG_PREEMPT
extern int _cond_resched(void);
#else
-static inline int _cond_resched(void) { return 0; }
+static inline int _cond_resched(void) { rcu_all_qs(); return 0; }
#endif
#define cond_resched() ({ \
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index c56fb57f2991..b442f8918eb9 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4907,6 +4907,7 @@ int __sched _cond_resched(void)
preempt_schedule_common();
return 1;
}
+ rcu_all_qs();
return 0;
}
EXPORT_SYMBOL(_cond_resched);
--
2.5.2
[toc] | [next] | [standalone]
| From | Josh Triplett <josh@joshtriplett.org> |
|---|---|
| Date | 2017-01-16 09:10 +0100 |
| Subject | Re: [PATCH tip/core/rcu 11/20] sched,rcu: Make cond_resched() provide RCU quiescent state |
| Message-ID | <t0awx-5S5-11@gated-at.bofh.it> |
| In reply to | #1558892 |
On Sat, Jan 14, 2017 at 01:13:12AM -0800, Paul E. McKenney wrote:
> There is some confusion as to which of cond_resched() or
> cond_resched_rcu_qs() should be added to long in-kernel loops.
> This commit therefore eliminates the decision by adding RCU
> quiescent states to cond_resched().
>
> Warning: This is a prototype. For example, it does not correctly
> handle Tasks RCU. Which is OK for the moment, given that no one
> actually uses Tasks RCU yet.
>
> Reported-by: Michal Hocko <mhocko@kernel.org>
> Not-yet-signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
Do you intend to merge this patch, or have you just posted it for
review?
You may want to remove it from this 20-patch series and post it as a
separate RFC PATCH, to ensure that it doesn't get merged while still a
prototype.
> include/linux/sched.h | 3 ++-
> kernel/sched/core.c | 1 +
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 4d1905245c7a..1531c48f56e2 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -3352,10 +3352,11 @@ static inline int signal_pending_state(long state, struct task_struct *p)
> * cond_resched_lock() will drop the spinlock before scheduling,
> * cond_resched_softirq() will enable bhs before scheduling.
> */
> +void rcu_all_qs(void);
> #ifndef CONFIG_PREEMPT
> extern int _cond_resched(void);
> #else
> -static inline int _cond_resched(void) { return 0; }
> +static inline int _cond_resched(void) { rcu_all_qs(); return 0; }
> #endif
>
> #define cond_resched() ({ \
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index c56fb57f2991..b442f8918eb9 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -4907,6 +4907,7 @@ int __sched _cond_resched(void)
> preempt_schedule_common();
> return 1;
> }
> + rcu_all_qs();
> return 0;
> }
> EXPORT_SYMBOL(_cond_resched);
> --
> 2.5.2
>
[toc] | [prev] | [next] | [standalone]
| From | "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> |
|---|---|
| Date | 2017-01-16 12:40 +0100 |
| Subject | Re: [PATCH tip/core/rcu 11/20] sched,rcu: Make cond_resched() provide RCU quiescent state |
| Message-ID | <t0dNL-8cl-15@gated-at.bofh.it> |
| In reply to | #1559507 |
On Mon, Jan 16, 2017 at 12:08:43AM -0800, Josh Triplett wrote:
> On Sat, Jan 14, 2017 at 01:13:12AM -0800, Paul E. McKenney wrote:
> > There is some confusion as to which of cond_resched() or
> > cond_resched_rcu_qs() should be added to long in-kernel loops.
> > This commit therefore eliminates the decision by adding RCU
> > quiescent states to cond_resched().
> >
> > Warning: This is a prototype. For example, it does not correctly
> > handle Tasks RCU. Which is OK for the moment, given that no one
> > actually uses Tasks RCU yet.
> >
> > Reported-by: Michal Hocko <mhocko@kernel.org>
> > Not-yet-signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
>
> Do you intend to merge this patch, or have you just posted it for
> review?
>
> You may want to remove it from this 20-patch series and post it as a
> separate RFC PATCH, to ensure that it doesn't get merged while still a
> prototype.
It does look like I should delay this one until I get my story straight.
Good catch!
Thanx, Paul
> > include/linux/sched.h | 3 ++-
> > kernel/sched/core.c | 1 +
> > 2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > index 4d1905245c7a..1531c48f56e2 100644
> > --- a/include/linux/sched.h
> > +++ b/include/linux/sched.h
> > @@ -3352,10 +3352,11 @@ static inline int signal_pending_state(long state, struct task_struct *p)
> > * cond_resched_lock() will drop the spinlock before scheduling,
> > * cond_resched_softirq() will enable bhs before scheduling.
> > */
> > +void rcu_all_qs(void);
> > #ifndef CONFIG_PREEMPT
> > extern int _cond_resched(void);
> > #else
> > -static inline int _cond_resched(void) { return 0; }
> > +static inline int _cond_resched(void) { rcu_all_qs(); return 0; }
> > #endif
> >
> > #define cond_resched() ({ \
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index c56fb57f2991..b442f8918eb9 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -4907,6 +4907,7 @@ int __sched _cond_resched(void)
> > preempt_schedule_common();
> > return 1;
> > }
> > + rcu_all_qs();
> > return 0;
> > }
> > EXPORT_SYMBOL(_cond_resched);
> > --
> > 2.5.2
> >
>
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-01-16 18:20 +0100 |
| Subject | Re: [PATCH tip/core/rcu 11/20] sched,rcu: Make cond_resched() provide RCU quiescent state |
| Message-ID | <t0j6O-3rL-19@gated-at.bofh.it> |
| In reply to | #1558892 |
On Sat, Jan 14, 2017 at 01:13:12AM -0800, Paul E. McKenney wrote: > There is some confusion as to which of cond_resched() or > cond_resched_rcu_qs() should be added to long in-kernel loops. > This commit therefore eliminates the decision by adding RCU > quiescent states to cond_resched(). Which would make: rcu_read_lock(); cond_resched(); rcu_read_unlock(); invalid under preemptible RCU. Is it already? > Warning: This is a prototype. For example, it does not correctly > handle Tasks RCU. Which is OK for the moment, given that no one > actually uses Tasks RCU yet. > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -4907,6 +4907,7 @@ int __sched _cond_resched(void) > preempt_schedule_common(); > return 1; > } > + rcu_all_qs(); > return 0; > } Still not a real fan of this, it does make cond_resched() touch a bunch more cachelines, also, I suppose that if we're going to do this, we should make __cond_resched_lock() and __cond_resched_softirq() act similarly.
[toc] | [prev] | [next] | [standalone]
| From | "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> |
|---|---|
| Date | 2017-01-17 05:00 +0100 |
| Subject | Re: [PATCH tip/core/rcu 11/20] sched,rcu: Make cond_resched() provide RCU quiescent state |
| Message-ID | <t0t6a-1IC-11@gated-at.bofh.it> |
| In reply to | #1559929 |
On Mon, Jan 16, 2017 at 06:11:30PM +0100, Peter Zijlstra wrote: > On Sat, Jan 14, 2017 at 01:13:12AM -0800, Paul E. McKenney wrote: > > There is some confusion as to which of cond_resched() or > > cond_resched_rcu_qs() should be added to long in-kernel loops. > > This commit therefore eliminates the decision by adding RCU > > quiescent states to cond_resched(). > > Which would make: rcu_read_lock(); cond_resched(); rcu_read_unlock(); > invalid under preemptible RCU. Is it already? In theory, yes. In practice, I just tested it with preemption and lockdep enabled, and it didn't complain. If further testing finds complaints, we can either fix those uses (preferred) or revert this patch. > > Warning: This is a prototype. For example, it does not correctly > > handle Tasks RCU. Which is OK for the moment, given that no one > > actually uses Tasks RCU yet. > > > --- a/kernel/sched/core.c > > +++ b/kernel/sched/core.c > > @@ -4907,6 +4907,7 @@ int __sched _cond_resched(void) > > preempt_schedule_common(); > > return 1; > > } > > + rcu_all_qs(); > > return 0; > > } > > Still not a real fan of this, it does make cond_resched() touch a bunch > more cachelines, also, I suppose that if we're going to do this, we > should make __cond_resched_lock() and __cond_resched_softirq() act > similarly. Michal (now CCed) argues that having to distinguish between cond_resched() and cond_resched_rcu_qs() is overly burdensome. Michal? Any thoughts on how we might remove this burden without the additional cache misses? I will take another look as well to see what could make it lower cost. There are probably ways... Would it make sense to have RCU maintain a need-rcu_all_qs() flage in the same cacheline as the __preempt_count? Perhaps throttling the writes to this flag from the RCU grace-period kthreads to once per 100 milliseconds or so? Thanx, Paul
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-01-17 12:10 +0100 |
| Subject | Re: [PATCH tip/core/rcu 11/20] sched,rcu: Make cond_resched() provide RCU quiescent state |
| Message-ID | <t0zOh-6m4-5@gated-at.bofh.it> |
| In reply to | #1560243 |
On Mon 16-01-17 16:54:03, Paul E. McKenney wrote: > On Mon, Jan 16, 2017 at 06:11:30PM +0100, Peter Zijlstra wrote: > > On Sat, Jan 14, 2017 at 01:13:12AM -0800, Paul E. McKenney wrote: > > > There is some confusion as to which of cond_resched() or > > > cond_resched_rcu_qs() should be added to long in-kernel loops. > > > This commit therefore eliminates the decision by adding RCU > > > quiescent states to cond_resched(). > > > > Which would make: rcu_read_lock(); cond_resched(); rcu_read_unlock(); > > invalid under preemptible RCU. Is it already? > > In theory, yes. In practice, I just tested it with preemption and > lockdep enabled, and it didn't complain. If further testing finds > complaints, we can either fix those uses (preferred) or revert > this patch. > > > > Warning: This is a prototype. For example, it does not correctly > > > handle Tasks RCU. Which is OK for the moment, given that no one > > > actually uses Tasks RCU yet. > > > > > --- a/kernel/sched/core.c > > > +++ b/kernel/sched/core.c > > > @@ -4907,6 +4907,7 @@ int __sched _cond_resched(void) > > > preempt_schedule_common(); > > > return 1; > > > } > > > + rcu_all_qs(); > > > return 0; > > > } > > > > Still not a real fan of this, it does make cond_resched() touch a bunch > > more cachelines, also, I suppose that if we're going to do this, we > > should make __cond_resched_lock() and __cond_resched_softirq() act > > similarly. > > Michal (now CCed) argues that having to distinguish between cond_resched() > and cond_resched_rcu_qs() is overly burdensome. Michal? Yes, it is really not clear which one is meant to be in which context. I really do not see which cond_resched should be turned intto cond_resched_rcu_qs. > Any thoughts on how we might remove this burden without the additional > cache misses? I will take another look as well to see what could make > it lower cost. There are probably ways... Would it make sense to > have RCU maintain a need-rcu_all_qs() flage in the same cacheline as > the __preempt_count? Perhaps throttling the writes to this flag from > the RCU grace-period kthreads to once per 100 milliseconds or so? Can the stall detector simply request rescheduling when it gets dangerously close to the timeout? -- Michal Hocko SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> |
|---|---|
| Date | 2017-01-17 13:10 +0100 |
| Subject | Re: [PATCH tip/core/rcu 11/20] sched,rcu: Make cond_resched() provide RCU quiescent state |
| Message-ID | <t0AKl-6VT-5@gated-at.bofh.it> |
| In reply to | #1560499 |
On Tue, Jan 17, 2017 at 11:51:41AM +0100, Michal Hocko wrote: > On Mon 16-01-17 16:54:03, Paul E. McKenney wrote: > > On Mon, Jan 16, 2017 at 06:11:30PM +0100, Peter Zijlstra wrote: > > > On Sat, Jan 14, 2017 at 01:13:12AM -0800, Paul E. McKenney wrote: > > > > There is some confusion as to which of cond_resched() or > > > > cond_resched_rcu_qs() should be added to long in-kernel loops. > > > > This commit therefore eliminates the decision by adding RCU > > > > quiescent states to cond_resched(). > > > > > > Which would make: rcu_read_lock(); cond_resched(); rcu_read_unlock(); > > > invalid under preemptible RCU. Is it already? > > > > In theory, yes. In practice, I just tested it with preemption and > > lockdep enabled, and it didn't complain. If further testing finds > > complaints, we can either fix those uses (preferred) or revert > > this patch. > > > > > > Warning: This is a prototype. For example, it does not correctly > > > > handle Tasks RCU. Which is OK for the moment, given that no one > > > > actually uses Tasks RCU yet. > > > > > > > --- a/kernel/sched/core.c > > > > +++ b/kernel/sched/core.c > > > > @@ -4907,6 +4907,7 @@ int __sched _cond_resched(void) > > > > preempt_schedule_common(); > > > > return 1; > > > > } > > > > + rcu_all_qs(); > > > > return 0; > > > > } > > > > > > Still not a real fan of this, it does make cond_resched() touch a bunch > > > more cachelines, also, I suppose that if we're going to do this, we > > > should make __cond_resched_lock() and __cond_resched_softirq() act > > > similarly. > > > > Michal (now CCed) argues that having to distinguish between cond_resched() > > and cond_resched_rcu_qs() is overly burdensome. Michal? > > Yes, it is really not clear which one is meant to be in which context. I > really do not see which cond_resched should be turned intto > cond_resched_rcu_qs. > > > Any thoughts on how we might remove this burden without the additional > > cache misses? I will take another look as well to see what could make > > it lower cost. There are probably ways... Would it make sense to > > have RCU maintain a need-rcu_all_qs() flage in the same cacheline as > > the __preempt_count? Perhaps throttling the writes to this flag from > > the RCU grace-period kthreads to once per 100 milliseconds or so? > > Can the stall detector simply request rescheduling when it gets > dangerously close to the timeout? It is quite possible that half of the stall timeout would be a better choice than my 100 milliseconds, but either way, there would be need for a flag or some such. Thanx, Paul
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-01-17 13:20 +0100 |
| Subject | Re: [PATCH tip/core/rcu 11/20] sched,rcu: Make cond_resched() provide RCU quiescent state |
| Message-ID | <t0AU1-6Zd-11@gated-at.bofh.it> |
| In reply to | #1560530 |
On Tue 17-01-17 04:05:13, Paul E. McKenney wrote: > On Tue, Jan 17, 2017 at 11:51:41AM +0100, Michal Hocko wrote: > > On Mon 16-01-17 16:54:03, Paul E. McKenney wrote: > > > On Mon, Jan 16, 2017 at 06:11:30PM +0100, Peter Zijlstra wrote: > > > > On Sat, Jan 14, 2017 at 01:13:12AM -0800, Paul E. McKenney wrote: > > > > > There is some confusion as to which of cond_resched() or > > > > > cond_resched_rcu_qs() should be added to long in-kernel loops. > > > > > This commit therefore eliminates the decision by adding RCU > > > > > quiescent states to cond_resched(). > > > > > > > > Which would make: rcu_read_lock(); cond_resched(); rcu_read_unlock(); > > > > invalid under preemptible RCU. Is it already? > > > > > > In theory, yes. In practice, I just tested it with preemption and > > > lockdep enabled, and it didn't complain. If further testing finds > > > complaints, we can either fix those uses (preferred) or revert > > > this patch. > > > > > > > > Warning: This is a prototype. For example, it does not correctly > > > > > handle Tasks RCU. Which is OK for the moment, given that no one > > > > > actually uses Tasks RCU yet. > > > > > > > > > --- a/kernel/sched/core.c > > > > > +++ b/kernel/sched/core.c > > > > > @@ -4907,6 +4907,7 @@ int __sched _cond_resched(void) > > > > > preempt_schedule_common(); > > > > > return 1; > > > > > } > > > > > + rcu_all_qs(); > > > > > return 0; > > > > > } > > > > > > > > Still not a real fan of this, it does make cond_resched() touch a bunch > > > > more cachelines, also, I suppose that if we're going to do this, we > > > > should make __cond_resched_lock() and __cond_resched_softirq() act > > > > similarly. > > > > > > Michal (now CCed) argues that having to distinguish between cond_resched() > > > and cond_resched_rcu_qs() is overly burdensome. Michal? > > > > Yes, it is really not clear which one is meant to be in which context. I > > really do not see which cond_resched should be turned intto > > cond_resched_rcu_qs. > > > > > Any thoughts on how we might remove this burden without the additional > > > cache misses? I will take another look as well to see what could make > > > it lower cost. There are probably ways... Would it make sense to > > > have RCU maintain a need-rcu_all_qs() flage in the same cacheline as > > > the __preempt_count? Perhaps throttling the writes to this flag from > > > the RCU grace-period kthreads to once per 100 milliseconds or so? > > > > Can the stall detector simply request rescheduling when it gets > > dangerously close to the timeout? > > It is quite possible that half of the stall timeout would be a better > choice than my 100 milliseconds, but either way, there would be need > for a flag or some such. E.g. set_tsk_need_resched() on the task currently running on a cpu which is preventing the rcu grace period for too long? That would only require change to the stall detector and the cond_resched could be left alone completely. -- Michal Hocko SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> |
|---|---|
| Date | 2017-01-18 08:20 +0100 |
| Subject | Re: [PATCH tip/core/rcu 11/20] sched,rcu: Make cond_resched() provide RCU quiescent state |
| Message-ID | <t0SHg-13d-17@gated-at.bofh.it> |
| In reply to | #1560541 |
On Tue, Jan 17, 2017 at 01:11:46PM +0100, Michal Hocko wrote: > On Tue 17-01-17 04:05:13, Paul E. McKenney wrote: > > On Tue, Jan 17, 2017 at 11:51:41AM +0100, Michal Hocko wrote: > > > On Mon 16-01-17 16:54:03, Paul E. McKenney wrote: > > > > On Mon, Jan 16, 2017 at 06:11:30PM +0100, Peter Zijlstra wrote: > > > > > On Sat, Jan 14, 2017 at 01:13:12AM -0800, Paul E. McKenney wrote: > > > > > > There is some confusion as to which of cond_resched() or > > > > > > cond_resched_rcu_qs() should be added to long in-kernel loops. > > > > > > This commit therefore eliminates the decision by adding RCU > > > > > > quiescent states to cond_resched(). > > > > > > > > > > Which would make: rcu_read_lock(); cond_resched(); rcu_read_unlock(); > > > > > invalid under preemptible RCU. Is it already? > > > > > > > > In theory, yes. In practice, I just tested it with preemption and > > > > lockdep enabled, and it didn't complain. If further testing finds > > > > complaints, we can either fix those uses (preferred) or revert > > > > this patch. > > > > > > > > > > Warning: This is a prototype. For example, it does not correctly > > > > > > handle Tasks RCU. Which is OK for the moment, given that no one > > > > > > actually uses Tasks RCU yet. > > > > > > > > > > > --- a/kernel/sched/core.c > > > > > > +++ b/kernel/sched/core.c > > > > > > @@ -4907,6 +4907,7 @@ int __sched _cond_resched(void) > > > > > > preempt_schedule_common(); > > > > > > return 1; > > > > > > } > > > > > > + rcu_all_qs(); > > > > > > return 0; > > > > > > } > > > > > > > > > > Still not a real fan of this, it does make cond_resched() touch a bunch > > > > > more cachelines, also, I suppose that if we're going to do this, we > > > > > should make __cond_resched_lock() and __cond_resched_softirq() act > > > > > similarly. > > > > > > > > Michal (now CCed) argues that having to distinguish between cond_resched() > > > > and cond_resched_rcu_qs() is overly burdensome. Michal? > > > > > > Yes, it is really not clear which one is meant to be in which context. I > > > really do not see which cond_resched should be turned intto > > > cond_resched_rcu_qs. > > > > > > > Any thoughts on how we might remove this burden without the additional > > > > cache misses? I will take another look as well to see what could make > > > > it lower cost. There are probably ways... Would it make sense to > > > > have RCU maintain a need-rcu_all_qs() flage in the same cacheline as > > > > the __preempt_count? Perhaps throttling the writes to this flag from > > > > the RCU grace-period kthreads to once per 100 milliseconds or so? > > > > > > Can the stall detector simply request rescheduling when it gets > > > dangerously close to the timeout? > > > > It is quite possible that half of the stall timeout would be a better > > choice than my 100 milliseconds, but either way, there would be need > > for a flag or some such. > > E.g. set_tsk_need_resched() on the task currently running on a cpu which > is preventing the rcu grace period for too long? > > That would only require change to the stall detector and the cond_resched > could be left alone completely. Thank you!!! The other complication is that under CONFIG_PREEMPT=y, _cond_resched() is an empty function. That would be one reason why use of cond_resched() wasn't always giving RCU the quiescent states that it needs. And that is a problem with this patch, which I therefore need to defer to 4.12. That aside, the reason I am reluctant to use the need-resched approach except as an emergency measure is that the way I have to set that bit remotely involves IPIs. But don't get me wrong, it is extremely useful as an emergency meaure. I am just trying to get cond_resched() to help on a non-emergency basis. Thanx, Paul
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web