* [RFC PATCH 1/2] irq_domain: correct a minor wrong comment for linear revmap @ 2012-06-20 9:00 Dong Aisheng 2012-06-20 9:00 ` [RFC PATCH 2/2] irq: add irq_desc_initialize to remove some duplicated lines Dong Aisheng 2012-07-11 14:07 ` [RFC PATCH 1/2] irq_domain: correct a minor wrong comment for linear revmap Grant Likely 0 siblings, 2 replies; 6+ messages in thread From: Dong Aisheng @ 2012-06-20 9:00 UTC (permalink / raw) To: linux-arm-kernel From: Dong Aisheng <dong.aisheng@linaro.org> The revmap type should be linear for irq_domain_add_linear function. Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org> --- kernel/irq/irqdomain.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c index 3e4ea85..cb83554 100644 --- a/kernel/irq/irqdomain.c +++ b/kernel/irq/irqdomain.c @@ -219,7 +219,7 @@ struct irq_domain *irq_domain_add_legacy(struct device_node *of_node, EXPORT_SYMBOL_GPL(irq_domain_add_legacy); /** - * irq_domain_add_linear() - Allocate and register a legacy revmap irq_domain. + * irq_domain_add_linear() - Allocate and register a linear revmap irq_domain. * @of_node: pointer to interrupt controller's device tree node. * @size: Number of interrupts in the domain. * @ops: map/unmap domain callbacks -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RFC PATCH 2/2] irq: add irq_desc_initialize to remove some duplicated lines 2012-06-20 9:00 [RFC PATCH 1/2] irq_domain: correct a minor wrong comment for linear revmap Dong Aisheng @ 2012-06-20 9:00 ` Dong Aisheng 2012-07-06 9:18 ` Dong Aisheng 2012-07-11 22:19 ` Thomas Gleixner 2012-07-11 14:07 ` [RFC PATCH 1/2] irq_domain: correct a minor wrong comment for linear revmap Grant Likely 1 sibling, 2 replies; 6+ messages in thread From: Dong Aisheng @ 2012-06-20 9:00 UTC (permalink / raw) To: linux-arm-kernel From: Dong Aisheng <dong.aisheng@linaro.org> There're two copies of irq_desc initialization code, reform them into an irq_desc_initialize function to call. Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org> --- kernel/irq/irqdesc.c | 51 +++++++++++++++++++++++++++---------------------- 1 files changed, 28 insertions(+), 23 deletions(-) diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c index 192a302..e29db67 100644 --- a/kernel/irq/irqdesc.c +++ b/kernel/irq/irqdesc.c @@ -131,34 +131,43 @@ static void free_masks(struct irq_desc *desc) static inline void free_masks(struct irq_desc *desc) { } #endif -static struct irq_desc *alloc_desc(int irq, int node, struct module *owner) +static inline int irq_desc_initialize(struct irq_desc *desc, + int irq, int node, struct module *owner) { - struct irq_desc *desc; - gfp_t gfp = GFP_KERNEL; - - desc = kzalloc_node(sizeof(*desc), gfp, node); - if (!desc) - return NULL; /* allocate based on nr_cpu_ids */ desc->kstat_irqs = alloc_percpu(unsigned int); if (!desc->kstat_irqs) - goto err_desc; + return -ENOMEM; - if (alloc_masks(desc, gfp, node)) - goto err_kstat; + if (alloc_masks(desc, GFP_KERNEL, node)) { + kfree(desc->kstat_irqs); + return -ENOMEM; + } raw_spin_lock_init(&desc->lock); lockdep_set_class(&desc->lock, &irq_desc_lock_class); desc_set_defaults(irq, desc, node, owner); - return desc; + return 0; +} -err_kstat: - free_percpu(desc->kstat_irqs); -err_desc: - kfree(desc); - return NULL; +static struct irq_desc *alloc_desc(int irq, int node, struct module *owner) +{ + struct irq_desc *desc; + int ret; + + desc = kzalloc_node(sizeof(*desc), GFP_KERNEL, node); + if (!desc) + return NULL; + + ret = irq_desc_initialize(desc, irq, node, owner); + if (ret) { + kfree(desc); + return NULL; + } + + return desc; } static void free_desc(unsigned int irq) @@ -260,13 +269,9 @@ int __init early_irq_init(void) desc = irq_desc; count = ARRAY_SIZE(irq_desc); - for (i = 0; i < count; i++) { - desc[i].kstat_irqs = alloc_percpu(unsigned int); - alloc_masks(&desc[i], GFP_KERNEL, node); - raw_spin_lock_init(&desc[i].lock); - lockdep_set_class(&desc[i].lock, &irq_desc_lock_class); - desc_set_defaults(i, &desc[i], node, NULL); - } + for (i = 0; i < count; i++) + irq_desc_initialize(desc, irq, node, NULL); + return arch_early_irq_init(); } -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RFC PATCH 2/2] irq: add irq_desc_initialize to remove some duplicated lines 2012-06-20 9:00 ` [RFC PATCH 2/2] irq: add irq_desc_initialize to remove some duplicated lines Dong Aisheng @ 2012-07-06 9:18 ` Dong Aisheng 2012-07-11 22:19 ` Thomas Gleixner 1 sibling, 0 replies; 6+ messages in thread From: Dong Aisheng @ 2012-07-06 9:18 UTC (permalink / raw) To: linux-arm-kernel On Wed, Jun 20, 2012 at 05:00:31PM +0800, Dong Aisheng wrote: > From: Dong Aisheng <dong.aisheng@linaro.org> > > There're two copies of irq_desc initialization code, reform them into > an irq_desc_initialize function to call. > > Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org> > --- > kernel/irq/irqdesc.c | 51 +++++++++++++++++++++++++++---------------------- > 1 files changed, 28 insertions(+), 23 deletions(-) > Ping... Regards Dong Aisheng > diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c > index 192a302..e29db67 100644 > --- a/kernel/irq/irqdesc.c > +++ b/kernel/irq/irqdesc.c > @@ -131,34 +131,43 @@ static void free_masks(struct irq_desc *desc) > static inline void free_masks(struct irq_desc *desc) { } > #endif > > -static struct irq_desc *alloc_desc(int irq, int node, struct module *owner) > +static inline int irq_desc_initialize(struct irq_desc *desc, > + int irq, int node, struct module *owner) > { > - struct irq_desc *desc; > - gfp_t gfp = GFP_KERNEL; > - > - desc = kzalloc_node(sizeof(*desc), gfp, node); > - if (!desc) > - return NULL; > /* allocate based on nr_cpu_ids */ > desc->kstat_irqs = alloc_percpu(unsigned int); > if (!desc->kstat_irqs) > - goto err_desc; > + return -ENOMEM; > > - if (alloc_masks(desc, gfp, node)) > - goto err_kstat; > + if (alloc_masks(desc, GFP_KERNEL, node)) { > + kfree(desc->kstat_irqs); > + return -ENOMEM; > + } > > raw_spin_lock_init(&desc->lock); > lockdep_set_class(&desc->lock, &irq_desc_lock_class); > > desc_set_defaults(irq, desc, node, owner); > > - return desc; > + return 0; > +} > > -err_kstat: > - free_percpu(desc->kstat_irqs); > -err_desc: > - kfree(desc); > - return NULL; > +static struct irq_desc *alloc_desc(int irq, int node, struct module *owner) > +{ > + struct irq_desc *desc; > + int ret; > + > + desc = kzalloc_node(sizeof(*desc), GFP_KERNEL, node); > + if (!desc) > + return NULL; > + > + ret = irq_desc_initialize(desc, irq, node, owner); > + if (ret) { > + kfree(desc); > + return NULL; > + } > + > + return desc; > } > > static void free_desc(unsigned int irq) > @@ -260,13 +269,9 @@ int __init early_irq_init(void) > desc = irq_desc; > count = ARRAY_SIZE(irq_desc); > > - for (i = 0; i < count; i++) { > - desc[i].kstat_irqs = alloc_percpu(unsigned int); > - alloc_masks(&desc[i], GFP_KERNEL, node); > - raw_spin_lock_init(&desc[i].lock); > - lockdep_set_class(&desc[i].lock, &irq_desc_lock_class); > - desc_set_defaults(i, &desc[i], node, NULL); > - } > + for (i = 0; i < count; i++) > + irq_desc_initialize(desc, irq, node, NULL); > + > return arch_early_irq_init(); > } > > -- > 1.7.0.4 > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC PATCH 2/2] irq: add irq_desc_initialize to remove some duplicated lines 2012-06-20 9:00 ` [RFC PATCH 2/2] irq: add irq_desc_initialize to remove some duplicated lines Dong Aisheng 2012-07-06 9:18 ` Dong Aisheng @ 2012-07-11 22:19 ` Thomas Gleixner 2012-07-12 3:26 ` Dong Aisheng 1 sibling, 1 reply; 6+ messages in thread From: Thomas Gleixner @ 2012-07-11 22:19 UTC (permalink / raw) To: linux-arm-kernel On Wed, 20 Jun 2012, Dong Aisheng wrote: > From: Dong Aisheng <dong.aisheng@linaro.org> > > There're two copies of irq_desc initialization code, reform them into > an irq_desc_initialize function to call. > > Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org> > --- > kernel/irq/irqdesc.c | 51 +++++++++++++++++++++++++++---------------------- > 1 files changed, 28 insertions(+), 23 deletions(-) We add more code by removing redundant copies? ^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC PATCH 2/2] irq: add irq_desc_initialize to remove some duplicated lines 2012-07-11 22:19 ` Thomas Gleixner @ 2012-07-12 3:26 ` Dong Aisheng 0 siblings, 0 replies; 6+ messages in thread From: Dong Aisheng @ 2012-07-12 3:26 UTC (permalink / raw) To: linux-arm-kernel Hi Thomas, Thanks for the review firstly. On Thu, Jul 12, 2012 at 06:19:18AM +0800, Thomas Gleixner wrote: > On Wed, 20 Jun 2012, Dong Aisheng wrote: > > From: Dong Aisheng <dong.aisheng@linaro.org> > > > > There're two copies of irq_desc initialization code, reform them into > > an irq_desc_initialize function to call. > > > > Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org> > > --- > > kernel/irq/irqdesc.c | 51 +++++++++++++++++++++++++++---------------------- > > 1 files changed, 28 insertions(+), 23 deletions(-) > > We add more code by removing redundant copies? > I also had this strange question. I looked the code a bit more, i guess the main problem is that the redundant copies is not too big, so we can not see great savings. Compare to the init code of irq_desc in original alloc_desc function, the new irq_desc_initialize function saves 4 lines. However, the new function also add 4 lines for defining extra function name, parameter and etc. And alloc_desc still needs to call irq_desc_initialize and checking return value which needs extra 6 lines. The main saving is another copy of irq_desc initialization in early_irq_init, but this copy does not check any return values which cause we did not save too much, only about 4 lines. Plus extra blan lines added, so totally it does not save more than new added. However, i was thinking it could make code looks a bit better. So i sent out this RFC patch. Do you think if it's reasonable? BTW, there's an issue in my patch, should change like: if (alloc_masks(desc, GFP_KERNEL, node)) { - kfree(desc->kstat_irqs); + free_percpu(desc->kstat_irqs); return -ENOMEM; } Regards Dong Aisheng ^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC PATCH 1/2] irq_domain: correct a minor wrong comment for linear revmap 2012-06-20 9:00 [RFC PATCH 1/2] irq_domain: correct a minor wrong comment for linear revmap Dong Aisheng 2012-06-20 9:00 ` [RFC PATCH 2/2] irq: add irq_desc_initialize to remove some duplicated lines Dong Aisheng @ 2012-07-11 14:07 ` Grant Likely 1 sibling, 0 replies; 6+ messages in thread From: Grant Likely @ 2012-07-11 14:07 UTC (permalink / raw) To: linux-arm-kernel On Wed, 20 Jun 2012 17:00:30 +0800, Dong Aisheng <b29396@freescale.com> wrote: > From: Dong Aisheng <dong.aisheng@linaro.org> > > The revmap type should be linear for irq_domain_add_linear function. > > Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org> Applied, thanks. g. > --- > kernel/irq/irqdomain.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c > index 3e4ea85..cb83554 100644 > --- a/kernel/irq/irqdomain.c > +++ b/kernel/irq/irqdomain.c > @@ -219,7 +219,7 @@ struct irq_domain *irq_domain_add_legacy(struct device_node *of_node, > EXPORT_SYMBOL_GPL(irq_domain_add_legacy); > > /** > - * irq_domain_add_linear() - Allocate and register a legacy revmap irq_domain. > + * irq_domain_add_linear() - Allocate and register a linear revmap irq_domain. > * @of_node: pointer to interrupt controller's device tree node. > * @size: Number of interrupts in the domain. > * @ops: map/unmap domain callbacks > -- > 1.7.0.4 > > -- Grant Likely, B.Sc, P.Eng. Secret Lab Technologies, Ltd. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-07-12 3:26 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-06-20 9:00 [RFC PATCH 1/2] irq_domain: correct a minor wrong comment for linear revmap Dong Aisheng 2012-06-20 9:00 ` [RFC PATCH 2/2] irq: add irq_desc_initialize to remove some duplicated lines Dong Aisheng 2012-07-06 9:18 ` Dong Aisheng 2012-07-11 22:19 ` Thomas Gleixner 2012-07-12 3:26 ` Dong Aisheng 2012-07-11 14:07 ` [RFC PATCH 1/2] irq_domain: correct a minor wrong comment for linear revmap Grant Likely
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).