From: Jean Delvare <khali@linux-fr.org>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-kernel@vger.kernel.org, Peter Wu <lekensteyn@gmail.com>
Subject: Freeing of dev->p
Date: Wed, 8 Jan 2014 16:40:58 +0100 [thread overview]
Message-ID: <20140108164058.059cf461@endymion.delvare> (raw)
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
next reply other threads:[~2014-01-08 15:41 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-01-08 15:40 Jean Delvare [this message]
2014-01-08 16:56 ` Freeing of dev->p Greg Kroah-Hartman
2014-01-08 20:33 ` Jean Delvare
2014-01-10 4:18 ` Greg Kroah-Hartman
2014-01-10 14:39 ` Jean Delvare
2014-01-10 15:24 ` Greg Kroah-Hartman
2014-01-10 22:05 ` Jean Delvare
2014-01-22 7:29 ` Jean Delvare
2014-01-25 18:03 ` Greg Kroah-Hartman
2014-04-08 9:47 ` Grant Likely
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140108164058.059cf461@endymion.delvare \
--to=khali@linux-fr.org \
--cc=gregkh@linuxfoundation.org \
--cc=lekensteyn@gmail.com \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox