[question] NR_IRQS in genirq

[question] NR_IRQS in genirq

[Haojian Zhuang]

Hi all,

I'm using the latest kernel 2.6.37-rc1. Now I met some issues on genirq.

1. While SPARSE IRQ is enabled, nr_irqs may be larger than NR_IRQS.
But the allocated_irqs bitmap (kernel/irq/irqdesc.c) is restricted in
NR_IRQS. Is it an issue?

2. irqs_resend bitmap of kernel/irq/resend.c is also restricted in
NR_IRQS. Is it an issue, too?

Thanks
Haojian

Re: [question] NR_IRQS in genirq

[Paul Mundt]

On Wed, Nov 24, 2010 at 03:28:37PM +0800, Haojian Zhuang wrote:
> Hi all,
> 
> I'm using the latest kernel 2.6.37-rc1. Now I met some issues on genirq.
> 
> 1. While SPARSE IRQ is enabled, nr_irqs may be larger than NR_IRQS.
> But the allocated_irqs bitmap (kernel/irq/irqdesc.c) is restricted in
> NR_IRQS. Is it an issue?
> 
> 2. irqs_resend bitmap of kernel/irq/resend.c is also restricted in
> NR_IRQS. Is it an issue, too?
> 
Perhaps something like:

diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 9988d03..11bd76c 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -215,6 +215,11 @@ int __init early_irq_init(void)
 	initcnt = arch_probe_nr_irqs();
 	printk(KERN_INFO "NR_IRQS:%d nr_irqs:%d %d\n", NR_IRQS, nr_irqs, initcnt);
 
