linux-kernel - Re: [RFC][PATCH v14 2/7] locking/mutex: Rework task_struct::blocked

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241216170734.GG12500@noisy.programming.kicks-ass.net>
Date: Mon, 16 Dec 2024 18:07:34 +0100
From: Peter Zijlstra <peterz@...radead.org>
To: John Stultz <jstultz@...gle.com>
Cc: LKML <linux-kernel@...r.kernel.org>, Joel Fernandes <joelaf@...gle.com>,
	Qais Yousef <qyousef@...alina.io>, Ingo Molnar <mingo@...hat.com>,
	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>,
	K Prateek Nayak <kprateek.nayak@....com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Daniel Lezcano <daniel.lezcano@...aro.org>, kernel-team@...roid.com,
	Connor O'Brien <connoro@...gle.com>
Subject: Re: [RFC][PATCH v14 2/7] locking/mutex: Rework
 task_struct::blocked_on

On Mon, Dec 16, 2024 at 05:54:19PM +0100, Peter Zijlstra wrote:

> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 6eaffa913495..30d7371bb5c4 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -4035,6 +4035,53 @@ static inline void activate_blocked_entities(struct rq *target_rq,
>  }
>  #endif /* CONFIG_SCHED_PROXY_EXEC */
>  
> +struct task_struct *proxy_handoff(struct mutex *lock);
> +{
> +	struct task_struct *next;
> +
> +	if (!sched_proxy_exec())
> +		return NULL;
> +
> +	/*
> +	 * current->blocked_donor can't change if we can't schedule
> +	 * caller needs to do this, since its needs stabiliy of return value
> +	 */
> +	lockdep_assert_preemption_disabled();
> +	next = current->blocked_donor;
> +	if (!next)
> +		return NULL;
> +
> +	scoped_guard (task_rq_lock, next) {
> +		/*
> +		 * current->blocked_donor had better be on the same CPU as current
> +		 */
> +		WARN_ON_ONCE(scope.rq != this_rq());
> +
> +		scoped_guard (raw_spin_lock, next->blocked_lock) {
> +			/*
> +			 * WARN_ON on this? How can this happen
> +			 */
> +			if (next->blocked_on != lock)
> +				return NULL;
> +		}
> +
> +		/*
> +		 * blocked_on relation is stable, since we hold both
> +		 * next->pi_lock and it's rq->lock
> +		 *
> +		 * OK -- we have a donor, it is blocked on the lock we're about
> +		 * to release and it cannot run on this CPU -- fixies are
> +		 * required.
> +		 *
> +		 * Dequeue the task, such that ttwu() can fix up the placement thing.
> +		 */
> +		if (!is_cpu_allowed(next, cpu_of(scope.rq)))
> +			deactivate_task(scope.rq, next, DEQUEUE_SLEEP);
> +	}

It is probably better to do:

	scoped_guard (raw_spin_lock_irq, next->pi_lock) {

		int cpu = smp_processor_id();
		WARN_ON_ONCE(task_cpu(next) != cpu);

		...

		if (!is_cpu_allowed(next, cpu)) {
			struct rq_flags rf;
			struct rq *rq;
			rq = __task_rq_lock(next, &rf);
			deactivate_task(rq, next, DEQUEUE_SLEEP);
			__task_rq_unlock(rq, &rf);
		}
	}	

In order to minize the amount or rq->lock'ing.

> +
> +	return next;
> +}
> +
>  #ifdef CONFIG_SMP
>  static inline bool proxy_needs_return(struct rq *rq, struct task_struct *p)
>  {