Messages in this thread | | | From | Vincent Guittot <> | Date | Tue, 7 Jun 2022 12:19:14 +0200 | Subject | Re: [PATCH v9 2/7] sched/fair: Decay task PELT values during wakeup migration |
| |
On Tue, 7 Jun 2022 at 12:03, Vincent Donnefort <vdonnefort@google.com> wrote: > > [...] > > > > > > > > > > diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h > > > > > index bf4a0ec98678..97bc26e5c8af 100644 > > > > > --- a/kernel/sched/sched.h > > > > > +++ b/kernel/sched/sched.h > > > > > @@ -648,6 +648,10 @@ struct cfs_rq { > > > > > int runtime_enabled; > > > > > s64 runtime_remaining; > > > > > > > > > > + u64 throttled_pelt_idle; > > > > > +#ifndef CONFIG_64BIT > > > > > + u64 throttled_pelt_idle_copy; > > > > > +#endif > > > > > u64 throttled_clock; > > > > > u64 throttled_clock_pelt; > > > > > u64 throttled_clock_pelt_time; > > > > > @@ -1020,6 +1024,12 @@ struct rq { > > > > > u64 clock_task ____cacheline_aligned; > > > > > u64 clock_pelt; > > > > > unsigned long lost_idle_time; > > > > > + u64 clock_pelt_idle; > > > > > + u64 enter_idle; > > > > > +#ifndef CONFIG_64BIT > > > > > + u64 clock_pelt_idle_copy; > > > > > + u64 enter_idle_copy; > > > > > +#endif > > > > > > > > > > atomic_t nr_iowait; > > > > > > > > `throttled_pelt_idle`, `clock_pelt_idle` and `enter_idle` are clock > > > > snapshots when cfs_rq resp. rq go idle. But the naming does not really > > > > show this relation. And this makes reading those equations rather difficult. > > > > > > > > What about something like `throttled_clock_pelt_time_enter_idle`, > > > > `clock_pelt_enter_idle`, `clock_enter_idle`? Especially the first one is > > > > too long but something which shows that those are clock snapshots when > > > > enter idle would IMHO augment readability in migrate_se_pelt_lag(). > > > > > > What if I drop the "enter"? > > > > > > clock_idle; > > > clock_pelt_idle; > > > throttled_clock_pelt_time_idle; > > > > and you can even remove the _time for throttled_clock_pelt_idle > > > > Hum, "throttled_clock_pelt" already exists, while what we really snapshot is > "throttled_clock_pelt_time".
What is snapshot is throttled_clock_pelt when entering idle so my proposal of "throttled_clock_pelt_idle" throttled_clock_pelt_idle doesn't exist and reflect that it's throttled_clock_pelt when cfs_rq enter idle just like clock_pelt_idle reflect clock_pelt when entering idle
| |