[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251204112947.GK2528459@noisy.programming.kicks-ass.net>
Date: Thu, 4 Dec 2025 12:29:47 +0100
From: Peter Zijlstra <peterz@...radead.org>
To: Vincent Guittot <vincent.guittot@...aro.org>
Cc: mingo@...hat.com, juri.lelli@...hat.com, dietmar.eggemann@....com,
rostedt@...dmis.org, bsegall@...gle.com, mgorman@...e.de,
vschneid@...hat.com, linux-kernel@...r.kernel.org,
pierre.gondois@....com, kprateek.nayak@....com, qyousef@...alina.io,
hongyan.xia2@....com, christian.loehle@....com,
luis.machado@....com
Subject: Re: [PATCH 4/6 v8] sched/fair: Add push task mechanism for fair
On Tue, Dec 02, 2025 at 07:12:40PM +0100, Vincent Guittot wrote:
> +/*
> + * See if the non running fair tasks on this rq can be sent on other CPUs
> + * that fits better with their profile.
> + */
> +static bool push_fair_task(struct rq *rq)
> +{
> + struct task_struct *next_task;
> + int prev_cpu, new_cpu;
> + struct rq *new_rq;
> +
> + next_task = pick_next_pushable_fair_task(rq);
> + if (!next_task)
> + return false;
> +
> + if (is_migration_disabled(next_task))
> + return true;
> +
> + /* We might release rq lock */
> + get_task_struct(next_task);
> +
> + prev_cpu = rq->cpu;
> +
> + new_cpu = select_task_rq_fair(next_task, prev_cpu, 0);
> +
> + if (new_cpu == prev_cpu)
> + goto out;
> +
> + new_rq = cpu_rq(new_cpu);
> +
> + if (double_lock_balance(rq, new_rq)) {
> + /* The task has already migrated in between */
> + if (task_cpu(next_task) != rq->cpu) {
> + double_unlock_balance(rq, new_rq);
> + goto out;
> + }
> +
> + deactivate_task(rq, next_task, 0);
> + set_task_cpu(next_task, new_cpu);
> + activate_task(new_rq, next_task, 0);
> +
> + resched_curr(new_rq);
> +
> + double_unlock_balance(rq, new_rq);
> + }
Why not use move_queued_task() ?
> +
> +out:
> + put_task_struct(next_task);
> +
> + return true;
> +}
> +
> +static void push_fair_tasks(struct rq *rq)
> +{
> + /* push_fair_task() will return true if it moved a fair task */
> + while (push_fair_task(rq))
> + ;
If we're going to be looping on that, why not also loop in
pick_next_pushable_task() like:
list_for_each_entity(p, &rq->cfs.pushable_tasks, pushable_tasks) {
if (!is_migration_disabled(p)) {
list_del(&p->pushable_tasks);
return p;
}
}
return NULL;
Because as is, I think you'll fail the moment there's a
migrate_disable() tasks at the head of things.
Powered by blists - more mailing lists