From: Jean Delvare <khali@linux-fr.org>
To: sensors@Stimpy.netroedge.com, linux-kernel@vger.kernel.org
Cc: greg@kroah.com, vsu@altlinux.ru,
"Mark M. Hoffman" <mhoffman@lightlink.com>
Subject: [PATCH 2.6] Fix memory leaks in w83781d and asb100
Date: Sun, 2 May 2004 22:06:32 +0200 [thread overview]
Message-ID: <20040502220632.06cefb60.khali@linux-fr.org> (raw)
In-Reply-To: <20040417145309.4831f2b6.khali@linux-fr.org>
Quoting myself
> U-ho. I think I've introduced a memory leak with this patch :(
>
> For drivers that handle subclients (asb100 and w83781d on i2c), the
> sublient memory is never released if I read the code correctly. This
> is because we now free the private data on unload, assuming that it
> contains the i2c client data as well. That's true for the main i2c
> client, but not for the subclients (data == NULL so nothing is freed).
>
> Could someone take a look and confirm?
I could test and actually saw memory leaking when cycling the w83781d
driver at a sustained rate (5/s).
> I can see two different fixes:
>
> 1* When freeing the memory, free the data if it's not NULL (main
> client), else free client (subclients). Cleaner (I suppose?).
>
> 2* When creating subclients, do data = &client instead of data = NULL.
> Then freeing will work. Less code, faster. Are there side effects? (I
> don't think so)
>
> My preference would go to 2*.
I ended up implementing 1*. That's cleaner and there's actually almost
no extra code.
Mark, can you confirm that I'm doing the correct thing? I'll do
something similar in our CVS repository (for now, the asb100 and w83781d
drivers had not their memory allocation scheme reworked there).
Greg, please apply if it looks good to you. Sorry for introducing the
leak in the first place...
Thanks.
--- linux-2.6.6-rc3/drivers/i2c/chips/asb100.c.orig Sun May 2 13:52:29 2004
+++ linux-2.6.6-rc3/drivers/i2c/chips/asb100.c Sun May 2 21:51:17 2004
@@ -855,7 +855,13 @@
return err;
}
- kfree(i2c_get_clientdata(client));
+ if (i2c_get_clientdata(client)==NULL) {
+ /* subclients */
+ kfree(client);
+ } else {
+ /* main client */
+ kfree(i2c_get_clientdata(client));
+ }
return 0;
}
--- linux-2.6.6-rc3/drivers/i2c/chips/w83781d.c.orig Sun May 2 15:09:44 2004
+++ linux-2.6.6-rc3/drivers/i2c/chips/w83781d.c Sun May 2 21:51:11 2004
@@ -1336,7 +1336,13 @@
return err;
}
- kfree(i2c_get_clientdata(client));
+ if (i2c_get_clientdata(client)==NULL) {
+ /* subclients */
+ kfree(client);
+ } else {
+ /* main client */
+ kfree(i2c_get_clientdata(client));
+ }
return 0;
}
--
Jean Delvare
http://khali.linux-fr.org/
next prev parent reply other threads:[~2004-05-02 20:06 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2004-04-03 17:10 [RFC 2.6] Rework memory allocation in i2c chip drivers Jean Delvare
2004-04-03 19:07 ` Geert Uytterhoeven
2004-04-03 20:20 ` Sergey Vlasov
2004-04-09 17:31 ` Greg KH
2004-04-10 14:58 ` [PATCH 2.6] Rework memory allocation in i2c chip drivers (second try) Jean Delvare
2004-04-17 12:53 ` Jean Delvare
2004-04-18 6:01 ` Jean Delvare
2004-05-02 20:06 ` Jean Delvare [this message]
2004-05-05 22:18 ` [PATCH 2.6] Fix memory leaks in w83781d and asb100 Greg KH
2004-05-07 12:42 ` Jean Delvare
2004-05-07 22:34 ` Greg KH
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=20040502220632.06cefb60.khali@linux-fr.org \
--to=khali@linux-fr.org \
--cc=greg@kroah.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mhoffman@lightlink.com \
--cc=sensors@Stimpy.netroedge.com \
--cc=vsu@altlinux.ru \
/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