On Wed, Jul 01, 2015 at 12:03:52PM +0200, Peter Zijlstra wrote: > On Tue, Jun 30, 2015 at 02:48:27PM -0700, Paul E. McKenney wrote: > > @@ -2121,17 +2137,24 @@ static int __noreturn rcu_gp_kthread(void *arg) > > TPS("fqswait")); > > rsp->gp_state = RCU_GP_WAIT_FQS; > > ret = wait_event_interruptible_timeout(rsp->gp_wq, > > - ((gf = READ_ONCE(rsp->gp_flags)) & > > - RCU_GP_FLAG_FQS) || > > - (!READ_ONCE(rnp->qsmask) && > > - !rcu_preempt_blocked_readers_cgp(rnp)), > > - j); > > + ((gf = READ_ONCE(rsp->gp_flags)) & > > + RCU_GP_FLAG_FQS) || > > + (!READ_ONCE(rnp->qsmask) && > > + !rcu_preempt_blocked_readers_cgp(rnp)) || > > + rcu_exp_gp_seq_done(rsp->exp_rsp, > > + rsp->gp_exp_snap), > > + j); > > How about using a helper function there? > > static inline bool rcu_gp_done(rsp, rnp) > { > /* Forced Quiescent State complete */ > if (READ_ONCE(rsp->gp_flags) & RCU_GP_FLAG_FQS) > return true; > > /* QS not masked and not blocked by preempted readers */ > if (!READ_ONCE(rnp->qsmask) && > !rcu_preempt_blocked_readers_cgp(rnp)) > return true; > > /* Expedited Grace Period completed */ > if (rcu_exp_gp_seq_done(rsp)) > return true; > > return false; > } > > ret = wait_event_interruptible_timeout(rsp->gp_wq, > rcu_gp_done(rsp, rnp), j);
Fair point, and applies to the original as well, give or take comments and naming. Please see below for the corresponding patch. Thoughts? Thanx, Paul ------------------------------------------------------------------------ commit a8bb9abbf3690e507d61efe8144cec091ce5fb02 Author: Paul E. McKenney <paul...@linux.vnet.ibm.com> Date: Wed Jul 1 13:50:28 2015 -0700 rcu: Pull out wait_event*() condition into helper function The condition for the wait_event_interruptible_timeout() that waits to do the next force-quiescent-state scan is a bit ornate: ((gf = READ_ONCE(rsp->gp_flags)) & RCU_GP_FLAG_FQS) || (!READ_ONCE(rnp->qsmask) && !rcu_preempt_blocked_readers_cgp(rnp)) This commit therefore pulls this condition out into a helper function and comments its component conditions. Reported-by: Peter Zijlstra <pet...@infradead.org> Signed-off-by: Paul E. McKenney <paul...@linux.vnet.ibm.com> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index 4992bfd360b6..2afb8e8c5134 100644 --- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -1904,6 +1904,26 @@ static int rcu_gp_init(struct rcu_state *rsp) } /* + * Helper function for wait_event_interruptible_timeout() wakeup + * at force-quiescent-state time. + */ +static bool rcu_gp_fqs_check_wake(struct rcu_state *rsp, int *gfp) +{ + struct rcu_node *rnp = rcu_get_root(rsp); + + /* Someone like call_rcu() requested a force-quiescent-state scan. */ + *gfp = READ_ONCE(rsp->gp_flags); + if (*gfp & RCU_GP_FLAG_FQS) + return true; + + /* The current grace period has completed. */ + if (!READ_ONCE(rnp->qsmask) && !rcu_preempt_blocked_readers_cgp(rnp)) + return true; + + return false; +} + +/* * Do one round of quiescent-state forcing. */ static int rcu_gp_fqs(struct rcu_state *rsp, int fqs_state_in) @@ -2067,11 +2087,7 @@ static int __noreturn rcu_gp_kthread(void *arg) TPS("fqswait")); rsp->gp_state = RCU_GP_WAIT_FQS; ret = wait_event_interruptible_timeout(rsp->gp_wq, - ((gf = READ_ONCE(rsp->gp_flags)) & - RCU_GP_FLAG_FQS) || - (!READ_ONCE(rnp->qsmask) && - !rcu_preempt_blocked_readers_cgp(rnp)), - j); + rcu_gp_fqs_check_wake(rsp, &gf), j); rsp->gp_state = RCU_GP_DONE_FQS; /* Locking provides needed memory barriers. */ /* If grace period done, leave loop. */ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/