Re: [PATCH 2/2] ata: libata-core: fix cancellation of a port deferred qc work

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

On 2/21/26 02:33, Igor Pylypiv wrote:
> On Fri, Feb 20, 2026 at 02:00:53PM +0900, Damien Le Moal wrote:
>> cancel_work_sync() is a sleeping function so it cannot be called with
>> the spin lock of a port being held. Move the call to this function in
>> ata_port_detach() before locking the port and add a call to
>> ata_scsi_requeue_deferred_qc() to make sure there are no remaining
>> deferred qc.
>>
>> Fixes: 0ea84089dbf6 ("ata: libata-scsi: avoid Non-NCQ command starvation")
>> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
>> ---
>>  drivers/ata/libata-core.c | 13 +++++++++----
>>  1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
>> index beb6984b379a..54afa4df987e 100644
>> --- a/drivers/ata/libata-core.c
>> +++ b/drivers/ata/libata-core.c
>> @@ -6254,9 +6254,18 @@ static void ata_port_detach(struct ata_port *ap)
>>  	/* Wait for any ongoing EH */
>>  	ata_port_wait_eh(ap);
>>  
>> +	/* Requeue any remaining deferred qc. */
>> +	ata_scsi_requeue_deferred_qc(ap);
>> +
> 
> ata_scsi_requeue_deferred_qc() requires the port lock to be held (function
> has a lockdep assert).

Doh ! Yes, of course ! I had lockdep disabled on my test rig... Bad idea :)
Let me fix this.

> 
>>  	mutex_lock(&ap->scsi_scan_mutex);
>> +
>> +	/* Make sure the deferred qc work finished. */
>> +	cancel_work_sync(&ap->deferred_qc_work);
>> +
>>  	spin_lock_irqsave(ap->lock, flags);
>>  
>> +	WARN_ON(ap->deferred_qc);
>> +
>>  	/* Remove scsi devices */
>>  	ata_for_each_link(link, ap, HOST_FIRST) {
>>  		ata_for_each_dev(dev, link, ALL) {
>> @@ -6269,10 +6278,6 @@ static void ata_port_detach(struct ata_port *ap)
>>  		}
>>  	}
>>  
>> -	/* Make sure the deferred qc work finished. */
>> -	cancel_work_sync(&ap->deferred_qc_work);
>> -	WARN_ON(ap->deferred_qc);
>> -
>>  	/* Tell EH to disable all devices */
>>  	ap->pflags |= ATA_PFLAG_UNLOADING;
>>  	ata_port_schedule_eh(ap);
>> -- 
>> 2.53.0
>>
>>


-- 
Damien Le Moal
Western Digital Research