[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230622145352.GB113759@maniforge>
Date: Thu, 22 Jun 2023 09:53:52 -0500
From: David Vernet <void@...ifault.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: linux-kernel@...r.kernel.org, mingo@...hat.com,
juri.lelli@...hat.com, vincent.guittot@...aro.org,
rostedt@...dmis.org, dietmar.eggemann@....com, bsegall@...gle.com,
mgorman@...e.de, bristot@...hat.com, vschneid@...hat.com,
joshdon@...gle.com, roman.gushchin@...ux.dev, tj@...nel.org,
kernel-team@...a.com
Subject: Re: [RFC PATCH 2/3] sched/fair: Add SWQUEUE sched feature and
skeleton calls
On Wed, Jun 21, 2023 at 02:49:33PM +0200, Peter Zijlstra wrote:
> On Tue, Jun 13, 2023 at 12:20:03AM -0500, David Vernet wrote:
>
> I can't help but read this thing as software-queue :/ Can we please pick
> a better name?
Yes, I'll think of a better one. Suggestions welcome if you have a
preference.
> > @@ -6368,6 +6390,9 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> > if (!task_new)
> > update_overutilized_status(rq);
> >
> > + if (sched_feat(SWQUEUE))
> > + swqueue_enqueue(rq, p, flags);
> > +
> > enqueue_throttle:
> > assert_list_leaf_cfs_rq(rq);
> >
> > @@ -6449,6 +6474,9 @@ static void dequeue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> > dequeue_throttle:
> > util_est_update(&rq->cfs, p, task_sleep);
> > hrtick_update(rq);
> > +
> > + if (sched_feat(SWQUEUE))
> > + swqueue_remove_task(p);
> > }
> >
> > #ifdef CONFIG_SMP
>
> _enqueue() should obviously be complemented by _dequeue(). This naming
> is offensive :-)
Ack
> > @@ -8155,12 +8183,18 @@ done: __maybe_unused;
> >
> > update_misfit_status(p, rq);
> >
> > + if (sched_feat(SWQUEUE))
> > + swqueue_remove_task(p);
> > +
> > return p;
> >
> > idle:
> > if (!rf)
> > return NULL;
> >
> > + if (sched_feat(SWQUEUE) && swqueue_pick_next_task(rq, rf))
> > + return RETRY_TASK;
> > +
> > new_tasks = newidle_balance(rq, rf);
> >
> > /*
>
> That's either not correct or insufficient or both.
>
> It fails to consider the whole core-scheduling mess. But it also fails
> to consider the regular (non optimized) pick case that should do newidle
> through put_prev_task_balance() -> balance_fair().
>
> I think placing the pick call in newidle_balance() itself is the
> simplest solution.
Yep, not sure where I went off the rails here -- the pick call clearly
belongs in newidle_balance(). I'll also make sure we handle core sched
correctly as well. Thanks for pointing those out.
Powered by blists - more mailing lists