[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <931db71f-a13c-48ab-a315-f04d671bdddb@amd.com>
Date: Fri, 14 Mar 2025 14:10:32 +0530
From: K Prateek Nayak <kprateek.nayak@....com>
To: John Stultz <jstultz@...gle.com>, LKML <linux-kernel@...r.kernel.org>
CC: Valentin Schneider <valentin.schneider@....com>, Joel Fernandes
<joelagnelf@...dia.com>, Qais Yousef <qyousef@...alina.io>, Ingo Molnar
<mingo@...hat.com>, Peter Zijlstra <peterz@...radead.org>, Juri Lelli
<juri.lelli@...hat.com>, Vincent Guittot <vincent.guittot@...aro.org>,
Dietmar Eggemann <dietmar.eggemann@....com>, Valentin Schneider
<vschneid@...hat.com>, Steven Rostedt <rostedt@...dmis.org>, Ben Segall
<bsegall@...gle.com>, Zimuzo Ezeozue <zezeozue@...gle.com>, Mel Gorman
<mgorman@...e.de>, Will Deacon <will@...nel.org>, Waiman Long
<longman@...hat.com>, Boqun Feng <boqun.feng@...il.com>, "Paul E. McKenney"
<paulmck@...nel.org>, Metin Kaya <Metin.Kaya@....com>, Xuewen Yan
<xuewen.yan94@...il.com>, Thomas Gleixner <tglx@...utronix.de>, Daniel
Lezcano <daniel.lezcano@...aro.org>, Suleiman Souhlal <suleiman@...gle.com>,
<kernel-team@...roid.com>, Connor O'Brien <connoro@...gle.com>
Subject: Re: [RFC PATCH v15 6/7] sched: Fix proxy/current (push,pull)ability
Hello John,
On 3/13/2025 3:41 AM, John Stultz wrote:
[..snip..]
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index b4f7b14f62a24..3596244f613f8 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -6722,6 +6722,23 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
> }
> #endif /* SCHED_PROXY_EXEC */
>
> +static inline void proxy_tag_curr(struct rq *rq, struct task_struct *owner)
> +{
> + if (!sched_proxy_exec())
> + return;
> + /*
> + * pick_next_task() calls set_next_task() on the chosen task
> + * at some point, which ensures it is not push/pullable.
> + * However, the chosen/donor task *and* the mutex owner form an
> + * atomic pair wrt push/pull.
> + *
> + * Make sure owner we run is not pushable. Unfortunately we can
> + * only deal with that by means of a dequeue/enqueue cycle. :-/
> + */
> + dequeue_task(rq, owner, DEQUEUE_NOCLOCK | DEQUEUE_SAVE);
> + enqueue_task(rq, owner, ENQUEUE_NOCLOCK | ENQUEUE_RESTORE);
> +}
> +
> /*
> * __schedule() is the main scheduler function.
> *
> @@ -6856,6 +6873,10 @@ static void __sched notrace __schedule(int sched_mode)
> * changes to task_struct made by pick_next_task().
> */
> RCU_INIT_POINTER(rq->curr, next);
> +
> + if (!task_current_donor(rq, next))
> + proxy_tag_curr(rq, next);
I don't see any dependency on rq->curr for task_current_donor() check.
Could this check be moved outside of the if-else block to avoid
duplicating in both places since rq_set_donor() was called just after
pick_next_task() or am I missing something?
> +
> /*
> * The membarrier system call requires each architecture
> * to have a full memory barrier after updating
> @@ -6890,6 +6911,10 @@ static void __sched notrace __schedule(int sched_mode)
> /* Also unlocks the rq: */
> rq = context_switch(rq, prev, next, &rf);
> } else {
> + /* In case next was already curr but just got blocked_donor */
> + if (!task_current_donor(rq, next))
> + proxy_tag_curr(rq, next);
> +
> rq_unpin_lock(rq, &rf);
> __balance_callbacks(rq);
> raw_spin_rq_unlock_irq(rq);
--
Thanks and Regards,
Prateek
Powered by blists - more mailing lists