Re: [PATCH v2 1/4] scsi: ufs: core: Add a helper for clearing hba->outstanding_reqs

[Bart Van Assche <bvanassche@acm.org>]

On 10/17/26 1:44 AM, Stanley Jhu wrote:
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index bcc2512bf34b..71f4b9d0a850 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -6052,6 +6052,37 @@ static void ufshcd_clear_polled(struct ufs_hba *hba,
>   	}
>   }
>   
> +static unsigned long __ufshcd_clear_outstanding_reqs(struct ufs_hba *hba,
> +						     unsigned long mask,
> +						     bool from_irq)
> +{
> +	unsigned long completed_reqs = mask & hba->outstanding_reqs;
> +
> +	lockdep_assert_held(&hba->outstanding_lock);
> +	WARN_ONCE(completed_reqs & ~hba->outstanding_reqs,
> +		  "completed: %#lx; outstanding: %#lx\n", completed_reqs,
> +		  hba->outstanding_reqs);
> +	if (from_irq) {
> +		/* Do not complete polled requests from interrupt context. */
> +		ufshcd_clear_polled(hba, &completed_reqs);
> +	}
> +	hba->outstanding_reqs &= ~completed_reqs;
> +
> +	return completed_reqs;
> +}

Please use __must_hold() instead of lockdep_assert_held(). __must_hold()
is evaluated at compile time (by Clang) while lockdep_assert_held() is
evaluated at run-time. If you don't want to depend on Clang, feel free
to use both.

> +static unsigned long ufshcd_clear_outstanding_reqs(struct ufs_hba *hba,
> +						   unsigned long mask)
> +{
> +	unsigned long completed_reqs, flags;
> +
> +	spin_lock_irqsave(&hba->outstanding_lock, flags);
> +	completed_reqs = __ufshcd_clear_outstanding_reqs(hba, mask, false);
> +	spin_unlock_irqrestore(&hba->outstanding_lock, flags);
> +
> +	return completed_reqs;
> +}

The seven-line body of the above function can be reduced to two lines by
using guard():

static unsigned long ufshcd_clear_outstanding_reqs(struct ufs_hba *hba,
						   unsigned long mask)
{
	guard(spinlock_irqsave)(&hba->outstanding_lock);
	return __ufshcd_clear_outstanding_reqs(hba, mask, false);
}

Otherwise this patch looks good to me.

Thanks,

Bart.