diff mbox

[v5,06/27] irq_domain/powerpc: eliminate irq_map; use irq_alloc_desc() instead

Message ID 20120404154020.40C513E09D5@localhost (mailing list archive)
State Not Applicable
Headers show

Commit Message

Grant Likely April 4, 2012, 3:40 p.m. UTC
On Tue, 03 Apr 2012 14:11:12 +0200, Andreas Schwab <schwab@linux-m68k.org> wrote:
> Grant Likely <grant.likely@secretlab.ca> writes:
> 
> > Can you dump out /debug/powerpc/virq_mapping from both before and
> > after the irq_map patch is applied?
> 
> before:
> virq   hwirq    chip name        chip data           host name
>    16  0x00000   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    21  0x00001   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    24  0x00002   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    25  0x00019   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    26  0x0001a   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    27  0x0001b   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    28  0x0001c   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    29  0x0003d   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    30  0x0001e   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    31  0x0003c   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    39  0x00027   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    40  0x00028   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    41  0x00029   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    42  0x0002a   MPIC 2          0xc00000017a011000  /u3@0,f8000000/mpic@f8040000
>    47  0x0002f   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    59  0x000fb   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    60  0x000fc   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    61  0x000fd   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    62  0x000fe   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    63  0x0003f   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 
> after:
> virq   hwirq    chip name        chip data           host name
>    16  0x00000   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    21  0x00001   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    24  0x00002   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    25  0x00019   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    26  0x0001a   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    27  0x0001b   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    28  0x0001c   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    30  0x0001e   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    39  0x00027   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    40  0x00028   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    41  0x00029   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    42  0x0002a   MPIC 2          0xc00000017a011000  /u3@0,f8000000/mpic@f8040000
>    47  0x0002f   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    59  0x000fb   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    60  0x000fc   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    61  0x000fd   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    62  0x000fe   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    63  0x0003f   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    64  0x0003d   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>    65  0x0003c   MPIC 1          0xc00000017a010000  /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 
> But I have NR_IRQS=64.  Bounds checking missing?  Irqs 64/65 are related
> to the sound chip (headphone-detect and line-out-detect).

I bet it is NR_IRQS related.  You have SPARSE_IRQ enabled, which means
the maximum number of irq_descs is IRQ_BITMAP_BITS (NR_IRQS + 8192).
The old powerpc code was strictly limited to NR_IRQS, but the new code
uses irq_alloc_descs() which isn't.  Yet I can see places in the
powerpc code that depends specifically on the value of NR_IRQS.  The
for_each_irq() macro for instance.  I think all the users there can be
switched to using for_each_irq_desc().

Can you attach console output logs for each of configs above and also
with NR_IRQS=128?  That might give me some clues as to which specific
code is causing the issues.  Also, as a quick test, try changing
for_each_irq_desc() to use "nr_irqs" instead of "NR_IRQS".  nr_irqs is
kept up to date with the real maximum number of irqs allocated in the
system:


g.

