Re: [PATCH v3] ata: libata: Document when host->eh_mutex should be held

[Damien Le Moal <dlemoal@kernel.org>]

On 5/28/26 05:25, Bart Van Assche wrote:
> Annotate the following functions with __must_hold(&host->eh_mutex):
>  * All ata_port_operations.error_handler() implementations.
>  * ata_eh_reset() and ata_eh_recover() because these functions call
>    ata_eh_release() and ata_eh_acquire().
>  * All callers of ata_eh_reset() and ata_eh_recover().
> 
> Enable Clang's context analysis. This will cause the build to fail if
> e.g. a locking bug would be introduced in an error path. This patch
> should not affect the generated assembler code.
> 
> Note: although the Linux kernel documentation specifies 22 as minimal
> version for Clang for context analysis support, annotating function
> pointers is a Clang 23 feature. As one can see here, a patch has been
> queued that fixes the kernel documentation:
> https://lore.kernel.org/all/177926568868.711.3058599932884307249.tip-bot2@tip-bot2/
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>

Bart,

Thanks for doing this.
I only have a couple of nits below. With that fixed, please add:

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

> +		/* Tell the compiler that link->ap == ap. */
> +		lockdep_assert_held(&link->ap->host->eh_mutex);

Please drop this comment in the 2 places you have it. The comment is confusing
because it does not describe at all what lockdep_assert_held() checks. No
comment is fine. lockdep_assert_held() as a name is clear enough.

> +	/* Tell the compiler that ap->link.ap == ap. */
> +	lockdep_assert_held(&ap->link.ap->host->eh_mutex);

Same thing here.

-- 
Damien Le Moal
Western Digital Research