Re: [PATCH 13/14] ahci: convert to new EH

["zhao, forrest" <forrest.zhao@intel.com>]

On Tue, 2006-04-11 at 22:48 +0900, Tejun Heo wrote:
> Convert AHCI to new EH.  Unfortunately, ICH7 AHCI reacts badly if IRQ
> mask is diddled during operation.  So, freezing is implemented by
> unconditionally clearing interrupt conditions while frozen.

>  
> -static inline int ahci_host_intr(struct ata_port *ap, struct ata_queued_cmd *qc)
> +static inline void ahci_host_intr(struct ata_port *ap)
>  {
> +	struct ahci_port_priv *pp = ap->private_data;
>  	void __iomem *mmio = ap->host_set->mmio_base;
>  	void __iomem *port_mmio = ahci_port_base(mmio, ap->port_no);
> -	u32 status, serr, ci;
> -
> -	serr = readl(port_mmio + PORT_SCR_ERR);
> -	writel(serr, port_mmio + PORT_SCR_ERR);
> +	u32 status, serror, ci;
> +	unsigned int eh_flags;
>  
>  	status = readl(port_mmio + PORT_IRQ_STAT);
>  	writel(status, port_mmio + PORT_IRQ_STAT);
>  
> -	ci = readl(port_mmio + PORT_CMD_ISSUE);
> -	if (likely((ci & 0x1) == 0)) {
> -		if (qc) {
> -			WARN_ON(qc->err_mask);
> -			ata_qc_complete(qc);
> -			qc = NULL;
> -		}
> +	/* AHCI gets unhappy if IRQ mask is diddled with while the
> +	 * port is active, so we cannot disable IRQ when freezing.
> +	 * Clear IRQ conditions and hope screaming IRQs don't happen.
> +	 */
> +	if (ap->flags & ATA_FLAG_FROZEN) {
> +		/* some AHCI errors hang the controller until SError
> +		 * is cleared.  Store and clear it.
> +		 */
> +		serror = scr_read(ap, SCR_ERROR);
> +		scr_write(ap, SCR_ERROR, serror);
> +		pp->eh_irq_stat |= status;
> +		pp->eh_serror |= serror;
> +		return;
>  	}
>  
> -	if (status & PORT_IRQ_FATAL) {
> -		unsigned int err_mask;
> -		if (status & PORT_IRQ_TF_ERR)
> -			err_mask = AC_ERR_DEV;
> -		else if (status & PORT_IRQ_IF_ERR)
> -			err_mask = AC_ERR_ATA_BUS;
> -		else
> -			err_mask = AC_ERR_HOST_BUS;
> -
> -		/* command processing has stopped due to error; restart */
> -		ahci_restart_port(ap, status);
> -
> -		if (qc) {
> -			qc->err_mask |= err_mask;
> -			ata_qc_complete(qc);
> +	if (!(status & PORT_IRQ_ERROR)) {
> +		struct ata_queued_cmd *qc;
> +
> +		if ((qc = ata_qc_from_tag(ap, ap->active_tag))) {
> +			ci = readl(port_mmio + PORT_CMD_ISSUE);
> +			if ((ci & 0x1) == 0) {
> +				ata_qc_complete(qc);
> +				return;
> +			}
>  		}
> +
> +		if (ata_ratelimit())
> +			printk(KERN_INFO "ata%u: spurious interrupt "
> +			       "(irq_stat 0x%x active_tag %d)\n",
> +			       ap->id, status, ap->active_tag);
> +
> +		return;
>  	}
>  
> -	return 1;
> +	/* Something weird is going on.  Hand over to EH. */
> +	serror = scr_read(ap, SCR_ERROR);
> +	scr_write(ap, SCR_ERROR, serror);
> +
> +	pp->eh_irq_stat = status;
> +	pp->eh_serror = serror;
> +
> +	eh_flags = ATA_EH_ABORT;
> +	if (status & PORT_IRQ_FREEZE)
> +		eh_flags |= ATA_EH_FREEZE;
> +
> +	ata_eh_schedule_port(ap, eh_flags);
>  }
>  

Anther problem with this patch is that interrupts happened
when port is in ATA_FLAG_FROZEN state are recorded in pp->eh_irq_stat.
So a later user-initiated warm-unplug operation might trigger EH
to handle the error conditions previously recorded in pp->eh_irq_stat.

I observed this problem in our lab
1 execute many hot-plug/unplug operations, and finally disk B is
at port 2

2 echo "1" > /sys/class/scsi_host/host1/device/target1\:0\:0/1\:0\:0\:0/
delete

3 then the output in dmesg is:
ata2: dev 0 disabled
ata2: stat 0x50 err 0x0 SError 0x40d0000 action 0x2
      (irq_stat 0x00400040, connection status changed)
ata2: probing bus for new devices
ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
ata2: dev 0 cfg 49:2f00 82:7469 83:7f01 84:4023 85:7469 86:3c01 87:4023
88:407f
ata2: dev 0 ATA-7, max UDMA/133, 156301488 sectors: LBA48
ata2: dev 0 configured for UDMA/133
  Vendor: ATA       Model: WDC WD800JD-22LS  Rev: 06.0
  Type:   Direct-Access                      ANSI SCSI revision: 05
SCSI device sdb: 156301488 512-byte hdwr sectors (80026 MB)
sdb: Write Protect is off
sdb: Mode Sense: 00 3a 00 00
SCSI device sdb: drive cache: write back
SCSI device sdb: 156301488 512-byte hdwr sectors (80026 MB)
sdb: Write Protect is off
sdb: Mode Sense: 00 3a 00 00
SCSI device sdb: drive cache: write back
 sdb: unknown partition table
sd 1:0:0:0: Attached scsi disk sdb

4 I execute "echo "1" > /sys/class/scsi_host/host1/device/target1\:0
\:0/1\:0\:0\:0/delete" again, then the output in dmesg is:
ata2: dev 0 disabled
ata2: stat 0x50 err 0x0 SError 0x0 action 0x2
      (irq_stat 0x00000000)
ata2: soft resetting channel for error handling
ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 300)


Thanks,
Forrest