> 
> When reconfiguring with NR_IRQS=128 interrupts are working again, but I
> still see a lot of spurious interrupts, and the X server is still broken
> (no input works, but I still don't know whether that is an unrelated
> bug).
> 
> This is a sample of /proc/interrupts from 3.3 (with NR_IRQS=64):
>            CPU0       CPU1       
>  16:       2039       6070   MPIC 1    Level     sata_svw
>  21:          0          0   MPIC 1    Edge      i2sbus: i2s-a (tx)
>  22:         12         20   MPIC 1    Level   
>  23:         14         18   MPIC 1    Level   
>  24:          0          0   MPIC 1    Edge      i2sbus: i2s-a (rx)
>  25:          3          0   MPIC 1    Level     VIA-PMU
>  26:         16         62   MPIC 1    Level     keywest i2c
>  27:          0          1   MPIC 1    Level     ohci_hcd:usb2
>  28:          0          1   MPIC 1    Level     ohci_hcd:usb3
>  29:          0          0   MPIC 1    Edge      headphone-detect
>  30:          0          0   MPIC 1    Level     i2sbus: i2s-a (control)
>  31:          0          0   MPIC 1    Edge      line-output-detect
>  39:         22         64   MPIC 1    Level     pata-pci-macio
>  40:          0          2   MPIC 1    Level     firewire_ohci
>  41:         52        147   MPIC 1    Level     eth0
>  42:       1732       5053   MPIC 2    Level     keywest i2c
>  47:          0          0   MPIC 1    Level     GPIO1 ADB
>  59:          0          0   MPIC 1    Edge      ipi call function
>  60:       2064       1940   MPIC 1    Edge      ipi reschedule
>  61:       3406        945   MPIC 1    Edge      ipi call function single
>  62:          0          0   MPIC 1    Edge      ipi debugger
>  63:         39         91   MPIC 1    Level     ehci_hcd:usb1, ohci_hcd:usb4, ohci_hcd:usb5
> LOC:       3503       3719   Local timer interrupts
> SPU:          2          0   Spurious interrupts
> CNT:          0          0   Performance monitoring interrupts
> MCE:          0          0   Machine check exceptions
> 
> This is a sample of /proc/interrupts from 3.4-rc1 (with NR_IRQS=128):
>            CPU0       CPU1       
>  16:       2603       7596   MPIC 1    Level     sata_svw
>  21:          1          0   MPIC 1    Edge      i2sbus: i2s-a (tx)
>  22:         13         19   MPIC 1    Level   
>  23:          8         24   MPIC 1    Level   
>  24:          0          1   MPIC 1    Edge      i2sbus: i2s-a (rx)
>  25:          2          1   MPIC 1    Level     VIA-PMU
>  26:         21         57   MPIC 1    Level     keywest i2c
>  27:          0          1   MPIC 1    Level     ohci_hcd:usb2
>  28:          0          1   MPIC 1    Level     ohci_hcd:usb3
>  30:          0          0   MPIC 1    Level     i2sbus: i2s-a (control)
>  39:         39        131   MPIC 1    Level     pata-pci-macio
>  40:          2          2   MPIC 1    Level     firewire_ohci
>  41:         93        268   MPIC 1    Level     eth0
>  42:       8569      24140   MPIC 2    Level     keywest i2c
>  47:          0          0   MPIC 1    Level     GPIO1 ADB
>  60:          1          0   MPIC 1    Edge      line-output-detect
>  61:          1          0   MPIC 1    Edge      headphone-detect
>  63:        153        502   MPIC 1    Level     ehci_hcd:usb1, ohci_hcd:usb4, ohci_hcd:usb5
> 123:          0          0   MPIC 1    Edge      ipi call function
> 124:       1978       2349   MPIC 1    Edge      ipi reschedule
> 125:       2356       1816   MPIC 1    Edge      ipi call function single
> 126:          0          0   MPIC 1    Edge      ipi debugger
> LOC:       4417       7985   Local timer interrupts
> SPU:       9586      25811   Spurious interrupts
> CNT:          0          0   Performance monitoring interrupts
> MCE:          0          0   Machine check exceptions
> 
> Andreas.
> 
> -- 
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
> "And now for something completely different."

Comments

Andreas Schwab April 5, 2012, 10:51 a.m. UTC | #1
Grant Likely <grant.likely@secretlab.ca> writes:

> I bet it is NR_IRQS related.  You have SPARSE_IRQ enabled, which means
> the maximum number of irq_descs is IRQ_BITMAP_BITS (NR_IRQS + 8192).

The actual definition uses NR_IRQS + 8196.  Guess that's a typo.  (Does
it really make sense to add NR_IRQS here?)

> diff --git a/arch/powerpc/include/asm/irq.h b/arch/powerpc/include/asm/irq.h
> index cf417e51..9edf499 100644
> --- a/arch/powerpc/include/asm/irq.h
> +++ b/arch/powerpc/include/asm/irq.h
> @@ -20,7 +20,7 @@
>  
>  /* Define a way to iterate across irqs. */
>  #define for_each_irq(i) \
> -       for ((i) = 0; (i) < NR_IRQS; ++(i))
> +       for ((i) = 0; (i) < nr_irqs; ++(i))

There are exactly two uses of for_each_irq, one is related to cpu
hotplug, the other to kexec, so that cannot make any difference.

Andreas.
Andreas Schwab April 5, 2012, 10:10 p.m. UTC | #2
Grant Likely <grant.likely@secretlab.ca> writes:

> Can you attach console output logs for each of configs above and also
> with NR_IRQS=128?  That might give me some clues as to which specific
> code is causing the issues.

It really looks like the issue starts when irq_expand_nr_irqs is called
the first time to make nr_irqs bigger than NR_IRQS.

Andreas.
Thomas Gleixner April 6, 2012, 11:12 a.m. UTC | #3
On Thu, 5 Apr 2012, Andreas Schwab wrote:
> Grant Likely <grant.likely@secretlab.ca> writes:
> 
> > I bet it is NR_IRQS related.  You have SPARSE_IRQ enabled, which means
> > the maximum number of irq_descs is IRQ_BITMAP_BITS (NR_IRQS + 8192).
> 
> The actual definition uses NR_IRQS + 8196.  Guess that's a typo.  (Does
> it really make sense to add NR_IRQS here?)
> 
> > diff --git a/arch/powerpc/include/asm/irq.h b/arch/powerpc/include/asm/irq.h
> > index cf417e51..9edf499 100644
> > --- a/arch/powerpc/include/asm/irq.h
> > +++ b/arch/powerpc/include/asm/irq.h
> > @@ -20,7 +20,7 @@
> >  
> >  /* Define a way to iterate across irqs. */
> >  #define for_each_irq(i) \
> > -       for ((i) = 0; (i) < NR_IRQS; ++(i))
> > +       for ((i) = 0; (i) < nr_irqs; ++(i))
> 
> There are exactly two uses of for_each_irq, one is related to cpu
> hotplug, the other to kexec, so that cannot make any difference.

