Re: [PATCH] sched/rt: fix call to cpufreq_update_util

[Vincent Guittot]

On 17 May 2018 at 06:08, Viresh Kumar  wrote:
> On 16-05-18, 20:18, Vincent Guittot wrote:
>> With commit 8f111bc357aa ("cpufreq/schedutil: Rewrite CPUFREQ_RT support")
>> schedutil governor uses rq->rt.rt_nr_running to detect whether a RT task is
>> currently running on the CPU and to set frequency to max if necessary.
>> cpufreq_update_util() is called in enqueue/dequeue_top_rt_rq() but
>> rq->rt.rt_nr_running as not been updated yet when dequeue_top_rt_rq() is
>> called so schedutil still considers that a RT task is running when the
>> last task is dequeued. The update of rq->rt.rt_nr_running happens later
>> in dequeue_rt_stack()
>>
>> Fixes: 8f111bc357aa ('cpufreq/schedutil: Rewrite CPUFREQ_RT support')
>> Cc:  # v4.16+
>> Signed-off-by: Vincent Guittot 
>> ---
>>  kernel/sched/rt.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
>> index 7aef6b4..6e74d3d 100644
>> --- a/kernel/sched/rt.c
>> +++ b/kernel/sched/rt.c
>> @@ -1001,8 +1001,6 @@ dequeue_top_rt_rq(struct rt_rq *rt_rq)
>>   sub_nr_running(rq, rt_rq->rt_nr_running);
>>   rt_rq->rt_queued = 0;
>>
>
> Remove this blank line as well ?

Yes good point

>
>> - /* Kick cpufreq (see the comment in kernel/sched/sched.h). */
>> - cpufreq_update_util(rq, 0);
>>  }
>>
>>  static void
>> @@ -1288,6 +1286,9 @@ static void dequeue_rt_stack(struct sched_rt_entity 
>> *rt_se, unsigned int flags)
>>   if (on_rt_rq(rt_se))
>>   __dequeue_rt_entity(rt_se, flags);
>>   }
>> +
>> + /* Kick cpufreq (see the comment in kernel/sched/sched.h). */
>> + cpufreq_update_util(rq_of_rt_rq(rt_rq_of_se(back)), 0);
>>  }
>>
>>  static void enqueue_rt_entity(struct sched_rt_entity *rt_se, unsigned int 
>> flags)
>> --
>> 2.7.4
>
> --
> viresh

Re: [PATCH] sched/rt: fix call to cpufreq_update_util

[Vincent Guittot]

On 17 May 2018 at 06:08, Viresh Kumar  wrote:
> On 16-05-18, 20:18, Vincent Guittot wrote:
>> With commit 8f111bc357aa ("cpufreq/schedutil: Rewrite CPUFREQ_RT support")
>> schedutil governor uses rq->rt.rt_nr_running to detect whether a RT task is
>> currently running on the CPU and to set frequency to max if necessary.
>> cpufreq_update_util() is called in enqueue/dequeue_top_rt_rq() but
>> rq->rt.rt_nr_running as not been updated yet when dequeue_top_rt_rq() is
>> called so schedutil still considers that a RT task is running when the
>> last task is dequeued. The update of rq->rt.rt_nr_running happens later
>> in dequeue_rt_stack()
>>
>> Fixes: 8f111bc357aa ('cpufreq/schedutil: Rewrite CPUFREQ_RT support')
>> Cc:  # v4.16+
>> Signed-off-by: Vincent Guittot 
>> ---
>>  kernel/sched/rt.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
>> index 7aef6b4..6e74d3d 100644
>> --- a/kernel/sched/rt.c
>> +++ b/kernel/sched/rt.c
>> @@ -1001,8 +1001,6 @@ dequeue_top_rt_rq(struct rt_rq *rt_rq)
>>   sub_nr_running(rq, rt_rq->rt_nr_running);
>>   rt_rq->rt_queued = 0;
>>
>
> Remove this blank line as well ?

Yes good point

