Messages in this thread | | | Date | Tue, 11 Jul 2023 09:10:24 -0400 | From | Phil Auld <> | Subject | Re: [PATCH v5 2/2] Sched/fair: Block nohz tick_stop when cfs bandwidth in use |
| |
On Mon, Jul 10, 2023 at 04:54:58PM -0700 Benjamin Segall wrote: > Phil Auld <pauld@redhat.com> writes: > > > CFS bandwidth limits and NOHZ full don't play well together. Tasks > > can easily run well past their quotas before a remote tick does > > accounting. This leads to long, multi-period stalls before such > > tasks can run again. Currently, when presented with these conflicting > > requirements the scheduler is favoring nohz_full and letting the tick > > be stopped. However, nohz tick stopping is already best-effort, there > > are a number of conditions that can prevent it, whereas cfs runtime > > bandwidth is expected to be enforced. > > > > Make the scheduler favor bandwidth over stopping the tick by setting > > TICK_DEP_BIT_SCHED when the only running task is a cfs task with > > runtime limit enabled. We use cfs_b->hierarchical_quota to > > determine if the task requires the tick. > > > > Add check in pick_next_task_fair() as well since that is where > > we have a handle on the task that is actually going to be running. > > > > Add check in sched_can_stop_tick() to cover some edge cases such > > as nr_running going from 2->1 and the 1 remains the running task. > > > > Add sched_feat HZ_BW (off by default) to control the tick_stop > > behavior. > > > > Signed-off-by: Phil Auld <pauld@redhat.com> > > Cc: Ingo Molnar <mingo@redhat.com> > > Cc: Peter Zijlstra <peterz@infradead.org> > > Cc: Vincent Guittot <vincent.guittot@linaro.org> > > Cc: Juri Lelli <juri.lelli@redhat.com> > > Cc: Dietmar Eggemann <dietmar.eggemann@arm.com> > > Cc: Valentin Schneider <vschneid@redhat.com> > > Cc: Ben Segall <bsegall@google.com> > > Cc: Frederic Weisbecker <frederic@kernel.org> > > --- > > kernel/sched/core.c | 12 ++++++++++ > > kernel/sched/fair.c | 49 +++++++++++++++++++++++++++++++++++++++++ > > kernel/sched/features.h | 2 ++ > > kernel/sched/sched.h | 1 + > > 4 files changed, 64 insertions(+) > > > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > > index 1b214e10c25d..4b8534abdf4f 100644 > > --- a/kernel/sched/core.c > > +++ b/kernel/sched/core.c > > @@ -1229,6 +1229,18 @@ bool sched_can_stop_tick(struct rq *rq) > > if (rq->nr_running > 1) > > return false; > > > > + /* > > + * If there is one task and it has CFS runtime bandwidth constraints > > + * and it's on the cpu now we don't want to stop the tick. > > + * This check prevents clearing the bit if a newly enqueued task here is > > + * dequeued by migrating while the constrained task continues to run. > > + * E.g. going from 2->1 without going through pick_next_task(). > > + */ > > + if (sched_feat(HZ_BW) && rq->nr_running == 1 && task_on_rq_queued(rq->curr)) { > > + if (cfs_task_bw_constrained(rq->curr)) > > + return false; > > + } > > + > > I think we still need the fair_sched_class check with the bit being on > cfs_rq/tg rather than task. >
Is there a way a non-fair_sched_class task will be in a cfs_rq with cfs_rq->runtime_enabled and/or cfs_b->hierarchical_quota set to non RUNTIME_INF? I suppose if they are stale and it's had its class changed?
That makes the condition pretty ugly but I can add that back if needed.
Thanks, Phil
--
| |