[PATCH 1/1] libata-dev: add flush task to ata_exec_internal()

[PATCH 1/1] libata-dev: add flush task to ata_exec_internal()

[Albert Lee]

Add ata_port_flush_task() to ata_exec_internal().

Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
---

It seems we also need ata_port_flush_task() in ata_exec_internal().

Patch against the upstream branch
(fb21f0d0ec7e31cc814165e1a9d2662d9c9dd980).

For your review, thanks,

Albert

--- upstream0/drivers/scsi/libata-core.c	2006-03-14 10:31:01.000000000 +0800
+++ exec_internal/drivers/scsi/libata-core.c	2006-03-14 10:55:24.000000000 +0800
@@ -962,6 +962,8 @@ ata_exec_internal(struct ata_port *ap, s
 	spin_unlock_irqrestore(&ap->host_set->lock, flags);
 
 	if (!wait_for_completion_timeout(&wait, ATA_TMOUT_INTERNAL)) {
+		ata_port_flush_task(ap);
+
 		spin_lock_irqsave(&ap->host_set->lock, flags);
 
 		/* We're racing with irq here.  If we lose, the

Re: [PATCH 1/1] libata-dev: add flush task to ata_exec_internal()

[Tejun Heo]

Albert Lee wrote:
> Add ata_port_flush_task() to ata_exec_internal().
> 
> Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
> ---
> 
> It seems we also need ata_port_flush_task() in ata_exec_internal().
> 
> Patch against the upstream branch
> (fb21f0d0ec7e31cc814165e1a9d2662d9c9dd980).
> 
> For your review, thanks,
> 
> Albert
> 
> --- upstream0/drivers/scsi/libata-core.c	2006-03-14 10:31:01.000000000 +0800
> +++ exec_internal/drivers/scsi/libata-core.c	2006-03-14 10:55:24.000000000 +0800
> @@ -962,6 +962,8 @@ ata_exec_internal(struct ata_port *ap, s
>  	spin_unlock_irqrestore(&ap->host_set->lock, flags);
>  
>  	if (!wait_for_completion_timeout(&wait, ATA_TMOUT_INTERNAL)) {
> +		ata_port_flush_task(ap);
> +
>  		spin_lock_irqsave(&ap->host_set->lock, flags);
>  
>  		/* We're racing with irq here.  If we lose, the
> 

Yes, we certainly need this. The irq handling isn't really correct 
either as irq handlers don't clear interrupt conditions on spurious 
interrupts. I hope/plan to address this during EH work.

Anyways, ACK.

Thanks.

-- 
tejun

Re: [PATCH 1/1] libata-dev: add flush task to ata_exec_internal()

[Jeff Garzik]

Tejun Heo wrote:
> Yes, we certainly need this. The irq handling isn't really correct 
> either as irq handlers don't clear interrupt conditions on spurious 
> interrupts. I hope/plan to address this during EH work.

Except for the HSM, there is no guaranteed way to know if an interrupt 
is spurious or not on many controllers, who lack an "I have an 
interrupt" bit.

	Jeff

Re: [PATCH 1/1] libata-dev: add flush task to ata_exec_internal()

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
> 
>> Yes, we certainly need this. The irq handling isn't really correct 
>> either as irq handlers don't clear interrupt conditions on spurious 
>> interrupts. I hope/plan to address this during EH work.
> 
> 
> Except for the HSM, there is no guaranteed way to know if an interrupt 
> is spurious or not on many controllers, who lack an "I have an 
> interrupt" bit.
> 

What I'm thinking is to disable interrupt (hopefully using controller 
flag, maybe PCI IRQ disable, ATA_NIEN if nothing else is possible) on 
error condition which may breach HSM and force controller and device to 
a known state in ->error_handler() or ->post_internal_command() before 
reenabling interrupt. This method will also make libata resistant to 
screaming IRQ cases some controllers/drives show under certain 
circumstances.

And I think clearing ->hsm_task_state belongs to (to-be-added) 
->post_internal_command() as it belongs to ->eng_timeout but not to EH 
framework.

Thanks.

-- 
tejun

Re: [PATCH 1/1] libata-dev: add flush task to ata_exec_internal()

[Jeff Garzik]

Albert Lee wrote:
> Add ata_port_flush_task() to ata_exec_internal().
> 
> Signed-off-by: Albert Lee <albertcc@tw.ibm.com>

applied