Re: [PATCH v2 9/9] ata: Annotate the code that uses the host lock

[Niklas Cassel <cassel@kernel.org>]

Hello Bart,

On Thu, May 21, 2026 at 10:33:37AM -0700, Bart Van Assche wrote:
> @@ -5162,6 +5165,10 @@ void ata_qc_issue(struct ata_port *ap, struct ata_queued_cmd *qc)
>  	struct ata_link *link = qc->dev->link;
>  	u8 prot = qc->tf.protocol;
>  
> +	/* Tell the compiler that qc->ap == qc->dev->link->ap->lock == ap. */
> +	__assume_ctx_lock(qc->ap->lock);
> +	__assume_ctx_lock(qc->dev->link->ap->lock);

When I look at the definition of lockdep_assert_held(), when building with
lockdep:
https://github.com/torvalds/linux/blob/v7.1-rc5/include/linux/lockdep.h#L284-L285

When building without lockdep:
https://github.com/torvalds/linux/blob/v7.1-rc5/include/linux/lockdep.h#L392

Even in the case where we don't build with lockdep, we will call
__assume_ctx_lock(l);

So my question is, why don't we instead add:
lockdep_assert_held(qc->ap->lock);
lockdep_assert_held(qc->dev->link->ap->lock);

Right now, there seems to be very few __assume_ctx_lock() in the code base,
would it not be more consistent to add lockdep_assert_held() calls instead?


> @@ -1149,6 +1154,9 @@ static void __ata_port_freeze(struct ata_port *ap)
>   */
>  int ata_port_freeze(struct ata_port *ap)
>  {
> +	/* TO DO: fix all callers of this function that do not hold ap->lock. */

Please write this as one word, i.e. TODO:



Kind regards,
Niklas