Re: [PATCH v3 10/27] genirq: Split __irq_reserve_irqs from irq_alloc_descs

From: Yinghai Lu
Date: Mon Jun 10 2013 - 15:16:42 EST


On Mon, Jun 10, 2013 at 6:51 AM, Alexander Gordeev <agordeev@xxxxxxxxxx> wrote:
> On Fri, Jun 07, 2013 at 03:30:56PM -0700, Yinghai Lu wrote:
>> irq_alloc_descs and irq_reserve_irqs are almost the same.
>> Separate code out to __irq_reserved_irqs, and other two reuse
>> __irq_reserve_irqs.
>>
>> We will use __irq_reserve_irqs for coming ioapic hotplug support.
>>
>> Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx>
>> Cc: Alexander Gordeev <agordeev@xxxxxxxxxx>
>> ---
>> include/linux/irq.h | 1 +
>> kernel/irq/irqdesc.c | 54 ++++++++++++++++++++++++++++++----------------------
>> 2 files changed, 32 insertions(+), 23 deletions(-)
>>
>> diff --git a/include/linux/irq.h b/include/linux/irq.h
>> index bc4e066..4e0fcbb 100644
>> --- a/include/linux/irq.h
>> +++ b/include/linux/irq.h
>> @@ -600,6 +600,7 @@ int __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
>>
>> void irq_free_descs(unsigned int irq, unsigned int cnt);
>> int irq_reserve_irqs(unsigned int from, unsigned int cnt);
>> +int __irq_reserve_irqs(int irq, unsigned int from, unsigned int cnt);
>>
>> static inline void irq_free_desc(unsigned int irq)
>> {
>> diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
>> index 192a302..3b9fb92 100644
>> --- a/kernel/irq/irqdesc.c
>> +++ b/kernel/irq/irqdesc.c
>> @@ -340,18 +340,15 @@ void irq_free_descs(unsigned int from, unsigned int cnt)
>> EXPORT_SYMBOL_GPL(irq_free_descs);
>>
>> /**
>> - * irq_alloc_descs - allocate and initialize a range of irq descriptors
>> - * @irq: Allocate for specific irq number if irq >= 0
>> + * __irq_reserve_descs - reserve and initialize a range of irq descriptors
>> + * @irq: Reserve for specific irq number if irq >= 0
>> * @from: Start the search from this irq number
>> - * @cnt: Number of consecutive irqs to allocate.
>> - * @node: Preferred node on which the irq descriptor should be allocated
>> - * @owner: Owning module (can be NULL)
>> + * @cnt: Number of consecutive irqs to reserve.
>> *
>> * Returns the first irq number or error code
>> */
>> int __ref
>> -__irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
>> - struct module *owner)
>> +__irq_reserve_irqs(int irq, unsigned int from, unsigned int cnt)
>> {
>> int start, ret;
>>
>> @@ -369,7 +366,7 @@ __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
>> start = bitmap_find_next_zero_area(allocated_irqs, IRQ_BITMAP_BITS,
>> from, cnt, 0);
>> ret = -EEXIST;
>> - if (irq >=0 && start != irq)
>> + if (irq >= 0 && start != irq)
>> goto err;
>>
>> if (start + cnt > nr_irqs) {
>> @@ -380,12 +377,36 @@ __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
>>
>> bitmap_set(allocated_irqs, start, cnt);
>> mutex_unlock(&sparse_irq_lock);
>> - return alloc_descs(start, cnt, node, owner);
>> + return start;
>>
>> err:
>> mutex_unlock(&sparse_irq_lock);
>> return ret;
>> }
>> +
>> +/**
>> + * irq_alloc_descs - allocate and initialize a range of irq descriptors
>> + * @irq: Allocate for specific irq number if irq >= 0
>> + * @from: Start the search from this irq number
>> + * @cnt: Number of consecutive irqs to allocate.
>> + * @node: Preferred node on which the irq descriptor should be allocated
>> + * @owner: Owning module (can be NULL)
>> + *
>> + * Returns the first irq number or error code
>> + */
>> +int __ref
>> +__irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
>> + struct module *owner)
>> +{
>> + int start;
>> +
>> + start = __irq_reserve_irqs(irq, from, cnt);
>> +
>> + if (start < 0)
>> + return start;
>> +
>> + return alloc_descs(start, cnt, node, owner);
>
> I think alloc_descs() fail path is needed before return.

__irq_reserve_irqs already return -EEXIST etc,

old kernel is like:

>> @@ -380,12 +377,36 @@ __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,

bitmap_set(allocated_irqs, start, cnt);
mutex_unlock(&sparse_irq_lock);
return alloc_descs(start, cnt, node, owner);

err:
mutex_unlock(&sparse_irq_lock);
return ret;

so i don't change the fail path handling.

Thanks

Yinghai
--
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/