* [PATCH] rcu/tree: Default jiffies_to_sched_qs to jiffies_till_sched_qs
@ 2019-03-11 9:46 Neeraj Upadhyay
[not found] ` <20190311151854.GG13351@linux.ibm.com>
0 siblings, 1 reply; 5+ messages in thread
From: Neeraj Upadhyay @ 2019-03-11 9:46 UTC (permalink / raw)
To: paulmck, josh, rostedt, mathieu.desnoyers, jiangshanlai
Cc: rcu, Neeraj Upadhyay
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@codeaurora.org>
---
kernel/rcu/tree.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index ddd5c74..10aeb89 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 &&
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of the Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] rcu/tree: Default jiffies_to_sched_qs to jiffies_till_sched_qs
[not found] ` <20190311151854.GG13351@linux.ibm.com>
@ 2019-03-11 15:19 ` Paul E. McKenney
2019-03-11 17:35 ` Joel Fernandes
0 siblings, 1 reply; 5+ messages in thread
From: Paul E. McKenney @ 2019-03-11 15:19 UTC (permalink / raw)
To: Neeraj Upadhyay; +Cc: josh, rostedt, mathieu.desnoyers, jiangshanlai, rcu
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@codeaurora.org>
>
> 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@codeaurora.org>
> 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@codeaurora.org>
> Signed-off-by: Paul E. McKenney <paulmck@linux.ibm.com>
>
> 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 &&
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] rcu/tree: Default jiffies_to_sched_qs to jiffies_till_sched_qs
2019-03-11 15:19 ` Paul E. McKenney
@ 2019-03-11 17:35 ` Joel Fernandes
2019-03-11 22:47 ` Paul E. McKenney
0 siblings, 1 reply; 5+ messages in thread
From: Joel Fernandes @ 2019-03-11 17:35 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Neeraj Upadhyay, josh, rostedt, mathieu.desnoyers, jiangshanlai, rcu
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@codeaurora.org>
> >
> > 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@codeaurora.org>
> > 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@codeaurora.org>
> > Signed-off-by: Paul E. McKenney <paulmck@linux.ibm.com>
> >
> > 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] rcu/tree: Default jiffies_to_sched_qs to jiffies_till_sched_qs
2019-03-11 17:35 ` Joel Fernandes
@ 2019-03-11 22:47 ` Paul E. McKenney
2019-03-12 14:07 ` Joel Fernandes
0 siblings, 1 reply; 5+ messages in thread
From: Paul E. McKenney @ 2019-03-11 22:47 UTC (permalink / raw)
To: Joel Fernandes
Cc: Neeraj Upadhyay, josh, rostedt, mathieu.desnoyers, jiangshanlai, rcu
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@codeaurora.org>
> > >
> > > 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@codeaurora.org>
> > > 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@codeaurora.org>
> > > Signed-off-by: Paul E. McKenney <paulmck@linux.ibm.com>
> > >
> > > 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?
Thanx, Paul
------------------------------------------------------------------------
commit b1d5aaf8eff7872a63531f35aa0490f2fc8118d6
Author: Paul E. McKenney <paulmck@linux.ibm.com>
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@joelfernandes.org>
Signed-off-by: Paul E. McKenney <paulmck@linux.ibm.com>
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)
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] rcu/tree: Default jiffies_to_sched_qs to jiffies_till_sched_qs
2019-03-11 22:47 ` Paul E. McKenney
@ 2019-03-12 14:07 ` Joel Fernandes
0 siblings, 0 replies; 5+ messages in thread
From: Joel Fernandes @ 2019-03-12 14:07 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Neeraj Upadhyay, josh, rostedt, mathieu.desnoyers, jiangshanlai, rcu
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@codeaurora.org>
> > > >
> > > > 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@codeaurora.org>
> > > > 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@codeaurora.org>
> > > > Signed-off-by: Paul E. McKenney <paulmck@linux.ibm.com>
> > > >
> > > > 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@linux.ibm.com>
> 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@joelfernandes.org>
> Signed-off-by: Paul E. McKenney <paulmck@linux.ibm.com>
>
> 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)
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-03-12 14:07 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-11 9:46 [PATCH] rcu/tree: Default jiffies_to_sched_qs to jiffies_till_sched_qs Neeraj Upadhyay
[not found] ` <20190311151854.GG13351@linux.ibm.com>
2019-03-11 15:19 ` Paul E. McKenney
2019-03-11 17:35 ` Joel Fernandes
2019-03-11 22:47 ` Paul E. McKenney
2019-03-12 14:07 ` Joel Fernandes
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.