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 */
thanks,
- Joel
