Messages in this thread | | | Date | Tue, 5 Sep 2023 17:16:25 -0400 | Subject | Re: [RFC PATCH 1/2] sched: Rate limit migrations to 1 per 2ms per task | From | Mathieu Desnoyers <> |
| |
On 9/5/23 16:28, Tim Chen wrote: > On Tue, 2023-09-05 at 13:11 -0400, Mathieu Desnoyers wrote: >> Rate limit migrations to 1 migration per 2 milliseconds per task. On a >> kernel with EEVDF scheduler (commit b97d64c722598ffed42ece814a2cb791336c6679), >> this speeds up hackbench from 62s to 45s on AMD EPYC 192-core (over 2 sockets). >> >> >> >> diff --git a/kernel/sched/core.c b/kernel/sched/core.c >> index 479db611f46e..0d294fce261d 100644 >> --- a/kernel/sched/core.c >> +++ b/kernel/sched/core.c >> @@ -4510,6 +4510,7 @@ static void __sched_fork(unsigned long clone_flags, struct task_struct *p) >> p->se.vruntime = 0; >> p->se.vlag = 0; >> p->se.slice = sysctl_sched_base_slice; >> + p->se.next_migration_time = 0; > > It seems like the next_migration_time should be initialized to the current time, > in case the system run for a long time and clock wrap around could cause problem.
next_migration_time is a u64, which should "never" overflow. Other scheduler code comparing with sched_clock() don't appear to care about u64 overflow. Sampling the next_migration_time on fork could delay migrations for a 2ms window after process creation, which I don't think is something we want. Or if we do want this behavior, it should be validated with benchmarks beforehand.
> >> INIT_LIST_HEAD(&p->se.group_node); >> >> #ifdef CONFIG_FAIR_GROUP_SCHED >> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c >> index d92da2d78774..24ac69913005 100644 >> --- a/kernel/sched/fair.c >> +++ b/kernel/sched/fair.c >> @@ -960,6 +960,14 @@ int sched_update_scaling(void) >> >> static void clear_buddies(struct cfs_rq *cfs_rq, struct sched_entity *se); >> >> +static bool should_migrate_task(struct task_struct *p, int prev_cpu) >> +{ >> + /* Rate limit task migration. */ >> + if (sched_clock_cpu(prev_cpu) < p->se.next_migration_time) > > Should we use time_before(sched_clock_cpu(prev_cpu), p->se.next_migration_time) ?
No, because time_before expects unsigned long parameters, and sched_clock_cpu() and next_migration_time are u64.
Thanks,
Mathieu
> >> + return false; >> + return true; >> +} >> + > > Thanks. > > Tim
-- Mathieu Desnoyers EfficiOS Inc. https://www.efficios.com
| |