[PATCH] sched/rt: fix rt timer activation/deactivation

[PATCH] sched/rt: fix rt timer activation/deactivation

[Juri Lelli]

Destroy rt bandwidth timer when rq has no more RT tasks, even when
CONFIG_RT_GROUP_SCHED is not set.

Signed-off-by: Juri Lelli <juri.lelli@gmail.com>
---
 kernel/sched/rt.c |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index a2740b7..7dba25a 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -86,12 +86,12 @@ void init_rt_rq(struct rt_rq *rt_rq, struct rq *rq)
 	raw_spin_lock_init(&rt_rq->rt_runtime_lock);
 }
 
-#ifdef CONFIG_RT_GROUP_SCHED
 static void destroy_rt_bandwidth(struct rt_bandwidth *rt_b)
 {
 	hrtimer_cancel(&rt_b->rt_period_timer);
 }
 
+#ifdef CONFIG_RT_GROUP_SCHED
 #define rt_entity_is_task(rt_se) (!(rt_se)->my_q)
 
 static inline struct task_struct *rt_task_of(struct sched_rt_entity *rt_se)
@@ -1011,8 +1011,12 @@ inc_rt_group(struct sched_rt_entity *rt_se, struct rt_rq *rt_rq)
 	start_rt_bandwidth(&def_rt_bandwidth);
 }
 
-static inline
-void dec_rt_group(struct sched_rt_entity *rt_se, struct rt_rq *rt_rq) {}
+static void
+dec_rt_group(struct sched_rt_entity *rt_se, struct rt_rq *rt_rq)
+{
+	if (!rt_rq->rt_nr_running)
+		destroy_rt_bandwidth(&def_rt_bandwidth);
+}
 
 #endif /* CONFIG_RT_GROUP_SCHED */
 
-- 
1.7.9.5

Re: [PATCH] sched/rt: fix rt timer activation/deactivation

[Kirill Tkhai]

On Вт, 2014-02-25 at 17:05 +0100, Juri Lelli wrote:
> Destroy rt bandwidth timer when rq has no more RT tasks, even when
> CONFIG_RT_GROUP_SCHED is not set.
> 
> Signed-off-by: Juri Lelli <juri.lelli@gmail.com>
> ---
>  kernel/sched/rt.c |   10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> index a2740b7..7dba25a 100644
> --- a/kernel/sched/rt.c
> +++ b/kernel/sched/rt.c
> @@ -86,12 +86,12 @@ void init_rt_rq(struct rt_rq *rt_rq, struct rq *rq)
>  	raw_spin_lock_init(&rt_rq->rt_runtime_lock);
>  }
>  
> -#ifdef CONFIG_RT_GROUP_SCHED
>  static void destroy_rt_bandwidth(struct rt_bandwidth *rt_b)
>  {
>  	hrtimer_cancel(&rt_b->rt_period_timer);
>  }
>  
> +#ifdef CONFIG_RT_GROUP_SCHED
>  #define rt_entity_is_task(rt_se) (!(rt_se)->my_q)
>  
>  static inline struct task_struct *rt_task_of(struct sched_rt_entity *rt_se)
> @@ -1011,8 +1011,12 @@ inc_rt_group(struct sched_rt_entity *rt_se, struct rt_rq *rt_rq)
>  	start_rt_bandwidth(&def_rt_bandwidth);
>  }
>  
> -static inline
> -void dec_rt_group(struct sched_rt_entity *rt_se, struct rt_rq *rt_rq) {}
> +static void
> +dec_rt_group(struct sched_rt_entity *rt_se, struct rt_rq *rt_rq)
> +{
> +	if (!rt_rq->rt_nr_running)
> +		destroy_rt_bandwidth(&def_rt_bandwidth);
> +}

The problem is bandwidth timer is not per-cpu. It's only for all
processors from the span (sched_rt_period_mask()). Other CPUs may
have enqueued RT tasks. So, it's not possible to do this.

>  
>  #endif /* CONFIG_RT_GROUP_SCHED */
>  

Re: [PATCH] sched/rt: fix rt timer activation/deactivation

[Mike Galbraith]

On Wed, 2014-02-26 at 00:56 +0400, Kirill Tkhai wrote: 
> On Вт, 2014-02-25 at 17:05 +0100, Juri Lelli wrote:
> > Destroy rt bandwidth timer when rq has no more RT tasks, even when
> > CONFIG_RT_GROUP_SCHED is not set.
> > 
> > Signed-off-by: Juri Lelli <juri.lelli@gmail.com>
> > ---
> >  kernel/sched/rt.c |   10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> > index a2740b7..7dba25a 100644
> > --- a/kernel/sched/rt.c
> > +++ b/kernel/sched/rt.c
> > @@ -86,12 +86,12 @@ void init_rt_rq(struct rt_rq *rt_rq, struct rq *rq)
> >  	raw_spin_lock_init(&rt_rq->rt_runtime_lock);
> >  }
> >  
> > -#ifdef CONFIG_RT_GROUP_SCHED
> >  static void destroy_rt_bandwidth(struct rt_bandwidth *rt_b)
> >  {
> >  	hrtimer_cancel(&rt_b->rt_period_timer);
> >  }
> >  
> > +#ifdef CONFIG_RT_GROUP_SCHED
> >  #define rt_entity_is_task(rt_se) (!(rt_se)->my_q)
> >  
> >  static inline struct task_struct *rt_task_of(struct sched_rt_entity *rt_se)
> > @@ -1011,8 +1011,12 @@ inc_rt_group(struct sched_rt_entity *rt_se, struct rt_rq *rt_rq)
> >  	start_rt_bandwidth(&def_rt_bandwidth);
> >  }
> >  
> > -static inline
> > -void dec_rt_group(struct sched_rt_entity *rt_se, struct rt_rq *rt_rq) {}
> > +static void
> > +dec_rt_group(struct sched_rt_entity *rt_se, struct rt_rq *rt_rq)
> > +{
> > +	if (!rt_rq->rt_nr_running)
> > +		destroy_rt_bandwidth(&def_rt_bandwidth);
> > +}
> 
> The problem is bandwidth timer is not per-cpu. It's only for all
> processors from the span (sched_rt_period_mask()). Other CPUs may
> have enqueued RT tasks. So, it's not possible to do this.

