Re: [PATCH v2 2/9] ata: libata: Pass the ATA port argument directly to __ata_scsi_queuecmd()

[Niklas Cassel <cassel@kernel.org>]

Hello Bart,

On Thu, May 21, 2026 at 10:33:30AM -0700, Bart Van Assche wrote:
> Prepare for adding lock context annotations that refer to the ATA port
> argument (ap). No functionality has been changed.

For this and the other preparation patches:
Please add some additional text explaining why this change is required for
us to add lock context annotations.

I guess it is not possible to simply add __must_hold(dev->link->ap->lock)
annotation to __ata_scsi_queuecmd() ?

But at the same time, you do add __must_hold(dev->link->ap->lock) to e.g.
ata_scsi_translate().



Looking at the C-file, I can see that patch 9/9 adds:

+	/* Tell the compiler that dev->link->ap == ap. */
+	__assume_ctx_lock(dev->link->ap->lock);

to __ata_scsi_queuecmd().

but, that annotation is using dev->link->ap and not ap directly.


Patch 9/9 also adds a __must_hold(ap->lock) annotation to the declaration
of __ata_scsi_queuecmd(), i.e. in the header file.

Personally, I think that it makes more sense to have the annotation in the
definition (C-file), since that is what we most often read.
If clang requires us to also add the annotation to the declaration, then
perhaps we can have the annotation both in the C-file and the header file?
(Especially since you do annotate the function definition for those functions
that do not have a declaration in the header file.)



Not strictly needed, but assuming that we still need to grow an ap parameter
to many functions, would it perhaps be possible to restructure the series like:
1) Pass the ATA port argument directly to __ata_scsi_queuecmd()
2) Add annotations to __ata_scsi_queuecmd()
3) Pass the ATA port argument directly to ata_qc_schedule_eh()
4) Add annotations to ata_qc_schedule_eh()
5) Pass the ATA port argument directly to ata_qc_complete()
6) Add annotations to ata_qc_complete()
...

That should be possible, right?
(I guess you might need to reorder some patches.)


Thank you for spending time on this!


Kind regards,
Niklas