Re: [PATCH 1/4 v4] sched/pelt: Move pelt related code in a dedicated file

[Dietmar Eggemann]

On 04/15/2018 02:16 PM, Vincent Guittot wrote:

On 15 April 2018 at 13:58, Dietmar Eggemann  wrote:

On 03/16/2018 12:25 PM, Vincent Guittot wrote:


We want to track rt_rq's utilization as a part of the estimation of the
whole rq's utilization. This is necessary because rt tasks can steal
utilization to cfs tasks and make them lighter than they are.
As we want to use the same load tracking mecanism for both and prevent
useless dependency between cfs and rt code, pelt code is moved in a
dedicated file.



This would mean that we introduce function calls into the cfs scheduler
fast-path, something we avoided so far (e.g. the cpu and frequency
invariance hooks). Are we OK with that?

Quentin mentioned this already during v3 review back in December.


Yes and I hadn't seen any differences in the code size with the patch
which should have been the case if inline function where replaced by
function call


I see a diff (e.g. for arm64 defconfig):

6d626e0aaf91 - (HEAD -> tip/sched/core_rt_rq_util_tracking) sched/nohz: 
monitor rt utilization (2018-04-15 Vincent Guittot)
3111c6206f0c - cpufreq/schedutil: add rt utilization tracking 
(2018-04-15 Vincent Guittot)
62e103d266ed - sched/rt: add rt_rq utilization tracking (2018-04-15 
Vincent Guittot)
8f78fef6b1a2 - sched/pelt: Move pelt related code in a dedicated file 
(2018-04-15 Vincent Guittot)
31e77c93e432 - (tip/sched/core_rt_rq_util_tracking_base) sched/fair: 
Update blocked load when newly idle (2018-03-09 Vincent Guittot)


deggeman-mac:/opt/git/kernel_org:tip/sched/core_rt_rq_util_tracking$ 
size vmlinux

   textdata bss dec hex filename
112868566154896  410296 178520481106690 vmlinux

versus:

31e77c93e432 - (HEAD -> tip/sched/core_rt_rq_util_tracking_base) 
sched/fair: Update blocked load when ne$

ly idle (2018-03-09 Vincent Guittot)

deggeman-mac:/opt/git/kernel_org:tip/sched/core_rt_rq_util_tracking_base$ 
size vmlinux
   textdata bss dec hex filename 


112950486154896  410296 178602401108690 vmlinux

So I assume that in kernel/sched/fair.o:

U __update_load_avg_blocked_se
U __update_load_avg_cfs_rq
U __update_load_avg_se

are function calls now.

Re: [PATCH 1/4 v4] sched/pelt: Move pelt related code in a dedicated file

[Dietmar Eggemann]

On 04/15/2018 02:16 PM, Vincent Guittot wrote:

On 15 April 2018 at 13:58, Dietmar Eggemann  wrote:

On 03/16/2018 12:25 PM, Vincent Guittot wrote:


We want to track rt_rq's utilization as a part of the estimation of the
whole rq's utilization. This is necessary because rt tasks can steal
utilization to cfs tasks and make them lighter than they are.
As we want to use the same load tracking mecanism for both and prevent
useless dependency between cfs and rt code, pelt code is moved in a
dedicated file.



This would mean that we introduce function calls into the cfs scheduler
fast-path, something we avoided so far (e.g. the cpu and frequency
invariance hooks). Are we OK with that?

Quentin mentioned this already during v3 review back in December.


Yes and I hadn't seen any differences in the code size with the patch
which should have been the case if inline function where replaced by
function call


I see a diff (e.g. for arm64 defconfig):

6d626e0aaf91 - (HEAD -> tip/sched/core_rt_rq_util_tracking) sched/nohz: 
monitor rt utilization (2018-04-15 Vincent Guittot)
3111c6206f0c - cpufreq/schedutil: add rt utilization tracking 
(2018-04-15 Vincent Guittot)
62e103d266ed - sched/rt: add rt_rq utilization tracking (2018-04-15 
Vincent Guittot)
8f78fef6b1a2 - sched/pelt: Move pelt related code in a dedicated file 
(2018-04-15 Vincent Guittot)
31e77c93e432 - (tip/sched/core_rt_rq_util_tracking_base) sched/fair: 
Update blocked load when newly idle (2018-03-09 Vincent Guittot)


deggeman-mac:/opt/git/kernel_org:tip/sched/core_rt_rq_util_tracking$ 
size vmlinux

   textdata bss dec hex filename
112868566154896  410296 178520481106690 vmlinux

versus:

31e77c93e432 - (HEAD -> tip/sched/core_rt_rq_util_tracking_base) 
sched/fair: Update blocked load when ne$

ly idle (2018-03-09 Vincent Guittot)

deggeman-mac:/opt/git/kernel_org:tip/sched/core_rt_rq_util_tracking_base$ 
size vmlinux
   textdata bss dec hex filename 


112950486154896  410296 178602401108690 vmlinux

So I assume that in kernel/sched/fair.o:

U __update_load_avg_blocked_se
U __update_load_avg_cfs_rq
U __update_load_avg_se

are function calls now.

Re: [PATCH 1/4 v4] sched/pelt: Move pelt related code in a dedicated file

[Vincent Guittot]

