lkml.org 
[lkml]   [2015]   [Sep]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH 2/2] kobject: use kvasprintf_const for formatting ->name
Date
Sometimes kobject_set_name_vargs is called with a format string
conaining no %, or a format string of precisely "%s", where the single
vararg happens to point to .rodata. kvasprintf_const detects these
cases for us and returns a copy of that pointer instead of duplicating
the string, thus saving some run-time memory. Otherwise, it falls back
to kvasprintf. We just need to always deallocate ->name using
kfree_const.

Unfortunately, the dance we need to do to perform the '/' -> '!'
sanitization makes the resulting code rather ugly.

I instrumented kstrdup_const to provide some statistics on the memory
saved, and for me this gave an additional ~14KB after boot (306KB was
already saved; this patch bumped that to 320KB). I have
KMALLOC_SHIFT_LOW==3, and since 80% of the kvasprintf_const hits were
satisfied by an 8-byte allocation, the 14K would roughly be quadrupled
when KMALLOC_SHIFT_LOW==5. Whether these numbers are sufficient to
justify the ugliness I'll leave to others to decide.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
lib/kobject.c | 30 ++++++++++++++++++++++--------
1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/lib/kobject.c b/lib/kobject.c
index 3e3a5c3cb330..fee2fd950306 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -257,18 +257,32 @@ static int kobject_add_internal(struct kobject *kobj)
int kobject_set_name_vargs(struct kobject *kobj, const char *fmt,
va_list vargs)
{
- char *s;
+ const char *s;

if (kobj->name && !fmt)
return 0;

- s = kvasprintf(GFP_KERNEL, fmt, vargs);
+ s = kvasprintf_const(GFP_KERNEL, fmt, vargs);
if (!s)
return -ENOMEM;

- /* ewww... some of these buggers have '/' in the name ... */
- strreplace(s, '/', '!');
- kfree(kobj->name);
+ /*
+ * ewww... some of these buggers have '/' in the name ... If
+ * that's the case, we need to make sure we have an actual
+ * allocated copy to modify, since kvasprintf_const may have
+ * returned something from .rodata.
+ */
+ if (strchr(s, '/')) {
+ char *t;
+
+ t = kstrdup(s, GFP_KERNEL);
+ kfree_const(s);
+ if (!t)
+ return -ENOMEM;
+ strreplace(t, '/', '!');
+ s = t;
+ }
+ kfree_const(kobj->name);
kobj->name = s;

return 0;
@@ -466,7 +480,7 @@ int kobject_rename(struct kobject *kobj, const char *new_name)
envp[0] = devpath_string;
envp[1] = NULL;

- name = dup_name = kstrdup(new_name, GFP_KERNEL);
+ name = dup_name = kstrdup_const(new_name, GFP_KERNEL);
if (!name) {
error = -ENOMEM;
goto out;
@@ -486,7 +500,7 @@ int kobject_rename(struct kobject *kobj, const char *new_name)
kobject_uevent_env(kobj, KOBJ_MOVE, envp);

out:
- kfree(dup_name);
+ kfree_const(dup_name);
kfree(devpath_string);
kfree(devpath);
kobject_put(kobj);
@@ -632,7 +646,7 @@ static void kobject_cleanup(struct kobject *kobj)
/* free name if we allocated it */
if (name) {
pr_debug("kobject: '%s': free name\n", name);
- kfree(name);
+ kfree_const(name);
}
}

--
2.1.3


\
 
 \ /
  Last update: 2015-09-10 00:01    [W:0.046 / U:1.388 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site