On Fri, 2007-05-11 at 13:26 -0700, David Miller wrote: > Hi Michael, I'm still working through the various regressions on > sparc64 added by your MSI changes :-) Hi Dave, Guilty as charged - I did CC you on the patches though ;) > The one I fixed the other day was a missed switch over to > alloc_pci_dev() in the sparc64 PCI probing code which caused an OOPS > in pci_enable_msi() because the list head of the pci dev was not > initialized. PowerPC's OBP firmware tree based PCI probing code > was updated, sparc64's wasnt. Sorry - not sure how I missed that one, it even matches "k.alloc(.*pci_dev" - thanks for fixing it :) > Today's find is a triggered assertion in msi_free_irqs() when the > system doesn't support MSI, in which case arch_setup_msi_irqs() always > returns an error. What do you need to determine that the system can't support MSI? Could you do that logic in arch_msi_check_device()? > The problem is that when this happens we branch into msi_free_irqs(), > to which you added the following assertion loop: > > list_for_each_entry(entry, &dev->msi_list, list) > BUG_ON(irq_has_action(entry->irq)); > > Well, if arch_setup_msi_irqs() fails, entry->irq will be zero and > although that's never assigned to any normal devices we use that IRQ > number for the timer interrupt on sparc64 so this assertion triggers. > > Better to test for zero before doing the irq_has_action() assertion > thing. Yep, looks good - it matches the logic in arch_teardown_msi_irqs(). cheers Acked-by: Michael Ellerman <[email protected]> > Signed-off-by: David S. Miller <[email protected]> > > diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c > index e6740d1..d9cbd58 100644 > --- a/drivers/pci/msi.c > +++ b/drivers/pci/msi.c > @@ -549,8 +549,10 @@ static int msi_free_irqs(struct pci_dev* dev) > { > struct msi_desc *entry, *tmp; > > - list_for_each_entry(entry, &dev->msi_list, list) > - BUG_ON(irq_has_action(entry->irq)); > + list_for_each_entry(entry, &dev->msi_list, list) { > + if (entry->irq) > + BUG_ON(irq_has_action(entry->irq)); > + } > > arch_teardown_msi_irqs(dev); > -- Michael Ellerman OzLabs, IBM Australia Development Lab wwweb: http://michael.ellerman.id.au phone: +61 2 6212 1183 (tie line 70 21183) We do not inherit the earth from our ancestors, we borrow it from our children. - S.M.A.R.T Person
Attachment:
signature.asc
Description: This is a digitally signed message part
- References:
- [PATCH]: Fix assertion failure with MSI on sparc64
- From: David Miller <[email protected]>
- [PATCH]: Fix assertion failure with MSI on sparc64
- Prev by Date: [patch 2/2] epoll locks changes and cleanups ...
- Next by Date: Re: [PATCH 1/12] crypto: don't pollute the global namespace with sg_next()
- Previous by thread: [PATCH]: Fix assertion failure with MSI on sparc64
- Next by thread: [PATCH] mmconfig x86_64/i386 : Insert unclaimed MMCONFIG resources.
- Index(es):
 |