Re: [PATCH 1/2] libata: fix ata_qc_issue failure path

[Tejun Heo <htejun@gmail.com>]

Hello, Jeff.

Jeff Garzik wrote:
> Tejun Heo wrote:
>> On sg_err failure path, ata_qc_issue() doesn't mark the qc active
>> before returning.  This triggers WARN_ON() in __ata_qc_complete() when
>> the qc gets completed.  This patch moves ap->active_tag and
>> QCFLAG_ACTIVE setting to the top of the function.
>>
>> Signed-off-by: Tejun Heo <htejun@gmail.com>
> 
> applied 1-2, but two comments:
> 
> * this patch widens the race window for the remaining unlocked uses of 
> ATA_QCFLAG_ACTIVE

Hmmm.. the only unloked use of ATA_QCFLAG_ACTIVE I could find was in 
pio_task and later patches will tighten that up. Any other places?

> * in the current code, its questionable whether ATA_QCFLAG_ACTIVE has 
> much value.  The flag may have more value after your EH work, but its 
> not terribly important in the current #upstream.

Yeap, ap->active_tag always coincides with ATA_QCFLAG_ACTIVE. And, yeah, 
it gets more important especially with NCQ as then we have two different 
mechanism to indicate active commands from the port (ap->active_tag and 
ap->sactive), so ATA_QCFLAG_ACTIVE is quite handy as aggregate test 
condition.

Thanks.

-- 
tejun