>
>> - /* Kick cpufreq (see the comment in kernel/sched/sched.h). */
>> - cpufreq_update_util(rq, 0);
>>  }
>>
>>  static void
>> @@ -1288,6 +1286,9 @@ static void dequeue_rt_stack(struct sched_rt_entity 
>> *rt_se, unsigned int flags)
>>   if (on_rt_rq(rt_se))
>>   __dequeue_rt_entity(rt_se, flags);
>>   }
>> +
>> + /* Kick cpufreq (see the comment in kernel/sched/sched.h). */
>> + cpufreq_update_util(rq_of_rt_rq(rt_rq_of_se(back)), 0);
>>  }
>>
>>  static void enqueue_rt_entity(struct sched_rt_entity *rt_se, unsigned int 
>> flags)
>> --
>> 2.7.4
>
> --
> viresh

Re: [PATCH] sched/rt: fix call to cpufreq_update_util

[Viresh Kumar]

On 16-05-18, 20:18, Vincent Guittot wrote:
> With commit 8f111bc357aa ("cpufreq/schedutil: Rewrite CPUFREQ_RT support")
> schedutil governor uses rq->rt.rt_nr_running to detect whether a RT task is
> currently running on the CPU and to set frequency to max if necessary.
> cpufreq_update_util() is called in enqueue/dequeue_top_rt_rq() but
> rq->rt.rt_nr_running as not been updated yet when dequeue_top_rt_rq() is
> called so schedutil still considers that a RT task is running when the
> last task is dequeued. The update of rq->rt.rt_nr_running happens later
> in dequeue_rt_stack()
> 
> Fixes: 8f111bc357aa ('cpufreq/schedutil: Rewrite CPUFREQ_RT support')
> Cc:  # v4.16+
> Signed-off-by: Vincent Guittot 
> ---
>  kernel/sched/rt.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> index 7aef6b4..6e74d3d 100644
> --- a/kernel/sched/rt.c
> +++ b/kernel/sched/rt.c
> @@ -1001,8 +1001,6 @@ dequeue_top_rt_rq(struct rt_rq *rt_rq)
>   sub_nr_running(rq, rt_rq->rt_nr_running);
>   rt_rq->rt_queued = 0;
>  

Remove this blank line as well ?

> - /* Kick cpufreq (see the comment in kernel/sched/sched.h). */
> - cpufreq_update_util(rq, 0);
>  }
>  
>  static void
> @@ -1288,6 +1286,9 @@ static void dequeue_rt_stack(struct sched_rt_entity 
> *rt_se, unsigned int flags)
>   if (on_rt_rq(rt_se))
>   __dequeue_rt_entity(rt_se, flags);
>   }
> +
> + /* Kick cpufreq (see the comment in kernel/sched/sched.h). */
> + cpufreq_update_util(rq_of_rt_rq(rt_rq_of_se(back)), 0);
>  }
>  
>  static void enqueue_rt_entity(struct sched_rt_entity *rt_se, unsigned int 
> flags)
> -- 
> 2.7.4

-- 
viresh

Re: [PATCH] sched/rt: fix call to cpufreq_update_util

[Viresh Kumar]

On 16-05-18, 20:18, Vincent Guittot wrote:
> With commit 8f111bc357aa ("cpufreq/schedutil: Rewrite CPUFREQ_RT support")
> schedutil governor uses rq->rt.rt_nr_running to detect whether a RT task is
> currently running on the CPU and to set frequency to max if necessary.
> cpufreq_update_util() is called in enqueue/dequeue_top_rt_rq() but
> rq->rt.rt_nr_running as not been updated yet when dequeue_top_rt_rq() is
> called so schedutil still considers that a RT task is running when the
> last task is dequeued. The update of rq->rt.rt_nr_running happens later
> in dequeue_rt_stack()
> 
> Fixes: 8f111bc357aa ('cpufreq/schedutil: Rewrite CPUFREQ_RT support')
> Cc:  # v4.16+
> Signed-off-by: Vincent Guittot 
> ---
>  kernel/sched/rt.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> index 7aef6b4..6e74d3d 100644
> --- a/kernel/sched/rt.c
> +++ b/kernel/sched/rt.c
> @@ -1001,8 +1001,6 @@ dequeue_top_rt_rq(struct rt_rq *rt_rq)
>   sub_nr_running(rq, rt_rq->rt_nr_running);
>   rt_rq->rt_queued = 0;
>  

Remove this blank line as well ?

