Re: [PATCH] pci: detect and abort on 0 irq in AER driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Saturday 03 January 2009, Arjan van de Ven wrote:
> 
> From 96cc66d7ebf9e55fe479cbae9f44619550758821 Mon Sep 17 00:00:00 2001
> From: Arjan van de Ven <arjan@xxxxxxxxxxxxxxx>
> Date: Sat, 3 Jan 2009 07:53:55 -0800
> Subject: [PATCH] pci: detect and abort on 0 irq in AER driver
> 
> irq 0 is not a valid irq for AER reporting, however sometimes
> a system that doesn't really support AER lets this slip through.

This is a result of a bug in the PCI Express port driver which shouldn't
try to register the AER service device in such a case.  I have a fix in the
works, but it depends on some other patches I've posted recently.

IMO, the entire handling of PCI Express port is overly complicated.  Namely,
instead of having the pcie_port_bus_type (which is buggy at the moment, as
described in http://marc.info/?l=linux-pci&m=123083608311844&w=4) it
would be better to register port services directly within the port driver using
a simple mechanism like an array (there are at most 4 services possible and
we know exactly what they are).

I have an idea how to rework it, but first I'd like to know what your opinions
are.

> This patch aborts early rather than registering at this invalid interrupt
> (which also happens to be the timer interrupt, much fun could happen)
> 
> As seen by kerneloops.org report 165038

That said, I think the patch is fine anyway.

Thanks,
Rafael


> Reported-by: kerneloops.org
> Signed-off-by: Arjan van de Ven <arjan@xxxxxxxxxxxxxxx>
> ---
>  drivers/pci/pcie/aer/aerdrv.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/pci/pcie/aer/aerdrv.c b/drivers/pci/pcie/aer/aerdrv.c
> index e390707..738563a 100644
> --- a/drivers/pci/pcie/aer/aerdrv.c
> +++ b/drivers/pci/pcie/aer/aerdrv.c
> @@ -214,6 +214,13 @@ static int __devinit aer_probe (struct pcie_device *dev,
>  	struct aer_rpc *rpc;
>  	struct device *device = &dev->device;
>  
> +	/*
> +	 * If dev->irq is 0, then AER is not working, and we should bail out
> +	 * early. This has been seen in the wild via kerneloops.org.
> +	 */
> +	if (!dev->irq)
> +		return -ENODEV;
> +
>  	/* Init */
>  	if ((status = aer_init(dev)))
>  		return status;
> -- 
> 1.6.0.6
> 
> 


-- 
Everyone knows that debugging is twice as hard as writing a program
in the first place.  So if you're as clever as you can be when you write it,
how will you ever debug it? --- Brian Kernighan
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux