On Fri, 8 Oct 2021 01:32:20 -0700
gavinli@xxxxxxxxxxxxxx wrote:
> From: Gavin Li <gavin@xxxxxxxxxxxx>
>
> viio_trigger_alloc() includes a get_device() call added in commit
> 5f9c035cae back in 2011. While I wasn't able to ascertain why it was
> added, I've noticed that it does cause a memory leak, especially when
> rmmod'ing iio modules. Here is a simple module that replicates this
> issue:
>
> #include <linux/module.h>
> #include <linux/iio/iio.h>
> #include <linux/iio/trigger.h>
>
> int my_init(void) {
> struct iio_trigger *trig = iio_trigger_alloc("leak-trig");
> if (!trig)
> return -ENOMEM;
>
> printk("iio-leak: %u uses at A\n", kref_read(&trig->dev.kobj.kref));
> iio_trigger_free(trig);
> printk("iio-leak: %u uses at B\n", kref_read(&trig->dev.kobj.kref));
>
> return 0;
> }
>
> void my_exit(void) {}
>
> module_init(my_init);
> module_exit(my_exit);
> MODULE_LICENSE("GPL v2");
>
> It prints the following in the kernel log:
>
> iio-leak: 2 uses at A
> iio-leak: 1 uses at B
>
> This patch removes the get_device() call. This shouldn't break
> subirqs with iio_trigger_attach_poll_func() because a reference should
> already exist via indio_dev->trig.
Agreed, it is most mysterious... I can't apply this patch though without a
Signed-off-by: ...
as even though it's trivial we need a developer certificate of origin.
(represented by the Signed-off-by line)
Thanks,
Jonathan
> ---
> drivers/iio/industrialio-trigger.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
> index b23caa2f2aa1f..93990ff1dfe39 100644
> --- a/drivers/iio/industrialio-trigger.c
> +++ b/drivers/iio/industrialio-trigger.c
> @@ -556,7 +556,6 @@ struct iio_trigger *viio_trigger_alloc(struct device *parent,
> irq_modify_status(trig->subirq_base + i,
> IRQ_NOREQUEST | IRQ_NOAUTOEN, IRQ_NOPROBE);
> }
> - get_device(&trig->dev);
>
> return trig;
>
