Patchwork [1/6] powerpc: Make NR_IRQS a CONFIG option

login
register
mail settings
Submitter Michael Ellerman
Date Oct. 14, 2009, 5:44 a.m.
Message ID <a6fc612b826ff12628e62601203d565df00c6436.1255499081.git.michael@ellerman.id.au>
Download mbox | patch
Permalink /patch/35914/
State Accepted
Commit 551b81f26ffc2135b8490babad1a9ab12d617e8d
Delegated to: Benjamin Herrenschmidt
Headers show

Comments

Michael Ellerman - Oct. 14, 2009, 5:44 a.m.
The irq_desc array consumes quite a lot of space, and for systems
that don't need or can't have 512 irqs it's just wasted space.

The first 16 are reserved for ISA, so the minimum of 32 is really
16 - and no one has asked for more than 512 so leave that as the
maximum.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/Kconfig           |   10 ++++++++++
 arch/powerpc/include/asm/irq.h |    4 ++--
 2 files changed, 12 insertions(+), 2 deletions(-)
Grant Likely - Oct. 14, 2009, 6:59 p.m.
On Tue, Oct 13, 2009 at 11:44 PM, Michael Ellerman
<michael@ellerman.id.au> wrote:
> The irq_desc array consumes quite a lot of space, and for systems
> that don't need or can't have 512 irqs it's just wasted space.
>
> The first 16 are reserved for ISA, so the minimum of 32 is really
> 16 - and no one has asked for more than 512 so leave that as the
> maximum.

Does it really make sense to have this as a user twiddlable value?
Especially when many users just don't have the background to know what
an appropriate value is here and will get it wrong?  I believe your
sparse IRQ patch has a bigger impact anyway on systems where memory is
tight.

g.

>
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> ---
>  arch/powerpc/Kconfig           |   10 ++++++++++
>  arch/powerpc/include/asm/irq.h |    4 ++--
>  2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 10a0a54..2230e75 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -56,6 +56,16 @@ config IRQ_PER_CPU
>        bool
>        default y
>
> +config NR_IRQS
> +       int "Number of virtual interrupt numbers"
> +       range 32 512
> +       default "512"
> +       help
> +         This defines the number of virtual interrupt numbers the kernel
> +         can manage. Virtual interrupt numbers are what you see in
> +         /proc/interrupts. If you configure your system to have too few,
> +         drivers will fail to load or worse - handle with care.
> +
>  config STACKTRACE_SUPPORT
>        bool
>        default y
> diff --git a/arch/powerpc/include/asm/irq.h b/arch/powerpc/include/asm/irq.h
> index bbcd1aa..b83fcc8 100644
> --- a/arch/powerpc/include/asm/irq.h
> +++ b/arch/powerpc/include/asm/irq.h
> @@ -34,8 +34,8 @@ extern atomic_t ppc_n_lost_interrupts;
>  */
>  #define NO_IRQ_IGNORE          ((unsigned int)-1)
>
> -/* Total number of virq in the platform (make it a CONFIG_* option ? */
> -#define NR_IRQS                512
> +/* Total number of virq in the platform */
> +#define NR_IRQS                CONFIG_NR_IRQS
>
>  /* Number of irqs reserved for the legacy controller */
>  #define NUM_ISA_INTERRUPTS     16
> --
> 1.6.2.1
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
Michael Ellerman - Oct. 14, 2009, 11:47 p.m.
On Wed, 2009-10-14 at 12:59 -0600, Grant Likely wrote:
> On Tue, Oct 13, 2009 at 11:44 PM, Michael Ellerman
> <michael@ellerman.id.au> wrote:
> > The irq_desc array consumes quite a lot of space, and for systems
> > that don't need or can't have 512 irqs it's just wasted space.
> >
> > The first 16 are reserved for ISA, so the minimum of 32 is really
> > 16 - and no one has asked for more than 512 so leave that as the
> > maximum.
> 
> Does it really make sense to have this as a user twiddlable value?
> Especially when many users just don't have the background to know what
> an appropriate value is here and will get it wrong?  I believe your
> sparse IRQ patch has a bigger impact anyway on systems where memory is
> tight.

We have users? But yes I think it's reasonable, there's a million other
options people can fiddle with and break their kernel, I don't see that
this is much different.

The sparse IRQ patch has a bigger difference on the size of the irq_desc
array, but there are still other things that are statically sized based
on NR_IRQs. So if you're building an machine-specific kernel and you
know you're only going to have N interrupts, then this will give you a
bigger saving.

But I'm not super fussed, if other people think it's too dangerous we
can drop it.

cheers

Patch

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 10a0a54..2230e75 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -56,6 +56,16 @@  config IRQ_PER_CPU
 	bool
 	default y
 
+config NR_IRQS
+	int "Number of virtual interrupt numbers"
+	range 32 512
+	default "512"
+	help
+	  This defines the number of virtual interrupt numbers the kernel
+	  can manage. Virtual interrupt numbers are what you see in
+	  /proc/interrupts. If you configure your system to have too few,
+	  drivers will fail to load or worse - handle with care.
+
 config STACKTRACE_SUPPORT
 	bool
 	default y
diff --git a/arch/powerpc/include/asm/irq.h b/arch/powerpc/include/asm/irq.h
index bbcd1aa..b83fcc8 100644
--- a/arch/powerpc/include/asm/irq.h
+++ b/arch/powerpc/include/asm/irq.h
@@ -34,8 +34,8 @@  extern atomic_t ppc_n_lost_interrupts;
  */
 #define NO_IRQ_IGNORE		((unsigned int)-1)
 
-/* Total number of virq in the platform (make it a CONFIG_* option ? */
-#define NR_IRQS		512
+/* Total number of virq in the platform */
+#define NR_IRQS		CONFIG_NR_IRQS
 
 /* Number of irqs reserved for the legacy controller */
 #define NUM_ISA_INTERRUPTS	16