> - /* Kick cpufreq (see the comment in kernel/sched/sched.h). */
> - cpufreq_update_util(rq, 0);
>  }
>  
>  static void
> @@ -1288,6 +1286,9 @@ static void dequeue_rt_stack(struct sched_rt_entity 
> *rt_se, unsigned int flags)
>   if (on_rt_rq(rt_se))
>   __dequeue_rt_entity(rt_se, flags);
>   }
> +
> + /* Kick cpufreq (see the comment in kernel/sched/sched.h). */
> + cpufreq_update_util(rq_of_rt_rq(rt_rq_of_se(back)), 0);
>  }
>  
>  static void enqueue_rt_entity(struct sched_rt_entity *rt_se, unsigned int 
> flags)
> -- 
> 2.7.4

-- 
viresh

[PATCH] sched/rt: fix call to cpufreq_update_util

[Vincent Guittot]

With commit 8f111bc357aa ("cpufreq/schedutil: Rewrite CPUFREQ_RT support")
schedutil governor uses rq->rt.rt_nr_running to detect whether a RT task is
currently running on the CPU and to set frequency to max if necessary.
cpufreq_update_util() is called in enqueue/dequeue_top_rt_rq() but
rq->rt.rt_nr_running as not been updated yet when dequeue_top_rt_rq() is
called so schedutil still considers that a RT task is running when the
last task is dequeued. The update of rq->rt.rt_nr_running happens later
in dequeue_rt_stack()

Fixes: 8f111bc357aa ('cpufreq/schedutil: Rewrite CPUFREQ_RT support')
Cc:  # v4.16+
Signed-off-by: Vincent Guittot 
---
 kernel/sched/rt.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 7aef6b4..6e74d3d 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -1001,8 +1001,6 @@ dequeue_top_rt_rq(struct rt_rq *rt_rq)
sub_nr_running(rq, rt_rq->rt_nr_running);
rt_rq->rt_queued = 0;
 
-   /* Kick cpufreq (see the comment in kernel/sched/sched.h). */
-   cpufreq_update_util(rq, 0);
 }
 
 static void
@@ -1288,6 +1286,9 @@ static void dequeue_rt_stack(struct sched_rt_entity 
*rt_se, unsigned int flags)
if (on_rt_rq(rt_se))
__dequeue_rt_entity(rt_se, flags);
}
+
+   /* Kick cpufreq (see the comment in kernel/sched/sched.h). */
+   cpufreq_update_util(rq_of_rt_rq(rt_rq_of_se(back)), 0);
 }
 
 static void enqueue_rt_entity(struct sched_rt_entity *rt_se, unsigned int 
flags)
-- 
2.7.4

[PATCH] sched/rt: fix call to cpufreq_update_util

[Vincent Guittot]

With commit 8f111bc357aa ("cpufreq/schedutil: Rewrite CPUFREQ_RT support")
schedutil governor uses rq->rt.rt_nr_running to detect whether a RT task is
currently running on the CPU and to set frequency to max if necessary.
cpufreq_update_util() is called in enqueue/dequeue_top_rt_rq() but
rq->rt.rt_nr_running as not been updated yet when dequeue_top_rt_rq() is
called so schedutil still considers that a RT task is running when the
last task is dequeued. The update of rq->rt.rt_nr_running happens later
in dequeue_rt_stack()

Fixes: 8f111bc357aa ('cpufreq/schedutil: Rewrite CPUFREQ_RT support')
Cc:  # v4.16+
Signed-off-by: Vincent Guittot 
---
 kernel/sched/rt.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 7aef6b4..6e74d3d 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -1001,8 +1001,6 @@ dequeue_top_rt_rq(struct rt_rq *rt_rq)
sub_nr_running(rq, rt_rq->rt_nr_running);
rt_rq->rt_queued = 0;
 
-   /* Kick cpufreq (see the comment in kernel/sched/sched.h). */
-   cpufreq_update_util(rq, 0);
 }
 
 static void
@@ -1288,6 +1286,9 @@ static void dequeue_rt_stack(struct sched_rt_entity 
*rt_se, unsigned int flags)
if (on_rt_rq(rt_se))
__dequeue_rt_entity(rt_se, flags);
}
+
+   /* Kick cpufreq (see the comment in kernel/sched/sched.h). */
+   cpufreq_update_util(rq_of_rt_rq(rt_rq_of_se(back)), 0);
 }
 
 static void enqueue_rt_entity(struct sched_rt_entity *rt_se, unsigned int 
flags)
-- 
2.7.4