Though that wants to be fixed nevertheless.

Thanks,

	tglx
Thomas Gleixner April 6, 2012, 11:17 a.m. UTC | #4
On Fri, 6 Apr 2012, Andreas Schwab wrote:

> Grant Likely <grant.likely@secretlab.ca> writes:
> 
> > Can you attach console output logs for each of configs above and also
> > with NR_IRQS=128?  That might give me some clues as to which specific
> > code is causing the issues.
> 
> It really looks like the issue starts when irq_expand_nr_irqs is called
> the first time to make nr_irqs bigger than NR_IRQS.

And it looks like the irqdomain code is the real culprit.

void irq_set_virq_count(unsigned int count)
{
        pr_debug("irq: Trying to set virq count to %d\n", count);

        BUG_ON(count < NUM_ISA_INTERRUPTS);
        if (count < NR_IRQS)
                irq_virq_count = count;
}

That looks simply wrong.....

s/NR_IRQS/nr_irqs/ should do the trick.

Thanks,

	tglx
Andreas Schwab April 6, 2012, 11:25 a.m. UTC | #5
Thomas Gleixner <tglx@linutronix.de> writes:

> And it looks like the irqdomain code is the real culprit.
>
> void irq_set_virq_count(unsigned int count)
> {
>         pr_debug("irq: Trying to set virq count to %d\n", count);
>
>         BUG_ON(count < NUM_ISA_INTERRUPTS);
>         if (count < NR_IRQS)
>                 irq_virq_count = count;
> }
>
> That looks simply wrong.....

There is a single use of irq_set_virq_count, which is only relevant to
the PS3.

Andreas.
Thomas Gleixner April 6, 2012, 11:28 a.m. UTC | #6
On Fri, 6 Apr 2012, Andreas Schwab wrote:

> Thomas Gleixner <tglx@linutronix.de> writes:
> 
> > And it looks like the irqdomain code is the real culprit.
> >
> > void irq_set_virq_count(unsigned int count)
> > {
> >         pr_debug("irq: Trying to set virq count to %d\n", count);
> >
> >         BUG_ON(count < NUM_ISA_INTERRUPTS);
> >         if (count < NR_IRQS)
> >                 irq_virq_count = count;
> > }
> >
> > That looks simply wrong.....
> 
> There is a single use of irq_set_virq_count, which is only relevant to
> the PS3.

Though irq_virq_count is statically initialized to NR_IRQS and it's
used in the code more than once.
Grant Likely April 7, 2012, 1:29 a.m. UTC | #7
On Fri, 6 Apr 2012 13:17:04 +0200 (CEST), Thomas Gleixner <tglx@linutronix.de> wrote:
> On Fri, 6 Apr 2012, Andreas Schwab wrote:
> 
> > Grant Likely <grant.likely@secretlab.ca> writes:
> > 
> > > Can you attach console output logs for each of configs above and also
> > > with NR_IRQS=128?  That might give me some clues as to which specific
> > > code is causing the issues.
> > 
> > It really looks like the issue starts when irq_expand_nr_irqs is called
> > the first time to make nr_irqs bigger than NR_IRQS.
> 
> And it looks like the irqdomain code is the real culprit.
> 
> void irq_set_virq_count(unsigned int count)
> {
>         pr_debug("irq: Trying to set virq count to %d\n", count);
> 
>         BUG_ON(count < NUM_ISA_INTERRUPTS);
>         if (count < NR_IRQS)
>                 irq_virq_count = count;
> }
> 
> That looks simply wrong.....
> 
> s/NR_IRQS/nr_irqs/ should do the trick.

Yeah, that code is wrong and I'll fix it, but the only purpose of that
code is to support the direct mapping on hardware that has limit on
the largest irq number that it can handle.  That shouldn't be the
problem here.

g.
diff mbox

Patch

diff --git a/arch/powerpc/include/asm/irq.h b/arch/powerpc/include/asm/irq.h
index cf417e51..9edf499 100644
--- a/arch/powerpc/include/asm/irq.h
+++ b/arch/powerpc/include/asm/irq.h
@@ -20,7 +20,7 @@ 
 
 /* Define a way to iterate across irqs. */
 #define for_each_irq(i) \
-       for ((i) = 0; (i) < NR_IRQS; ++(i))
+       for ((i) = 0; (i) < nr_irqs; ++(i))
 
 extern atomic_t ppc_n_lost_interrupts;