On 15 April 2018 at 13:58, Dietmar Eggemann  wrote:
> On 03/16/2018 12:25 PM, Vincent Guittot wrote:
>>
>> We want to track rt_rq's utilization as a part of the estimation of the
>> whole rq's utilization. This is necessary because rt tasks can steal
>> utilization to cfs tasks and make them lighter than they are.
>> As we want to use the same load tracking mecanism for both and prevent
>> useless dependency between cfs and rt code, pelt code is moved in a
>> dedicated file.
>
>
> This would mean that we introduce function calls into the cfs scheduler
> fast-path, something we avoided so far (e.g. the cpu and frequency
> invariance hooks). Are we OK with that?
>
> Quentin mentioned this already during v3 review back in December.

Yes and I hadn't seen any differences in the code size with the patch
which should have been the case if inline function where replaced by
function call


>
> [...]

Re: [PATCH 1/4 v4] sched/pelt: Move pelt related code in a dedicated file

[Vincent Guittot]

On 15 April 2018 at 13:58, Dietmar Eggemann  wrote:
> On 03/16/2018 12:25 PM, Vincent Guittot wrote:
>>
>> We want to track rt_rq's utilization as a part of the estimation of the
>> whole rq's utilization. This is necessary because rt tasks can steal
>> utilization to cfs tasks and make them lighter than they are.
>> As we want to use the same load tracking mecanism for both and prevent
>> useless dependency between cfs and rt code, pelt code is moved in a
>> dedicated file.
>
>
> This would mean that we introduce function calls into the cfs scheduler
> fast-path, something we avoided so far (e.g. the cpu and frequency
> invariance hooks). Are we OK with that?
>
> Quentin mentioned this already during v3 review back in December.

Yes and I hadn't seen any differences in the code size with the patch
which should have been the case if inline function where replaced by
function call


>
> [...]

Re: [PATCH 1/4 v4] sched/pelt: Move pelt related code in a dedicated file

[Dietmar Eggemann]

On 03/16/2018 12:25 PM, Vincent Guittot wrote:

We want to track rt_rq's utilization as a part of the estimation of the
whole rq's utilization. This is necessary because rt tasks can steal
utilization to cfs tasks and make them lighter than they are.
As we want to use the same load tracking mecanism for both and prevent
useless dependency between cfs and rt code, pelt code is moved in a
dedicated file.


This would mean that we introduce function calls into the cfs scheduler 
fast-path, something we avoided so far (e.g. the cpu and frequency 
invariance hooks). Are we OK with that?


Quentin mentioned this already during v3 review back in December.

[...]

Re: [PATCH 1/4 v4] sched/pelt: Move pelt related code in a dedicated file

[Dietmar Eggemann]

On 03/16/2018 12:25 PM, Vincent Guittot wrote:

[...]


diff --git a/kernel/sched/pelt.h b/kernel/sched/pelt.h
new file mode 100644
index 000..c312d8c
--- /dev/null
+++ b/kernel/sched/pelt.h
@@ -0,0 +1,17 @@
+#ifdef CONFIG_SMP
+
+int __update_load_avg_blocked_se(u64 now, int cpu, struct sched_entity *se);
+int __update_load_avg_se(u64 now, int cpu, struct cfs_rq *cfs_rq, struct 
sched_entity *se);
+int __update_load_avg_cfs_rq(u64 now, int cpu, struct cfs_rq *cfs_rq);
+
+#else
+
+static inline int
+update_cfs_rq_load_avg(u64 now, struct cfs_rq *cfs_rq)
+{
+   return 0;
+}
+
+#endif


Exporting __update_load_avg_cfs_rq() for SMP and 
update_cfs_rq_load_avg() for !SMP seems weird.


[...]

Re: [PATCH 1/4 v4] sched/pelt: Move pelt related code in a dedicated file

[Dietmar Eggemann]

On 03/16/2018 12:25 PM, Vincent Guittot wrote:

[...]


diff --git a/kernel/sched/pelt.h b/kernel/sched/pelt.h
new file mode 100644
index 000..c312d8c
--- /dev/null
+++ b/kernel/sched/pelt.h
@@ -0,0 +1,17 @@
+#ifdef CONFIG_SMP
+
+int __update_load_avg_blocked_se(u64 now, int cpu, struct sched_entity *se);
+int __update_load_avg_se(u64 now, int cpu, struct cfs_rq *cfs_rq, struct 
sched_entity *se);
+int __update_load_avg_cfs_rq(u64 now, int cpu, struct cfs_rq *cfs_rq);
+
+#else
+
+static inline int
+update_cfs_rq_load_avg(u64 now, struct cfs_rq *cfs_rq)
+{
+   return 0;
+}
+
+#endif


Exporting __update_load_avg_cfs_rq() for SMP and 
update_cfs_rq_load_avg() for !SMP seems weird.


[...]

Re: [PATCH 1/4 v4] sched/pelt: Move pelt related code in a dedicated file

[Dietmar Eggemann]

On 03/16/2018 12:25 PM, Vincent Guittot wrote:

We want to track rt_rq's utilization as a part of the estimation of the
whole rq's utilization. This is necessary because rt tasks can steal
utilization to cfs tasks and make them lighter than they are.
As we want to use the same load tracking mecanism for both and prevent
useless dependency between cfs and rt code, pelt code is moved in a
dedicated file.


This would mean that we introduce function calls into the cfs scheduler 
fast-path, something we avoided so far (e.g. the cpu and frequency 
invariance hooks). Are we OK with that?


Quentin mentioned this already during v3 review back in December.

[...]