Re: genirq: Ensure we locate the passed IRQ in irq_alloc_descs()

[Mark Brown]

On Fri, Jun 03, 2011 at 04:24:02AM -0500, Milton Miller wrote:
> On Thu, 02 Jun 2011 17:55:13 -0000, Mark Brown wrote:

> >  	start = bitmap_find_next_zero_area(allocated_irqs, IRQ_BITMAP_BITS,

> and then right after this the code continues:

>         ret = -EEXIST;
>         if (irq >=0 && start != irq)
>                 goto err;

> This patch enables exactly the calls I want to forbid !  Why do

Which you wish to forbid because...?  You've not articulated any
motivation for doing this which makes it rather hard to engage here.

> you need to verify that there are no irqs between from and irq ?

I don't care if there are IRQs between from and the specified irq, all I
care about is that we get back the irq we requested (apart from anything
else the function will later error out if the allocated IRQ doesn't
match the one that was specified - it seems very clear from both the
code and documentation that if an IRQ is specified we're supposed to get
it back).

 - The specified IRQ is ignored except 

> What is your use case?

I've requested a base IRQ but the only attention that irq_alloc_descs()
is paying to it is that it generates an error rather than allocating
something 

> Change your caller to specify the irq twice if you need a specific irq

This seems like a poor UI for the function, if the user specified an
irq_base and there's a suitable range of IRQs available at that base 
what is the benefit in refusing to allocate there?  That's just going to
make the system fragile against init ordering and driver disabling.

It's also going to be a bit more cumbersome to use:

	if (pdata->irq_base > 0) {
		irq_base = pdata->irq_base;
		from = pdata->irq_base;
	} else {
		irq_base = -1;
		from = 0;
	}

> block, or if you only need one then use the helper irq_alloc_desc_at.

I need about 60 IRQs in the particular driver where I noticed this.

> If you want to change irq_alloc_descs, please make it return -EINVAL
> if irq >=0 && from != irq (like I did).

> See http://lkml.indiana.edu/hypermail/linux/kernel/1105.3/00739.html
> [PATCH RFC 4/4] irq: allow a per-allocation upper limit when allocating irqs

> (and yes, I have made the changes based on the feedback but haven't

I don't really see the relevance of this patch?  You're adding
functionality for limiting the maximum IRQ number allocated which seems
orthogonal to the issue.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/