BTW, I noticed you can no longer turn the turn the noisy thing off since
we grew DL.  I added an old SGI boot parameter to tell it to go away.

-Mike

Re: [PATCH] sched/rt: fix rt timer activation/deactivation

[Peter Zijlstra]

On Wed, Feb 26, 2014 at 03:37:38AM +0100, Mike Galbraith wrote:
> BTW, I noticed you can no longer turn the turn the noisy thing off since
> we grew DL.  I added an old SGI boot parameter to tell it to go away.

You're talking about the rt badnwidth timer, right?

 1) why won't it go away with DL added?

 2) it should never appear when !rt_bandwidth_enabled(), so if you set
 sysctl_sched_rt_runtime to -1 all this should go away already, no extra
 patches required.

Re: [PATCH] sched/rt: fix rt timer activation/deactivation

[Kirill Tkhai]

26.02.2014, 13:07, "Peter Zijlstra" <peterz@infradead.org>:
> On Wed, Feb 26, 2014 at 03:37:38AM +0100, Mike Galbraith wrote:
>
>>  BTW, I noticed you can no longer turn the turn the noisy thing off since
>>  we grew DL.  I added an old SGI boot parameter to tell it to go away.
>
> You're talking about the rt badnwidth timer, right?
>
>  1) why won't it go away with DL added?

We know nothing about rt_time and rt_runtime values, when we are doing
update_curr_dl().

rt_time maybe not zero, so we increment rt_time.

rt_rq->rt_runtime maybe less than maximum available for RT(and for dl too),
because someone could borrow a part of rt_runtime during previous
balance_runtime().

If dl task uses all available runtime, we always will have "idle = 0"

        if (rt_rq->rt_time || rt_rq->rt_nr_running)
                idle = 0;

in do_sched_rt_period_timer(). Timer will be restarted.

This is boundary case, of course.

>  2) it should never appear when !rt_bandwidth_enabled(), so if you set
>  sysctl_sched_rt_runtime to -1 all this should go away already, no extra
>  patches required.

Re: [PATCH] sched/rt: fix rt timer activation/deactivation

[Kirill Tkhai]

26.02.2014, 13:35, "Kirill Tkhai" <tkhai@yandex.ru>:
> 26.02.2014, 13:07, "Peter Zijlstra" <peterz@infradead.org>:
>
>>  On Wed, Feb 26, 2014 at 03:37:38AM +0100, Mike Galbraith wrote:
>>>   BTW, I noticed you can no longer turn the turn the noisy thing off since
>>>   we grew DL.  I added an old SGI boot parameter to tell it to go away.
>>  You're talking about the rt badnwidth timer, right?
>>
>>   1) why won't it go away with DL added?
>
> We know nothing about rt_time and rt_runtime values, when we are doing
> update_curr_dl().
>
> rt_time maybe not zero, so we increment rt_time.
>
> rt_rq->rt_runtime maybe less than maximum available for RT(and for dl too),
> because someone could borrow a part of rt_runtime during previous
> balance_runtime().
>
> If dl task uses all available runtime, we always will have "idle = 0"
>
>         if (rt_rq->rt_time || rt_rq->rt_nr_running)
>                 idle = 0;
>
> in do_sched_rt_period_timer(). Timer will be restarted.
>
> This is boundary case, of course.

I wrote this in assumtion, that patch like Juri's is applied
[https://lkml.org/lkml/2014/2/25/307].

In current tip.git state above is always.


>>   2) it should never appear when !rt_bandwidth_enabled(), so if you set
>>   sysctl_sched_rt_runtime to -1 all this should go away already, no extra
>>   patches required.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] sched/rt: fix rt timer activation/deactivation

[Mike Galbraith]

On Wed, 2014-02-26 at 10:07 +0100, Peter Zijlstra wrote: 
> On Wed, Feb 26, 2014 at 03:37:38AM +0100, Mike Galbraith wrote:
> > BTW, I noticed you can no longer turn the turn the noisy thing off since
> > we grew DL.  I added an old SGI boot parameter to tell it to go away.
> 
> You're talking about the rt badnwidth timer, right?

Right.

> 1) why won't it go away with DL added?

Hm, it _was_ saying go away when I said -1, but 7472e009a3f1 is obeying,
so I guess it got fixed.. so never mind.

-Mike