On Mon, Mar 11, 2019 at 03:47:42PM -0700, Paul E. McKenney wrote: > On Mon, Mar 11, 2019 at 01:35:40PM -0400, Joel Fernandes wrote: > > On Mon, Mar 11, 2019 at 08:19:47AM -0700, Paul E. McKenney wrote: > > > This time keeping the CC list... > > > > > > Thanx, Paul > > > > > > On Mon, Mar 11, 2019 at 08:18:54AM -0700, Paul E. McKenney wrote: > > > > On Mon, Mar 11, 2019 at 03:16:11PM +0530, Neeraj Upadhyay wrote: > > > > > Current code does not call adjust_jiffies_till_sched_qs(), > > > > > if jiffies_till_sched_qs is specified. For the case, where > > > > > jiffies_till_first_fqs and jiffies_till_next_fqs are default, > > > > > jiffies_to_sched_qs won't be a correct adjustment of > > > > > jiffies_till_sched_qs. > > > > > > > > > > Signed-off-by: Neeraj Upadhyay <neeraju@xxxxxxxxxxxxxx> > > > > > > > > Good catch! Queued and pushed. Please see below for updated > > > > commit log. On future patches, could you please first describe > > > > the problem and consequences, then what the fix is? This approach > > > > makes it much easier for people later on who will be trying to > > > > figure out what is going on, and who might or might not have much > > > > understanding of RCU. (For example, they might be doing a bisection > > > > or some such.) > > > > > > > > Not a big deal, as I can touch this up, but a good habit to get into. > > > > > > > > And no, rcutorture currently does not specify non-default values > > > > for jiffies_till_sched_qs. Which should probably be fixed. I could > > > > make rcu_implicit_dynticks_qs() complain if jiffies_to_sched_qs is > > > > zero, but that feels a bit hacky and specific. :-/ > > > > > > > > Thanx, Paul > > > > > > > > ------------------------------------------------------------------------ > > > > > > > > commit ee474b85fa0815be940ed89a91e0d84a110a0a92 > > > > Author: Neeraj Upadhyay <neeraju@xxxxxxxxxxxxxx> > > > > Date: Mon Mar 11 15:16:11 2019 +0530 > > > > > > > > rcu: Default jiffies_to_sched_qs to jiffies_till_sched_qs > > > > > > > > The current code only calls adjust_jiffies_till_sched_qs() if > > > > jiffies_till_sched_qs is left at its default value, so when the > > > > jiffies_till_sched_qs kernel-boot parameter actually is specified, > > > > jiffies_to_sched_qs will be left with the value zero, which > > > > will result in useless slowdowns of cond_resched(). This commit > > > > therefore changes rcu_init_geometry() to unconditionally invoke > > > > adjust_jiffies_till_sched_qs(), which ensures that jiffies_to_sched_qs > > > > will be initialized in all cases, thus maintaining good cond_resched() > > > > performance. > > > > > > > > Signed-off-by: Neeraj Upadhyay <neeraju@xxxxxxxxxxxxxx> > > > > Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxx> > > > > > > > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c > > > > index ddd5c74e386b..10aeb89395ea 100644 > > > > --- a/kernel/rcu/tree.c > > > > +++ b/kernel/rcu/tree.c > > > > @@ -3259,8 +3259,7 @@ static void __init rcu_init_geometry(void) > > > > jiffies_till_first_fqs = d; > > > > if (jiffies_till_next_fqs == ULONG_MAX) > > > > jiffies_till_next_fqs = d; > > > > - if (jiffies_till_sched_qs == ULONG_MAX) > > > > - adjust_jiffies_till_sched_qs(); > > > > + adjust_jiffies_till_sched_qs(); > > > > > > > > /* If the compile-time values are accurate, just leave. */ > > > > if (rcu_fanout_leaf == RCU_FANOUT_LEAF && > > > > > > > Makes sense to me. > > > > Also the comment here needs an update too I think: > > > > static ulong jiffies_to_sched_qs; /* Adjusted version of above if not default */ > > > > Seems to me, after your patch jiffies_to_sched_qs will always be an adjusted > > value of some sort, unless jiffies_till_sched_qs is specified. > > > > Comment should be some thing like this then? > > > > /* Either the above, or an adjusted default version based on > > * jiffies_till_{first,next}_fqs if it is not specified */ > > Good point, but how about the patch below? Yes, your comment is better :-) thanks, - Joel > Thanx, Paul > > ------------------------------------------------------------------------ > > commit b1d5aaf8eff7872a63531f35aa0490f2fc8118d6 > Author: Paul E. McKenney <paulmck@xxxxxxxxxxxxx> > Date: Mon Mar 11 15:45:13 2019 -0700 > > rcu: Update jiffies_to_sched_qs and adjust_jiffies_till_sched_qs() comments > > This commit better documents the jiffies_to_sched_qs default-value > strategy used by adjust_jiffies_till_sched_qs() > > Reported-by: Joel Fernandes <joel@xxxxxxxxxxxxxxxxx> > Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxx> > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c > index 10aeb89395ea..355775a82581 100644 > --- a/kernel/rcu/tree.c > +++ b/kernel/rcu/tree.c > @@ -400,7 +400,7 @@ static bool rcu_kick_kthreads; > */ > static ulong jiffies_till_sched_qs = ULONG_MAX; > module_param(jiffies_till_sched_qs, ulong, 0444); > -static ulong jiffies_to_sched_qs; /* Adjusted version of above if not default */ > +static ulong jiffies_to_sched_qs; /* See adjust_jiffies_till_sched_qs(). */ > module_param(jiffies_to_sched_qs, ulong, 0444); /* Display only! */ > > /* > @@ -418,6 +418,7 @@ static void adjust_jiffies_till_sched_qs(void) > WRITE_ONCE(jiffies_to_sched_qs, jiffies_till_sched_qs); > return; > } > + /* Otherwise, set to third fqs scan, but bound below on large system. */ > j = READ_ONCE(jiffies_till_first_fqs) + > 2 * READ_ONCE(jiffies_till_next_fqs); > if (j < HZ / 10 + nr_cpu_ids / RCU_JIFFIES_FQS_DIV) >