+	if (unlikely(nr_irqs > NR_IRQS)) {
+		WARN(1, "Probed more than NR_IRQS, chomping.\n");
+		nr_irqs = NR_IRQS;
+	}
+
 	for (i = 0; i < initcnt; i++) {
 		desc = alloc_desc(i, node);
 		set_bit(i, allocated_irqs);

?

Re: [question] NR_IRQS in genirq

[Thomas Gleixner]

On Wed, 24 Nov 2010, Haojian Zhuang wrote:

> Hi all,
> 
> I'm using the latest kernel 2.6.37-rc1. Now I met some issues on genirq.
> 
> 1. While SPARSE IRQ is enabled, nr_irqs may be larger than NR_IRQS.
> But the allocated_irqs bitmap (kernel/irq/irqdesc.c) is restricted in
> NR_IRQS. Is it an issue?

Why can nr_irqs become larger? Is that a theoretical problem or did
you run into this ?

Thanks,

	tglx

Re: [question] NR_IRQS in genirq

[Haojian Zhuang]

On Wed, Nov 24, 2010 at 9:18 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Wed, 24 Nov 2010, Haojian Zhuang wrote:
>
>> Hi all,
>>
>> I'm using the latest kernel 2.6.37-rc1. Now I met some issues on genirq.
>>
>> 1. While SPARSE IRQ is enabled, nr_irqs may be larger than NR_IRQS.
>> But the allocated_irqs bitmap (kernel/irq/irqdesc.c) is restricted in
>> NR_IRQS. Is it an issue?
>
> Why can nr_irqs become larger? Is that a theoretical problem or did
> you run into this ?
>
>

Hi Thomas,

My hardware environment is ARM. Each machine description can specify
nr_irqs. In my implementation of PXA, NR_IRQS is fixed for SoC
internal IRQs. And there's some additional board IRQs, we arrange them
between NR_IRQS and nr_irqs. So nr_irqs will be larger than NR_IRQS if
board IRQs exists.

We use dynamic board IRQs since each machine has different
requirement. And CONFIG_HARDIRQ_SW_RESEND is occasionally, I actually
met issue in resend_irqs because of accessing bitmap memory out of
bound.

Do you mean that we shouldn't use SPARSE IRQ by this way?

Best Regards
Haojian

Re: [question] NR_IRQS in genirq

[Mark Brown]

On Wed, Nov 24, 2010 at 09:46:06PM +0800, Haojian Zhuang wrote:

> My hardware environment is ARM. Each machine description can specify
> nr_irqs. In my implementation of PXA, NR_IRQS is fixed for SoC
> internal IRQs. And there's some additional board IRQs, we arrange them
> between NR_IRQS and nr_irqs. So nr_irqs will be larger than NR_IRQS if
> board IRQs exists.

Most ARM platforms have come up with some Kconfig gunk to allow boards
to extend this for off-SoC GPIOs.  It'd be really nice to get rid of
NR_IRQS and stop having to worry about this at all :(

Re: [question] NR_IRQS in genirq

[Thomas Gleixner]

On Wed, 24 Nov 2010, Haojian Zhuang wrote:
> On Wed, Nov 24, 2010 at 9:18 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > Why can nr_irqs become larger? Is that a theoretical problem or did
> > you run into this ?
> 
> My hardware environment is ARM. Each machine description can specify
> nr_irqs. In my implementation of PXA, NR_IRQS is fixed for SoC
> internal IRQs. And there's some additional board IRQs, we arrange them
> between NR_IRQS and nr_irqs. So nr_irqs will be larger than NR_IRQS if
> board IRQs exists.

And that's wrong. NR_IRQS is the upper bound. nr_irqs is the runtime
bound which is supposed to be <= NR_IRQS.

The whole point of sparse_irq is that it does not statically allocate
irq_desc[NR_IRQS] to reduce memory consumption if only a small number
of irqs are actuallt used by a specific board.

Thanks,

	tglx

Re: [question] NR_IRQS in genirq

[Thomas Gleixner]

On Wed, 24 Nov 2010, Mark Brown wrote:

> On Wed, Nov 24, 2010 at 09:46:06PM +0800, Haojian Zhuang wrote:
> 
> > My hardware environment is ARM. Each machine description can specify
> > nr_irqs. In my implementation of PXA, NR_IRQS is fixed for SoC
> > internal IRQs. And there's some additional board IRQs, we arrange them
> > between NR_IRQS and nr_irqs. So nr_irqs will be larger than NR_IRQS if
> > board IRQs exists.
> 
> Most ARM platforms have come up with some Kconfig gunk to allow boards
> to extend this for off-SoC GPIOs.  It'd be really nice to get rid of
> NR_IRQS and stop having to worry about this at all :(

I mean with sparse_irq you can set NR_IRQS insanely high w/o
increasing memory consumption. That's the whole point.

Thanks,

	tglx

Re: [question] NR_IRQS in genirq

[Thomas Gleixner]

On Wed, 24 Nov 2010, Paul Mundt wrote:

> On Wed, Nov 24, 2010 at 03:28:37PM +0800, Haojian Zhuang wrote:
> > Hi all,
> > 
> > I'm using the latest kernel 2.6.37-rc1. Now I met some issues on genirq.
> > 
> > 1. While SPARSE IRQ is enabled, nr_irqs may be larger than NR_IRQS.
> > But the allocated_irqs bitmap (kernel/irq/irqdesc.c) is restricted in
> > NR_IRQS. Is it an issue?
> > 
> > 2. irqs_resend bitmap of kernel/irq/resend.c is also restricted in
> > NR_IRQS. Is it an issue, too?
> > 
> Perhaps something like:
> 
> diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
> index 9988d03..11bd76c 100644
> --- a/kernel/irq/irqdesc.c
> +++ b/kernel/irq/irqdesc.c
> @@ -215,6 +215,11 @@ int __init early_irq_init(void)
>  	initcnt = arch_probe_nr_irqs();
>  	printk(KERN_INFO "NR_IRQS:%d nr_irqs:%d %d\n", NR_IRQS, nr_irqs, initcnt);
>  
> +	if (unlikely(nr_irqs > NR_IRQS)) {
> +		WARN(1, "Probed more than NR_IRQS, chomping.\n");
> +		nr_irqs = NR_IRQS;
> +	}

Yep, we need that sanity check, but the WARN() is overkill as we
already know the call chain :)

Thanks,

	tglx

Re: [question] NR_IRQS in genirq

[Mark Brown]

On Wed, Nov 24, 2010 at 02:54:38PM +0100, Thomas Gleixner wrote:
> On Wed, 24 Nov 2010, Mark Brown wrote:
> > On Wed, Nov 24, 2010 at 09:46:06PM +0800, Haojian Zhuang wrote:

> > Most ARM platforms have come up with some Kconfig gunk to allow boards
> > to extend this for off-SoC GPIOs.  It'd be really nice to get rid of
> > NR_IRQS and stop having to worry about this at all :(

> I mean with sparse_irq you can set NR_IRQS insanely high w/o
> increasing memory consumption. That's the whole point.

Yeah, I was just pointing out common practice on ARM (sparse IRQ isn't
widely enough deployed there :/ ).

Would it be worth having sparse_irq change the default NR_IRQS to be
something suitably large - there doesn't seem any point in having
platforms using it each pick their own particular definition of insanely
high?  I'll take a look and cook up a patch unless I can spot anything
silly about that by myself.