Messages in this thread | | | Date | Thu, 29 Sep 2022 21:19:28 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH v2] sched/fair: limit sched slice duration |
| |
On Thu, Sep 29, 2022 at 07:15:42PM +0200, Vincent Guittot wrote: > On Thu, 29 Sept 2022 at 18:14, Peter Zijlstra <peterz@infradead.org> wrote:
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > > index 5ffec4370602..2b218167fadf 100644 > > --- a/kernel/sched/fair.c > > +++ b/kernel/sched/fair.c > > @@ -4575,17 +4575,33 @@ dequeue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags) > > } > > > > /* > > - * Preempt the current task with a newly woken task if needed: > > + * Tick driven preemption; preempt the task if it has ran long enough. > > + * Allows other tasks to have a go. > > */ > > static void > > check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr) > > { > > - unsigned long ideal_runtime, delta_exec; > > struct sched_entity *se; > > - s64 delta; > > + s64 delta, delta_exec; > > + u64 ideal_runtime; > > > > - ideal_runtime = sched_slice(cfs_rq, curr); > > + /* How long has this task been on the CPU for [walltime]. */ > > delta_exec = curr->sum_exec_runtime - curr->prev_sum_exec_runtime; > > + > > + /* > > + * Ensure that a task that missed wakeup preemption by a > > + * narrow margin doesn't have to wait for a full slice. > > + * This also mitigates buddy induced latencies under load. > > + */ > > + if (delta_exec < sysctl_sched_min_granularity) > > + return; > > ideal_runtime can be lower than sysctl_sched_min_granularity. It can > be as low as sysctl_sched_idle_min_granularity for idle task. In this > case, we want to resched even if(delta_exec < > sysctl_sched_min_granularity). That's why the 1st test was still done > before
Duh, indeed.
> > + > > + /* > > + * When many tasks blow up the sched_period; it is possible that > > + * sched_slice() reports unusually large results (when many tasks are > > + * very light for example). Therefore impose a maximum. > > + */ > > + ideal_runtime = min_t(u64, sched_slice(cfs_rq, curr), sysctl_sched_latency); > > I didn't cap ideal_runtime before this test because we have situations > where large ideal_runtime is ok: If there is only one normal thread > with hundreds of idle threads as an example: Is it acceptable to > trigger a useless resched in this case ? That's why I have computed > the virtual time generated by the capped version of ideal_runtime.
Yeah; I think that should be fine. It's an edge case, and sched_latency is fairly large already.
> > se = __pick_first_entity(cfs_rq); > > delta = curr->vruntime - se->vruntime; > > - > > if (delta < 0) > > return; > > > > + /* > > + * Compare @delta [vtime] to @ideal_runtime [walltime]. This means that > > + * heavy tasks (for which vtime goes slower) get relatively more time > > + * before preemption, while light tasks (for which vtime goes faster) > > + * get relatively less time. IOW, heavy task get to run longer. > > + */ > > After your comment on v1, I looked more deeply on this and the > comparison of [vtime] with [walltime] can create a large unfairness. > vtime of nice-20 increases by ~250us for 24ms of walltime which means > that the nice-20 will have to run for a long time before reaching this > walltime delta (assuming the vruntime were similar at the beg)
As I wrote, strictly speaking we should do without this. The entire vtime thing is a band-aid.
I'm sure I've tried taking it out at least once; but sadly I seem to have forgotten everything relevant :-( That is, I can't tell you why this code exists.
| |