| Message ID | 1370644273-10495-11-git-send-email-yinghai@kernel.org |
|---|---|
| State | Not Applicable |
| Headers | show |
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@kernel.org> > Cc: Alexander Gordeev <agordeev@redhat.com> > --- > 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. > +} > EXPORT_SYMBOL_GPL(__irq_alloc_descs); > > /** > @@ -397,20 +418,7 @@ EXPORT_SYMBOL_GPL(__irq_alloc_descs); > */ > int irq_reserve_irqs(unsigned int from, unsigned int cnt) > { > - unsigned int start; > - int ret = 0; > - > - if (!cnt || (from + cnt) > nr_irqs) > - return -EINVAL; > - > - mutex_lock(&sparse_irq_lock); > - start = bitmap_find_next_zero_area(allocated_irqs, nr_irqs, from, cnt, 0); > - if (start == from) > - bitmap_set(allocated_irqs, start, cnt); > - else > - ret = -EEXIST; > - mutex_unlock(&sparse_irq_lock); > - return ret; > + return __irq_reserve_irqs(from, from, cnt); > } > > /** > -- > 1.8.1.4 >
On Mon, Jun 10, 2013 at 6:51 AM, Alexander Gordeev <agordeev@redhat.com> 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@kernel.org> >> Cc: Alexander Gordeev <agordeev@redhat.com> >> --- >> 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-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 7 Jun 2013, Yinghai Lu wrote: > /** > - * 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 Did you even bother to run docbook ? No you didn't. Otherwise you'd have noticed that documentation for __irq_reserve_descs is not the right thing for a function named __irq_reserve_irqs. > + * __irq_reserve_descs - reserve and initialize a range of irq descriptors The function only reserves a range of irq descriptors and does not initialize them. > + > +/** > + * irq_alloc_descs - allocate and initialize a range of irq descriptors Again you document non matching function names. > +int __ref > +__irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node, > + struct module *owner) Sloppy as usual along with a sucky changelog as usual..... Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 10, 2013 at 12:16:16PM -0700, Yinghai Lu wrote: > On Mon, Jun 10, 2013 at 6:51 AM, Alexander Gordeev <agordeev@redhat.com> wrote: > >> +/** > >> + * 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. I rather meant the bits should be unset in case alloc_descs() failed. But I failed to notice alloc_descs() does it. Therefore.. Reviewed-by: Alexander Gordeev <agordeev@redhat.com> > Thanks > > Yinghai
On Mon, Jun 10, 2013 at 12:39 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > On Fri, 7 Jun 2013, Yinghai Lu wrote: >> /** >> - * 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 > > Did you even bother to run docbook ? > > No you didn't. Otherwise you'd have noticed that documentation for > __irq_reserve_descs is not the right thing for a function named > __irq_reserve_irqs. sorry, will them consistent. Thanks Yinghai -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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); +} EXPORT_SYMBOL_GPL(__irq_alloc_descs); /** @@ -397,20 +418,7 @@ EXPORT_SYMBOL_GPL(__irq_alloc_descs); */ int irq_reserve_irqs(unsigned int from, unsigned int cnt) { - unsigned int start; - int ret = 0; - - if (!cnt || (from + cnt) > nr_irqs) - return -EINVAL; - - mutex_lock(&sparse_irq_lock); - start = bitmap_find_next_zero_area(allocated_irqs, nr_irqs, from, cnt, 0); - if (start == from) - bitmap_set(allocated_irqs, start, cnt); - else - ret = -EEXIST; - mutex_unlock(&sparse_irq_lock); - return ret; + return __irq_reserve_irqs(from, from, cnt); } /**
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@kernel.org> Cc: Alexander Gordeev <agordeev@redhat.com> --- include/linux/irq.h | 1 + kernel/irq/irqdesc.c | 54 ++++++++++++++++++++++++++++++---------------------- 2 files changed, 32 insertions(+), 23 deletions(-)