2017-03-08 17:11:19

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v2] irq: generic-chip: provide irq_free_generic_chip()

Some users of irq_alloc_generic_chip() are modules which can be
removed (e.g. gpio-ml-ioh) but have no means of freeing the allocated
generic chip. Provide a function for that.

Signed-off-by: Bartosz Golaszewski <[email protected]>
---
v1 -> v2:
- added the kernel doc

include/linux/irq.h | 1 +
kernel/irq/generic-chip.c | 12 ++++++++++++
2 files changed, 13 insertions(+)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index f887351..04e5120 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -950,6 +950,7 @@ void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
int irq_setup_alt_chip(struct irq_data *d, unsigned int type);
void irq_remove_generic_chip(struct irq_chip_generic *gc, u32 msk,
unsigned int clr, unsigned int set);
+void irq_free_generic_chip(struct irq_chip_generic *gc);

struct irq_chip_generic *irq_get_domain_generic_chip(struct irq_domain *d, unsigned int hw_irq);

diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index ee32870..242836e 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -545,6 +545,18 @@ void irq_remove_generic_chip(struct irq_chip_generic *gc, u32 msk,
}
EXPORT_SYMBOL_GPL(irq_remove_generic_chip);

+/**
+ * irq_free_generic_chip - Free the memory allocated for a chip
+ * @gc: Generic irq chip
+ *
+ * Free the data previously allocated by irq_allocate_generic_chip().
+ */
+void irq_free_generic_chip(struct irq_chip_generic *gc)
+{
+ kfree(gc);
+}
+EXPORT_SYMBOL_GPL(irq_free_generic_chip);
+
static struct irq_data *irq_gc_get_irq_data(struct irq_chip_generic *gc)
{
unsigned int virq;
--
2.9.3


2017-03-09 08:07:18

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v2] irq: generic-chip: provide irq_free_generic_chip()

On Wed, Mar 08 2017 at 5:04:05 pm GMT, Bartosz Golaszewski <[email protected]> wrote:
> Some users of irq_alloc_generic_chip() are modules which can be
> removed (e.g. gpio-ml-ioh) but have no means of freeing the allocated
> generic chip. Provide a function for that.
>
> Signed-off-by: Bartosz Golaszewski <[email protected]>

Acked-by: Marc Zyngier <[email protected]>

M.
--
Jazz is not dead, it just smell funny.

2017-03-09 10:17:36

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH v2] irq: generic-chip: provide irq_free_generic_chip()

On Wed, 8 Mar 2017, Bartosz Golaszewski wrote:

> Some users of irq_alloc_generic_chip() are modules which can be
> removed (e.g. gpio-ml-ioh) but have no means of freeing the allocated
> generic chip. Provide a function for that.

They have means, i.e. kfree(gc). If you want a wrapper for that then please
make it a simple static inline w/o the bloat of an exported symbol.

But, what you really want is a proper counterpart to
irq_alloc_generic_chip() which undoes everything what
irq_alloc_generic_chip() does.

You have to call irq_remove_generic_chip() before calling kfree() or the
wrapper anyway. And looking at the 3 users of irq_remove_generic_chip():

Two of them (drivers/gpio/gpio-tb10x.c drivers/mfd/jz4740-adc.c) do

irq_remove_generic_chip(gc)
kfree(gc)

The third one (arch/arm/mach-omap2/prm_common.c) leaks the generic chip
memory....

So the proper solution is to rename irq_remove_generic_chip() to
irq_destroy_generic_chip() and do the kfree() at the end of that function.

Hmm?

tglx


2017-03-09 10:44:43

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH v2] irq: generic-chip: provide irq_free_generic_chip()

2017-03-09 11:17 GMT+01:00 Thomas Gleixner <[email protected]>:
> On Wed, 8 Mar 2017, Bartosz Golaszewski wrote:
>
>> Some users of irq_alloc_generic_chip() are modules which can be
>> removed (e.g. gpio-ml-ioh) but have no means of freeing the allocated
>> generic chip. Provide a function for that.
>
> They have means, i.e. kfree(gc). If you want a wrapper for that then please
> make it a simple static inline w/o the bloat of an exported symbol.
>

Yes, I want a wrapper, since there are no guarantees as to what
irq_alloc_generic_chip() does internally.

> But, what you really want is a proper counterpart to
> irq_alloc_generic_chip() which undoes everything what
> irq_alloc_generic_chip() does.
>
> You have to call irq_remove_generic_chip() before calling kfree() or the
> wrapper anyway. And looking at the 3 users of irq_remove_generic_chip():
>
> Two of them (drivers/gpio/gpio-tb10x.c drivers/mfd/jz4740-adc.c) do
>
> irq_remove_generic_chip(gc)
> kfree(gc)
>
> The third one (arch/arm/mach-omap2/prm_common.c) leaks the generic chip
> memory....
>
> So the proper solution is to rename irq_remove_generic_chip() to
> irq_destroy_generic_chip() and do the kfree() at the end of that function.
>
> Hmm?
>

Sounds good!

Thanks,
Bartosz