Re: [RESEND][PATCH v21 1/6] locking: Add task::blocked_lock to serialize blocked_on state

[K Prateek Nayak <kprateek.nayak@amd.com>]

Hello John,

On 9/4/2025 5:51 AM, John Stultz wrote:
> @@ -693,25 +697,30 @@ __mutex_lock_common(struct mutex *lock, unsigned int state, unsigned int subclas
>  			break;
>  
>  		if (first) {
> -			trace_contention_begin(lock, LCB_F_MUTEX | LCB_F_SPIN);
> +			bool opt_acquired;
> +
>  			/*
>  			 * mutex_optimistic_spin() can call schedule(), so
> -			 * clear blocked on so we don't become unselectable
> +			 * we need to release these locks before calling it,
> +			 * and clear blocked on so we don't become unselectable
>  			 * to run.
>  			 */
> -			clear_task_blocked_on(current, lock);
> -			if (mutex_optimistic_spin(lock, ww_ctx, &waiter))
> +			__clear_task_blocked_on(current, lock);
> +			raw_spin_unlock(&current->blocked_lock);
> +			raw_spin_unlock_irqrestore(&lock->wait_lock, flags);
> +			trace_contention_begin(lock, LCB_F_MUTEX | LCB_F_SPIN);
> +			opt_acquired = mutex_optimistic_spin(lock, ww_ctx, &waiter);
> +			raw_spin_lock_irqsave(&lock->wait_lock, flags);
> +			raw_spin_lock(&current->blocked_lock);
> +			__set_task_blocked_on(current, lock);
> +			if (opt_acquired)
>  				break;

nit. Is there any reason for setting the blocked state before the break
other than for symmetry? The blocked_lock is held and we'll soon clear
it after the break anyways so does it have any other subtle purpose?

Also super silly but that above block is too dense. Can we add some
spaces in between, groping the relevant ops, to make it easier on the
eyes :)

Apart from those nit picks, I don't see anything out of the place.
Feel free to include:

Reviewed-by: K Prateek Nayak <kprateek.nayak@amd.com>

I'm still lagging behind on testing (Sorry about that!) but I should
have the results for this by Monday.

-- 
Thanks and Regards,
Prateek

> -			set_task_blocked_on(current, lock);
>  			trace_contention_begin(lock, LCB_F_MUTEX);
>  		}
> -
> -		raw_spin_lock_irqsave(&lock->wait_lock, flags);
>  	}
> -	raw_spin_lock_irqsave(&lock->wait_lock, flags);
> -acquired:
>  	__clear_task_blocked_on(current, lock);
>  	__set_current_state(TASK_RUNNING);
> +	raw_spin_unlock(&current->blocked_lock);
>  
>  	if (ww_ctx) {
>  		/*