lkml.org 
[lkml]   [2013]   [Oct]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [RFC] Add inline routine to free memory used in kobject name
On 10/07/2013 03:10 PM, Greg KH wrote:
> On Mon, Oct 07, 2013 at 12:43:41PM -0500, Larry Finger wrote:
>> At present, if one wants to free the memory allocation used for
>> a dev->kobj name, it is necessary to go quite deeply into the structure.
>
> Why would you ever want to do this?
>
>> To avoid this much dependence on the structure details in driver
>> code, a new inline routine is created.
>>
>> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
>> ---
>>
>> Index: wireless-testing-save/include/linux/device.h
>> ===================================================================
>> --- wireless-testing-save.orig/include/linux/device.h
>> +++ wireless-testing-save/include/linux/device.h
>> @@ -27,6 +27,7 @@
>> #include <linux/ratelimit.h>
>> #include <linux/uidgid.h>
>> #include <asm/device.h>
>> +#include <linux/slab.h>
>>
>> struct device;
>> struct device_private;
>> @@ -789,6 +790,11 @@ static inline const char *dev_name(const
>> return kobject_name(&dev->kobj);
>> }
>>
>> +static inline void dev_free_name(struct device *dev)
>> +{
>> + kfree(dev->kobj.name);
>> +}
>
> Please show how you would use this function, I can't add functions that
> no one calls.
>
> And given that this type of thing hasn't been needed before, I'm
> thinking that it still isn't needed :)

In the thread at
http://lkml.indiana.edu/hypermail/linux/kernel/1310.0/02008.html, I reported a
leak of kobj->name in the error path of the memstick driver. My solution was to
free it in the error path by using

kfree(card->dev.kobj.name);

Catalin Marinas responded with "It looks weird to go into dev.kobj internals
here for freeing the name. There is also memstick_free_card() which doesn't seem
to do anything about the name freeing."

Later in the thread, he agreed that having a new function sounded like a good
idea. I should have submitted the second patch using the new function as follows:

Index: wireless-testing-save/drivers/memstick/core/memstick.c
===================================================================
--- wireless-testing-save.orig/drivers/memstick/core/memstick.c
+++ wireless-testing-save/drivers/memstick/core/memstick.c
@@ -195,6 +195,7 @@ static void memstick_free_card(struct de
{
struct memstick_dev *card = container_of(dev, struct memstick_dev,
dev);
+ dev_free_name(&card->dev);
kfree(card);
}

@@ -415,6 +416,7 @@ static struct memstick_dev *memstick_all
return card;
err_out:
host->card = old_card;
+ dev_free_name(&card->dev);
kfree(card);
return NULL;
}
These changes clear up the memory leak that started the whole subject. The
difference would be between
kfree(card->dev.kobj.name);
and
dev_free_name(&card->dev);
I have not looked at every line in the kernel containing both "kfree" and "name"
to see how many other places that the new routine could be used. I'm not even
sure how other drivers free the space allocated for a name.
Larry


\
 
 \ /
  Last update: 2013-10-07 23:01    [W:2.031 / U:0.240 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site