Re: [PATCH v4]: PCI Error Recovery: Symbios SCSI device driver

[Matthew Wilcox <matthew@wil.cx>]

On Wed, Nov 01, 2006 at 07:19:37PM -0600, Linas Vepstas wrote:
> @@ -657,6 +657,10 @@ static irqreturn_t sym53c8xx_intr(int ir
>  	unsigned long flags;
>  	struct sym_hcb *np = (struct sym_hcb *)dev_id;
>  
> +	/* Avoid spinloop trying to handle interrupts on frozen device */
> +	if (pci_channel_offline(np->s.device))
> +		return IRQ_HANDLED;
> +
>  	if (DEBUG_FLAGS & DEBUG_TINY) printf_debug ("[");
>  
>  	spin_lock_irqsave(np->s.host->host_lock, flags);

Just wondering ... should we really be returning HANDLED?  What if the
IRQ is shared?  Will the hardware de-assert the level interrupt when it
puts the device in reset (ie is this a transitory glitch?), or do we
have to cope with a screaming interrupt?

> +#define WAIT_FOR_PCI_RECOVERY	35
> +	if (pci_channel_offline(np->s.device))
> +	{

I prefer if () {

> +static pci_ers_result_t sym2_io_slot_reset (struct pci_dev *pdev)
> +{
> +	struct sym_hcb *np = pci_get_drvdata(pdev);
> +
> +	printk(KERN_INFO "%s: recovering from a PCI slot reset\n",
> +	          sym_name(np));
> +
> +	if (pci_enable_device(pdev)) {
> +		printk(KERN_ERR "%s: Unable to enable afer PCI reset\n",
> +		        sym_name(np));
> +		return PCI_ERS_RESULT_DISCONNECT;
> +	}
> +
> +	pci_set_master(pdev);
> +	enable_irq(pdev->irq);

Hm.  If we need to call pci_set_master, then we're also going to need to
call pci_set_mwi (if appropriate) which is currently done in
sym_set_workarounds().  Except you don't have a sym_device, or a
sym_chip around at this point.  Bother.  Need to do some refactoring to
take care of that.

> +		/* Prevent deadlock waiting on a condition that may never clear. */
> +		if (unlikely(sist == 0xffff && dstat == 0xff)) {
> +			if (unlikely(pci_channel_offline(np->s.device)))
> +				return;
> +		}

I like the first unlikely ... but I'd drop the second one.  If they are
both ffff ff, I'd say it's quite likely ;-)  Anyway, the first unlikely
is good enough a hint to GCC, IMO.

Thanks!