2014-02-25 16:05:26

by Juri Lelli

[permalink] [raw]
Subject: [PATCH] sched/rt: fix rt timer activation/deactivation

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 <[email protected]>
---
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


2014-02-25 20:59:37

by Kirill Tkhai

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

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 <[email protected]>
> ---
> 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 */
>

2014-02-26 02:43:37

by Mike Galbraith

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

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 <[email protected]>
> > ---
> > 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

2014-02-26 09:07:16

by Peter Zijlstra

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

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.

2014-02-26 09:35:53

by Kirill Tkhai

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



26.02.2014, 13:07, "Peter Zijlstra" <[email protected]>:
> 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.

2014-02-26 10:01:26

by Kirill Tkhai

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



26.02.2014, 13:35, "Kirill Tkhai" <[email protected]>:
> 26.02.2014, 13:07, "Peter Zijlstra" <[email protected]>:
>
>> ?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 [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/

2014-02-26 10:02:19

by Mike Galbraith

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

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