Re: [RFC][PATCH v14 2/7] locking/mutex: Rework task_struct::blocked_on

Peter Zijlstra
2024-12-16T17:07:42Z

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)
> {