Date:	Wed, 8 Jan 2014 16:40:58 +0100
From:	Jean Delvare <khali@...ux-fr.org>
To:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:	linux-kernel@...r.kernel.org, Peter Wu <lekensteyn@...il.com>
Subject: Freeing of dev->p

Hi Greg, hi all,

A memory leak has been reported to me:
http://marc.info/?l=linux-i2c&m=138779165123331&w=2

The leak is in i801_probe, caused by an early call to
i2c_set_adapdata() which in turn calls dev_set_drvdata() which
allocates some memory in device_private_init(). That memory is only
freed by the driver core when the i2c_adapter class device is removed.
But if the parent (PCI) device probing itself fails for whatever
reason, the class device never gets to be created, so it's never
removed, thus the memory is never freed.

It is not possible to move the call to i2c_set_adapdata() until after
the class device is created, because the data pointed is needed very
early after (almost during) i2c adapter creation. So I could make the
leak less likely to happen but I can't fix it completely.

I am wondering how this can be solved, and this brought three questions:

1* What is the rationale for allocating dev->p dynamically? It is
allocated as soon as the device is created (in device_add), so as far
as I can see every device will need the allocation. Including struct
device_private in struct device would not cost more memory, plus it
would reduce memory fragmentation. So is this a lifetime issue? Or
something else I can't think of?

2* What is the rationale for making void *driver_data part of struct
device_private and not struct device itself?

3* If the current implementation is considered correct, does it mean
that dev_set_drvdata() should never be used for class devices?

Thanks,
-- 
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists