Re: [RFC] Remove WARN_ON in pci_get_dev_by_id

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

 



On Thu, 17 Jun 2010 16:43:13 -0400
Don Zickus <dzickus@xxxxxxxxxx> wrote:

> I have been trying to track down sources of unknown NMIs and spurious
> interrupts recently using old fashion detective work of inspecting all
> devices on the PCI bus to see what ERR/INT bits are set.
> 
> Using a for-loop like this inside an NMI or interrupt context:
> 
> 	err_bits = <some interesting value>
> 	while ((dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL) {
> 		pci_read_config_word(dev,0x6,&data);
> 		if (data & err_bits)
> 			printk(...<found what i was looking for>...);
> 	}
> 
> Of course every time this code gets executed the I get bit by the
> WARN_ON(in_interrupt) inside of pci_get_dev_by_id().
> 
> I was trying to figure out why it was there.  I don't see any locks inside
> the call path or anything else that might be a no-no inside an interrupt
> context.  Perhaps I missed something?  Or is there a better way to do what
> I want to do calling lower level functions?
> 
> I understand my code is hack'ish and there could be other sources of NMIs
> or interrupts but for debugging purposes it sure is helping debug a bunch
> of problems (related to firmware).
> 
> Thanks,
> Don
> 
> diff --git a/drivers/pci/search.c b/drivers/pci/search.c
> index ec41535..874c975 100644
> --- a/drivers/pci/search.c
> +++ b/drivers/pci/search.c
> @@ -209,7 +209,6 @@ static struct pci_dev *pci_get_dev_by_id(const struct pci_device_id *id,
>  	struct device *dev_start = NULL;
>  	struct pci_dev *pdev = NULL;
>  
> -	WARN_ON(in_interrupt());
>  	if (from)
>  		dev_start = &from->dev;
>  	dev = bus_find_device(&pci_bus_type, dev_start, (void *)id,
> 

Well, let's see:
  pci_get_subsys calls kzalloc with GFP_KERNEL, but that's higher up in
  the stack (but still makes your initial usage unsafe in the general
  case)
  pci_get_dev_by_id calls bus_find_device, which may end up doing a
  klist_put, which takes the klist lock without irq protection... There
  are also possible match() and put() functions that could get called,
  maybe they assume !in_interrupt()?  And in pci_dev_put path there are
  more callbacks, like kobject_cleanup, which can sleep or take locks.

So unfortunately it's not really safe to do this in general.  You can
talk to Greg about rewriting the object core to be safe in interrupt
context though. :)



-- 
Jesse Barnes, Intel Open Source Technology Center
--
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