lkml.org 
[lkml]   [2023]   [Sep]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [RFC PATCH 1/2] sched: Rate limit migrations to 1 per 2ms per task
    From
    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

    \
     
     \ /
      Last update: 2023-09-05 23:16    [W:4.216 / U:3.176 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site