Re: [PATCH linux-2.6.13-rc3] Mod15Write quirk against v2.6.13

[Jeff Garzik <jgarzik@pobox.com>]

Tejun Heo wrote:
> 
>  Hi, Jeff.
> 
> Jeff Garzik wrote:
> 
>> Tejun Heo wrote:
>>
>>>  sata_sil Mod15Write workaround was broken by the following commit by
>>> Albert Lee.
>>>
>>> Commit: 21b1ed74ee3667dcabcba92e486988ea9119a085
>>> [PATCH] libata: Prevent the interrupt handler from completing a 
>>> command twice
>>>
>>>  This commit clears ATA_QCFLAG_ACTIVE in ata_qc_complete() and doesn't
>>> handle IRQ if ATA_QCFLAG_ACTIVE is cleared on entry to interrupt
>>> routine.  As m15w workaround executes single command multiple times,
>>> the flag is cleared after the first chunk completion and the following
>>> interrupt gets ignored resulting in "nobody cared" interrupt error.
>>>
>>>  The following changes are made in m15w workaround to fix this.
>>>
>>>  * Moved clearing of ATA_QCFLAG_ACTIVE before invoking ->complete_fn,
>>>    so that ->complete_fn can mangle with the flag.  This doesn't affect
>>>    any users.
>>>  * Added setting ATA_QCFLAG_ACTIVE in m15w chunk completion function.
>>
>>
>>
>>> diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
>>> --- a/drivers/scsi/libata-core.c
>>> +++ b/drivers/scsi/libata-core.c
>>> @@ -3087,8 +3087,8 @@ void ata_qc_complete(struct ata_queued_c
>>>          ata_sg_clean(qc);
>>>  
>>>      /* call completion callback */
>>> -    rc = qc->complete_fn(qc, drv_stat);
>>>      qc->flags &= ~ATA_QCFLAG_ACTIVE;
>>> +    rc = qc->complete_fn(qc, drv_stat);
>>>  
>>>      /* if callback indicates not to complete command (non-zero),
>>>       * return immediately
>>
>>
>>
>>
>> I'm leaning towards applying latest Albert's ATA_QCFLAG_ACTIVE fix, 
>> which does the same thing your patch did.
>>
>> For various complex tasks in the SCSI translation layer (SAT), we may 
>> wish to issue multiple ATA commands, before signalling completion.  As 
>> your mod15write patch is an example of this, it helps point out what 
>> parts of libata need work in order to accomplish this.
>>
>> Any comments before I apply Albert's patch?
>>
> 
>  I think the following patch I've posted deal with the same problem.
> 
> http://marc.theaimsgroup.com/?l=linux-ide&m=112454734102242&w=2


In the SCSI translation layer (SAT), some simulation may require issuing 
multiple ATA commands, before indicating a completion to the upper 
layers.  That capability is something I definitely want libata to 
support.  Your mod15write patch is an excellent example of this, even 
though I NAK'd it due to long term sata_sil maintenance burden.

That's why I think move ATA_QCFLAG_ACTIVE -anyway- may be a good idea.

	Jeff