Re: [PATCH 1/3] ata: libata-scsi: Remove lockdep_assert_held() in ata_scsi_translate()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Removing the runtime `lockdep_assert_held()` assertion weakens dynamic lock verification in favor of static analysis.
--

commit 5b34afedb186c216f09eff8d76c27e0b405885ea
Author: Niklas Cassel <cassel@kernel.org>

ata: libata-scsi: Remove lockdep_assert_held() in ata_scsi_translate()

This commit removes the lockdep_assert_held() check from
ata_scsi_translate(), stating that it suppresses context analysis by
indirectly calling __assume_ctx_lock(). The commit message notes this
will be replaced with context analysis in a follow-up commit.

> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index f44612e269a4d..c31267091662a 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -1852,8 +1852,6 @@ static int ata_scsi_translate(struct ata_device *dev, struct scsi_cmnd *cmd,
>  	struct ata_port *ap = dev->link->ap;
>  	struct ata_queued_cmd *qc;
>  
> -	lockdep_assert_held(ap->lock);
> -

[Severity: Medium]
Is it safe to completely remove this runtime assertion instead of combining
it with the static analysis annotation?

While __assume_ctx_lock() tells the compiler the lock is assumed held inside
the function, the standard practice to statically enforce that the caller
holds the lock is to add __must_hold(ap->lock) to the function signature
while retaining lockdep_assert_held() in the body. Retaining both provides
valuable run-time validation alongside compile-time checks.

Additionally, does removing this check here and introducing the static context
analysis in a follow-up commit temporarily leave this function without any
locking verification during intermediate bisect steps?

>  	/*
>  	 * ata_scsi_qc_new() calls scsi_done(cmd) in case of failure. So we
>  	 * have nothing further to do when allocating a qc fails.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260528172855.703631-5-cassel@kernel.org?part=1