Messages in this thread | | | Date | Fri, 11 May 2018 09:17:46 -0700 | From | "Paul E. McKenney" <> | Subject | Re: [PATCH] rcu: Report a quiescent state when it's exactly in the state |
| |
On Fri, May 11, 2018 at 09:57:54PM +0900, Byungchul Park wrote: > Hello folks, > > I think I wrote the title in a misleading way. > > Please change the title to something else such as, > "rcu: Report a quiescent state when it's in the state" or, > "rcu: Add points reporting quiescent states where proper" or so on. > > On 2018-05-11 오후 5:30, Byungchul Park wrote: > >We expect a quiescent state of TASKS_RCU when cond_resched_tasks_rcu_qs() > >is called, no matter whether it actually be scheduled or not. However, > >it currently doesn't report the quiescent state when the task enters > >into __schedule() as it's called with preempt = true. So make it report > >the quiescent state unconditionally when cond_resched_tasks_rcu_qs() is > >called. > > > >And in TINY_RCU, even though the quiescent state of rcu_bh also should > >be reported when the tick interrupt comes from user, it doesn't. So make > >it reported. > > > >Lastly in TREE_RCU, rcu_note_voluntary_context_switch() should be > >reported when the tick interrupt comes from not only user but also idle, > >as an extended quiescent state. > > > >Signed-off-by: Byungchul Park <byungchul.park@lge.com> > >--- > > include/linux/rcupdate.h | 4 ++-- > > kernel/rcu/tiny.c | 6 +++--- > > kernel/rcu/tree.c | 4 ++-- > > 3 files changed, 7 insertions(+), 7 deletions(-) > > > >diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h > >index ee8cf5fc..7432261 100644 > >--- a/include/linux/rcupdate.h > >+++ b/include/linux/rcupdate.h > >@@ -195,8 +195,8 @@ static inline void exit_tasks_rcu_finish(void) { } > > */ > > #define cond_resched_tasks_rcu_qs() \ > > do { \ > >- if (!cond_resched()) \ > >- rcu_note_voluntary_context_switch_lite(current); \ > >+ rcu_note_voluntary_context_switch_lite(current); \ > >+ cond_resched(); \
Ah, good point.
Peter, I have to ask... Why is "cond_resched()" considered a preemption while "schedule()" is not?
> > } while (0) > > /* > >diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c > >index a64eee0..68d2332 100644 > >--- a/kernel/rcu/tiny.c > >+++ b/kernel/rcu/tiny.c > >@@ -120,12 +120,12 @@ void rcu_bh_qs(void) > > */ > > void rcu_check_callbacks(int user) > > { > >- if (user) > >+ if (user) { > > rcu_sched_qs(); > >- else if (!in_softirq()) > > rcu_bh_qs(); > >- if (user) > > rcu_note_voluntary_context_switch(current); > >+ } else if (!in_softirq()) > >+ rcu_bh_qs(); > > } > > /* > >diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c > >index 36075dd..1abe29a 100644 > >--- a/kernel/rcu/tree.c > >+++ b/kernel/rcu/tree.c > >@@ -2595,6 +2595,7 @@ void rcu_check_callbacks(int user) > > rcu_sched_qs(); > > rcu_bh_qs(); > >+ rcu_note_voluntary_context_switch(current); > > } else if (!in_softirq()) { > >@@ -2610,8 +2611,7 @@ void rcu_check_callbacks(int user) > > rcu_preempt_check_callbacks(); > > if (rcu_pending()) > > invoke_rcu_core(); > >- if (user) > >- rcu_note_voluntary_context_switch(current); > >+
I recall that I had some reason for wanting this down here, but do not recall the reason itself. I will try testing this patch to see if rcutorture reminds me.
Thanx, Paul
> > trace_rcu_utilization(TPS("End scheduler-tick")); > > } > > > > -- > Thanks, > Byungchul >
| |