MSI entry unlinked before arch_teardown_msi_irq().

MSI entry unlinked before arch_teardown_msi_irq().

[David Miller]

Eric, you recommended to me that I use:

	struct msi_desc *entry = get_irq_data(irq);

in my arch_teardown_msi_irq() routine earlier, but the current
code unlinks the entry before the call to arch_teardown_msi_irq()
so I get OOPS's on shutdown on sparc64 because of this since
get_irq_data() will return NULL at that point.

Can you fix this up or suggest how I should code the patch to
your liking so I can take care of it?

Thanks!

Re: MSI entry unlinked before arch_teardown_msi_irq().

[Eric W. Biederman]

David Miller <davem@davemloft.net> writes:

> ric, you recommended to me that I use:
>
> 	struct msi_desc *entry = get_irq_data(irq);
>
> in my arch_teardown_msi_irq() routine earlier, but the current
> code unlinks the entry before the call to arch_teardown_msi_irq()
> so I get OOPS's on shutdown on sparc64 because of this since
> get_irq_data() will return NULL at that point.
>
> Can you fix this up or suggest how I should code the patch to
> your liking so I can take care of it?

I'm guessing my patch to switch the appropriate from get_irq_data to
get_irq_msi was merged in between that conversation and this.

Sorry about that. 

There was a real problem that ppc used the data stored in get_irq_data
for something else.  So we added a dedicated pointer to struct irq_desc.

If that isn't the issue holler and I will look into it a little more.

Eric

Re: MSI entry unlinked before arch_teardown_msi_irq().

[David Miller]

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Sun, 25 Feb 2007 02:08:07 -0700

> If that isn't the issue holler and I will look into it a little more.

Talking in code is always more effective :-)

This patch fixes the OOPS I was talking about:

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 68555c1..2f2d92e 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -678,12 +678,14 @@ static int msi_free_irq(struct pci_dev* dev, int irq)
 	}
 	type = entry->msi_attrib.type;
 	entry_nr = entry->msi_attrib.entry_nr;
+
+	arch_teardown_msi_irq(irq);
+
 	head = entry->link.head;
 	base = entry->mask_base;
 	get_irq_msi(entry->link.head)->link.tail = entry->link.tail;
 	get_irq_msi(entry->link.tail)->link.head = entry->link.head;
 
-	arch_teardown_msi_irq(irq);
 	kmem_cache_free(msi_cachep, entry);
 
 	if (type == PCI_CAP_ID_MSIX) {

Re: MSI entry unlinked before arch_teardown_msi_irq().

[Eric W. Biederman]

David Miller <davem@davemloft.net> writes:

> From: ebiederm@xmission.com (Eric W. Biederman)
> Date: Sun, 25 Feb 2007 02:08:07 -0700
>
>> If that isn't the issue holler and I will look into it a little more.
>
> Talking in code is always more effective :-)
>
> This patch fixes the OOPS I was talking about:

Ok.  That is clearly reasonable.  Why do you need to traverse
the list here?  I just want to understand the requirements.

Eric

Re: MSI entry unlinked before arch_teardown_msi_irq().

[David Miller]

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Sun, 25 Feb 2007 17:14:43 -0700

> David Miller <davem@davemloft.net> writes:
> 
> > From: ebiederm@xmission.com (Eric W. Biederman)
> > Date: Sun, 25 Feb 2007 02:08:07 -0700
> >
> >> If that isn't the issue holler and I will look into it a little more.
> >
> > Talking in code is always more effective :-)
> >
> > This patch fixes the OOPS I was talking about:
> 
> Ok.  That is clearly reasonable.  Why do you need to traverse
> the list here?  I just want to understand the requirements.

Nevermind, something strange is going on here.  I think my
patch is bogus and the thing is still crashing, let me get
back to you.

Re: MSI entry unlinked before arch_teardown_msi_irq().

[Eric W. Biederman]

David Miller <davem@davemloft.net> writes:

> Nevermind, something strange is going on here.  I think my
> patch is bogus and the thing is still crashing, let me get
> back to you.

Sure.  

Eric

Re: MSI entry unlinked before arch_teardown_msi_irq().

[David Miller]

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Sun, 25 Feb 2007 17:14:43 -0700

> David Miller <davem@davemloft.net> writes:
> 
> > From: ebiederm@xmission.com (Eric W. Biederman)
> > Date: Sun, 25 Feb 2007 02:08:07 -0700
> >
> >> If that isn't the issue holler and I will look into it a little more.
> >
> > Talking in code is always more effective :-)
> >
> > This patch fixes the OOPS I was talking about:
> 
> Ok.  That is clearly reasonable.  Why do you need to traverse
> the list here?  I just want to understand the requirements.

The real problem turned out to be my using get_irq_data() instead
of get_irq_msi() :-)

Sorry for the noise.

Re: MSI entry unlinked before arch_teardown_msi_irq().

[Eric W. Biederman]

David Miller <davem@davemloft.net> writes:

> From: ebiederm@xmission.com (Eric W. Biederman)
> Date: Sun, 25 Feb 2007 17:14:43 -0700
>
>> David Miller <davem@davemloft.net> writes:
>> 
>> > From: ebiederm@xmission.com (Eric W. Biederman)
>> > Date: Sun, 25 Feb 2007 02:08:07 -0700
>> >
>> >> If that isn't the issue holler and I will look into it a little more.
>> >
>> > Talking in code is always more effective :-)
>> >
>> > This patch fixes the OOPS I was talking about:
>> 
>> Ok.  That is clearly reasonable.  Why do you need to traverse
>> the list here?  I just want to understand the requirements.
>
> The real problem turned out to be my using get_irq_data() instead
> of get_irq_msi() :-)
>
> Sorry for the noise.

No problem I helped cause the confusion...

Eric