Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1434522 > unrolled thread

Re: [PATCH 2/3] sched: Unloop sched avg decaying

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/3] sched: Unloop sched avg decaying Frederic Weisbecker <fweisbec@gmail.com> - 2016-06-30 15:00 +0200
    Re: [PATCH 2/3] sched: Unloop sched avg decaying Peter Zijlstra <peterz@infradead.org> - 2016-06-30 16:00 +0200
      Re: [PATCH 2/3] sched: Unloop sched avg decaying Frederic Weisbecker <fweisbec@gmail.com> - 2016-07-06 14:00 +0200

#1434522 — Re: [PATCH 2/3] sched: Unloop sched avg decaying

On Tue, Jun 14, 2016 at 05:58:42PM +0200, Peter Zijlstra wrote:
> On Tue, Jun 14, 2016 at 05:28:01PM +0200, Frederic Weisbecker wrote:
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index 385c947..0c0578a 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -666,17 +666,17 @@ bool sched_can_stop_tick(struct rq *rq)
> >  void sched_avg_update(struct rq *rq)
> >  {
> >  	s64 period = sched_avg_period();
> > +	s64 delta;
> > +	u64 rem;
> > +	int pending;
> >  
> > +	delta = (s64)(rq_clock(rq) - rq->age_stamp);
> > +	if (delta <= period)
> > +		return;
> > +
> > +	pending = div64_u64_rem(delta, period, &rem);
> > +	rq->age_stamp += delta - rem;
> > +	rq->rt_avg >>= pending;
> >  }
> 
> Blergh, and now do the profile on machine that doesn't have major
> transistor count dedicated to divisions.

Indeed I was afraid of something like that. That said I'm not sure which
is worse between iteration or division out of native asm.

Would it be worth testing?

> 
> Why not add the division to the nohz exit path only?

It would be worse I think because we may exit much more often from nohz
than we reach a sched_avg_period().

So the only safe optimization I can do for now is:

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 8b489fc..4632d31 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -626,8 +626,9 @@ bool sched_can_stop_tick(struct rq *rq)
 void sched_avg_update(struct rq *rq)
 {
 	s64 period = sched_avg_period();
+	int i;
 
-	while ((s64)(rq_clock(rq) - rq->age_stamp) > period) {
+	for (i = 0; (s64)(rq_clock(rq) - rq->age_stamp) > period); i++) {
 		/*
 		 * Inline assembly required to prevent the compiler
 		 * optimising this loop into a divmod call.
@@ -635,8 +636,8 @@ void sched_avg_update(struct rq *rq)
 		 */
 		asm("" : "+rm" (rq->age_stamp));
 		rq->age_stamp += period;
-		rq->rt_avg /= 2;
 	}
+	rq->rt_avg >>= i;
 }
 
 #endif /* CONFIG_SMP */

[toc] | [next] | [standalone]

#1434585

On Thu, Jun 30, 2016 at 02:52:26PM +0200, Frederic Weisbecker wrote:
> On Tue, Jun 14, 2016 at 05:58:42PM +0200, Peter Zijlstra wrote:

> > Why not add the division to the nohz exit path only?
> 
> It would be worse I think because we may exit much more often from nohz
> than we reach a sched_avg_period().
> 
> So the only safe optimization I can do for now is:

How about something like this then?

---

 kernel/sched/core.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 3387e4f14fc9..fd1ae4c4105f 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -665,9 +665,23 @@ bool sched_can_stop_tick(struct rq *rq)
 
 void sched_avg_update(struct rq *rq)
 {
-	s64 period = sched_avg_period();
+	s64 delta, period = sched_avg_period();
 
-	while ((s64)(rq_clock(rq) - rq->age_stamp) > period) {
+	delta = (s64)(rq_clock(rq) - rq->age_stamp);
+	if (likely(delta < period))
+		return;
+
+	if (unlikely(delta > 3*period)) {
+		int pending;
+		u64 rem;
+
+		pending = div64_u64_rem(delta, period, &rem);
+		rq->age_stamp += delta - rem;
+		rq->rt_avg >>= pending;
+		return;
+	}
+
+	while (delta > period) {
 		/*
 		 * Inline assembly required to prevent the compiler
 		 * optimising this loop into a divmod call.
@@ -675,6 +689,7 @@ void sched_avg_update(struct rq *rq)
 		 */
 		asm("" : "+rm" (rq->age_stamp));
 		rq->age_stamp += period;
+		delta -= period;
 		rq->rt_avg /= 2;
 	}
 }

[toc] | [prev] | [next] | [standalone]

#1437662

On Thu, Jun 30, 2016 at 03:20:37PM +0200, Peter Zijlstra wrote:
> On Thu, Jun 30, 2016 at 02:52:26PM +0200, Frederic Weisbecker wrote:
> > On Tue, Jun 14, 2016 at 05:58:42PM +0200, Peter Zijlstra wrote:
> 
> > > Why not add the division to the nohz exit path only?
> > 
> > It would be worse I think because we may exit much more often from nohz
> > than we reach a sched_avg_period().
> > 
> > So the only safe optimization I can do for now is:
> 
> How about something like this then?
> 
> ---
> 
>  kernel/sched/core.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 3387e4f14fc9..fd1ae4c4105f 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -665,9 +665,23 @@ bool sched_can_stop_tick(struct rq *rq)
>  
>  void sched_avg_update(struct rq *rq)
>  {
> -	s64 period = sched_avg_period();
> +	s64 delta, period = sched_avg_period();
>  
> -	while ((s64)(rq_clock(rq) - rq->age_stamp) > period) {
> +	delta = (s64)(rq_clock(rq) - rq->age_stamp);
> +	if (likely(delta < period))
> +		return;
> +
> +	if (unlikely(delta > 3*period)) {
> +		int pending;
> +		u64 rem;
> +
> +		pending = div64_u64_rem(delta, period, &rem);
> +		rq->age_stamp += delta - rem;
> +		rq->rt_avg >>= pending;
> +		return;
> +	}
> +
> +	while (delta > period) {
>  		/*
>  		 * Inline assembly required to prevent the compiler
>  		 * optimising this loop into a divmod call.
> @@ -675,6 +689,7 @@ void sched_avg_update(struct rq *rq)
>  		 */
>  		asm("" : "+rm" (rq->age_stamp));
>  		rq->age_stamp += period;
> +		delta -= period;
>  		rq->rt_avg /= 2;
>  	}
>  }

Makes sense. I'm going to do some tests. We might want to precompute 3*period maybe.

Thanks.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web