Tetsuo Handa wrote:
> This is a draft patch which changes task_struct->comm to use RCU.
Changes from previous draft version:
Changed "struct rcu_comm" to use copy-on-write approach. Those multi-thread
or multi-process applications which do not change comm name will consume
memory for only one "struct rcu_comm".
Changed do_commset() not to sleep.
Changed to tolerate loss of consistency when memory allocation failed.
Fixed race condition in copy_process().
Regards.
----------
>From ada6c4d94f5afda36c7c21869d38b7111a6fe9bc Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-ker...@i-love.sakura.ne.jp>
Date: Mon, 17 Feb 2014 14:32:11 +0900
Subject: [PATCH] Change task_struct->comm to use RCU.
This patch changes task_struct->comm to be updated using RCU
(unless memory allocation fails).
Signed-off-by: Tetsuo Handa <penguin-ker...@i-love.sakura.ne.jp>
---
fs/exec.c | 145 +++++++++++++++++++++++++++++++++++++++------
include/linux/init_task.h | 4 +-
include/linux/sched.h | 37 ++++++++++--
kernel/fork.c | 9 +++
kernel/kthread.c | 4 +-
kernel/sched/core.c | 2 +-
6 files changed, 173 insertions(+), 28 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index a781dec..8a68ab3 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -263,6 +263,11 @@ extern char ___assert_task_state[1 - 2*!!(
/* Task command name length */
#define TASK_COMM_LEN 16
+struct rcu_comm {
+ char name[TASK_COMM_LEN];
+ atomic_t usage;
+ struct rcu_head rcu;
+};
#include <linux/spinlock.h>
@@ -1317,10 +1322,12 @@ struct task_struct {
* credentials (COW) */
const struct cred __rcu *cred; /* effective (overridable) subjective
task
* credentials (COW) */
- char comm[TASK_COMM_LEN]; /* executable name excluding path
- - access with [gs]et_task_comm (which lock
- it with task_lock())
- - initialized normally by setup_new_exec */
+ struct rcu_comm __rcu *rcu_comm; /* executable name excluding path (COW)
+ - update with set_task_comm() or
+ do_set_task_comm[_fmt]()
+ - read with commcpy() or %pT
+ - initialized normally by
+ setup_new_exec */
/* file system info */
int link_count, total_link_count;
#ifdef CONFIG_SYSVIPC
@@ -1792,6 +1799,23 @@ static inline cputime_t task_gtime(struct task_struct *t)
extern void task_cputime_adjusted(struct task_struct *p, cputime_t *ut,
cputime_t *st);
extern void thread_group_cputime_adjusted(struct task_struct *p, cputime_t
*ut, cputime_t *st);
+/**
+ * commcpy - Copy task_struct->comm to buffer.
+ *
+ * @buf: Buffer to copy @tsk->comm which must be at least TASK_COMM_LEN bytes.
+ * @tsk: Pointer to "struct task_struct".
+ *
+ * Returns @buf .
+ */
+static inline char *commcpy(char *buf, const struct task_struct *tsk)
+{
+ rcu_read_lock();
+ memcpy(buf, rcu_dereference(tsk->rcu_comm)->name, TASK_COMM_LEN);
+ rcu_read_unlock();
+ buf[TASK_COMM_LEN - 1] = '\0';
+ return buf;
+}
+
/*
* Per process flags
*/
@@ -2320,7 +2344,10 @@ struct task_struct *fork_idle(int);
extern pid_t kernel_thread(int (*fn)(void *), void *arg, unsigned long flags);
extern void set_task_comm(struct task_struct *tsk, char *from);
-extern char *get_task_comm(char *to, struct task_struct *tsk);
+extern void do_set_task_comm(struct task_struct *tsk, const char *buf);
+extern void do_set_task_comm_fmt(struct task_struct *tsk, const char *fmt, ...)
+ __printf(2, 3);
+extern void put_commname(struct rcu_comm *comm);
#ifdef CONFIG_SMP
void scheduler_ipi(void);
diff --git a/include/linux/init_task.h b/include/linux/init_task.h
index 6df7f9f..b217f8c 100644
--- a/include/linux/init_task.h
+++ b/include/linux/init_task.h
@@ -154,7 +154,7 @@ extern struct task_group root_task_group;
# define INIT_VTIME(tsk)
#endif
-#define INIT_TASK_COMM "swapper"
+extern struct rcu_comm init_rcu_comm;
#ifdef CONFIG_RT_MUTEXES
# define INIT_RT_MUTEXES(tsk) \
@@ -201,7 +201,7 @@ extern struct task_group root_task_group;
.group_leader = &tsk, \
RCU_POINTER_INITIALIZER(real_cred, &init_cred), \
RCU_POINTER_INITIALIZER(cred, &init_cred), \
- .comm = INIT_TASK_COMM, \
+ .rcu_comm = &init_rcu_comm, \
.thread = INIT_THREAD, \
.fs = &init_fs, \
.files = &init_files, \
diff --git a/fs/exec.c b/fs/exec.c
index 3d78fcc..85c57a6 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1026,30 +1026,11 @@ killed:
return -EAGAIN;
}
-char *get_task_comm(char *buf, struct task_struct *tsk)
-{
- /* buf must be at least sizeof(tsk->comm) in size */
- task_lock(tsk);
- strncpy(buf, tsk->comm, sizeof(tsk->comm));
- task_unlock(tsk);
- return buf;
-}
-EXPORT_SYMBOL_GPL(get_task_comm);
-
/*
* These functions flushes out all traces of the currently running executable
* so that a new one can be started
*/
-void set_task_comm(struct task_struct *tsk, char *buf)
-{
- task_lock(tsk);
- trace_task_rename(tsk, buf);
- strlcpy(tsk->comm, buf, sizeof(tsk->comm));
- task_unlock(tsk);
- perf_event_comm(tsk);
-}
-
static void filename_to_taskname(char *tcomm, const char *fn, unsigned int len)
{
int i, ch;
@@ -1626,3 +1607,129 @@ asmlinkage long compat_sys_execve(const char __user *
filename,
return compat_do_execve(getname(filename), argv, envp);
}
#endif
+
+/* Task's comm name handler functions. */
+static struct kmem_cache *commname_cachep __read_mostly;
+
+/* comm name for init_task. */
+struct rcu_comm init_rcu_comm = {
+ .name = "swapper",
+ .usage = ATOMIC_INIT(INT_MAX) /* Mark this object static. */
+};
+
+/**
+ * set_task_comm - Change task_struct->comm with tracer and perf hooks called.
+ *
+ * @tsk: Pointer to "struct task_struct".
+ * @buf: New comm name.
+ *
+ * Returns nothing.
+ */
+void set_task_comm(struct task_struct *tsk, char *buf)
+{
+ /*
+ * Question: Do we need to use task_lock()/task_unlock() ?
+ */
+ task_lock(tsk);
+ trace_task_rename(tsk, buf);
+ task_unlock(tsk);
+ do_set_task_comm(tsk, buf);
+ perf_event_comm(tsk);
+}
+
+/**
+ * do_set_task_comm_fmt - Change task_struct->comm.
+ *
+ * @tsk: Pointer to "struct task_struct".
+ * @fmt: Format string, followed by arguments.
+ *
+ * Returns nothing.
+ */
+void do_set_task_comm_fmt(struct task_struct *tsk, const char *fmt, ...)
+{
+ va_list args;
+ char name[TASK_COMM_LEN];
+
+ va_start(args, fmt);
+ vsnprintf(name, TASK_COMM_LEN, fmt, args);
+ va_end(args);
+ do_set_task_comm(tsk, name);
+}
+
+/**
+ * do_set_task_comm - Change task_struct->comm.
+ *
+ * @tsk: Pointer to "struct task_struct".
+ * @buf: New comm name.
+ *
+ * Returns nothing.
+ */
+void do_set_task_comm(struct task_struct *tsk, const char *buf)
+{
+ struct rcu_comm *comm;
+
+ comm = kmem_cache_zalloc(commname_cachep, GFP_ATOMIC | __GFP_NOWARN);
+ if (likely(comm)) {
+ atomic_set(&comm->usage, 1);
+ strncpy(comm->name, buf, TASK_COMM_LEN - 1);
+ smp_wmb(); /* Behave like rcu_assign_pointer(). */
+ comm = xchg(&tsk->rcu_comm, comm);
+ put_commname(comm);
+ } else {
+ /*
+ * Memory allocation failed. We have to tolerate loss of
+ * consistency.
+ *
+ * Question 1: Do we want to reserve some amount of static
+ * "struct rcu_comm" arrays for use upon allocation failures?
+ *
+ * Question 2: Do we perfer unchanged comm name over
+ * overwritten comm name because comm name is copy-on-write ?
+ */
+ WARN_ONCE(1, "Failed to allocate memory for comm name.\n");
+ rcu_read_lock();
+ do {
+ comm = rcu_dereference(tsk->rcu_comm);
+ } while (!atomic_read(&comm->usage));
+ strncpy(comm->name, buf, TASK_COMM_LEN - 1);
+ rcu_read_unlock();
+ }
+}
+
+/**
+ * free_commname - Release "struct rcu_comm".
+ *
+ * @rcu: Pointer to "struct rcu_head".
+ *
+ * Returns nothing.
+ */
+static void free_commname(struct rcu_head *rcu)
+{
+ struct rcu_comm *comm = container_of(rcu, struct rcu_comm, rcu);
+ kmem_cache_free(commname_cachep, comm);
+}
+
+/**
+ * put_commname - Release "struct rcu_comm".
+ *
+ * @comm: Pointer to "struct rcu_comm".
+ *
+ * Returns nothing.
+ */
+void put_commname(struct rcu_comm *comm)
+{
+ if (atomic_dec_and_test(&comm->usage))
+ call_rcu(&comm->rcu, free_commname);
+}
+
+/**
+ * commname_init - Initialize "struct rcu_comm".
+ *
+ * Returns nothing.
+ */
+void __init commname_init(void)
+{
+ commname_cachep = kmem_cache_create("commname",
+ sizeof(struct rcu_comm),
+ 0, SLAB_PANIC, NULL);
+}
diff --git a/kernel/fork.c b/kernel/fork.c
index a17621c..23be1ef 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -214,6 +214,7 @@ void free_task(struct task_struct *tsk)
ftrace_graph_exit_task(tsk);
put_seccomp_filter(tsk);
arch_release_task_struct(tsk);
+ put_commname(tsk->rcu_comm);
free_task_struct(tsk);
}
EXPORT_SYMBOL(free_task);
@@ -249,6 +250,8 @@ EXPORT_SYMBOL_GPL(__put_task_struct);
void __init __weak arch_task_cache_init(void) { }
+extern void __init commname_init(void);
+
void __init fork_init(unsigned long mempages)
{
#ifndef CONFIG_ARCH_TASK_STRUCT_ALLOCATOR
@@ -260,6 +263,7 @@ void __init fork_init(unsigned long mempages)
kmem_cache_create("task_struct", sizeof(struct task_struct),
ARCH_MIN_TASKALIGN, SLAB_PANIC | SLAB_NOTRACK, NULL);
#endif
+ commname_init();
/* do the arch specific task caches init */
arch_task_cache_init();
@@ -1191,6 +1195,11 @@ static struct task_struct *copy_process(unsigned long
clone_flags,
p = dup_task_struct(current);
if (!p)
goto fork_out;
+ rcu_read_lock();
+ do {
+ p->rcu_comm = rcu_dereference(current->rcu_comm);
+ } while (!atomic_inc_not_zero(&p->rcu_comm->usage));
+ rcu_read_unlock();
ftrace_graph_init_task(p);
get_seccomp_filter(p);
diff --git a/kernel/kthread.c b/kernel/kthread.c
index b5ae3ee..e83b67c 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -309,10 +309,12 @@ struct task_struct *kthread_create_on_node(int
(*threadfn)(void *data),
if (!IS_ERR(task)) {
static const struct sched_param param = { .sched_priority = 0 };
va_list args;
+ char name[TASK_COMM_LEN];
va_start(args, namefmt);
- vsnprintf(task->comm, sizeof(task->comm), namefmt, args);
+ vsnprintf(name, TASK_COMM_LEN, namefmt, args);
va_end(args);
+ do_set_task_comm(task, name);
/*
* root may have changed our (kthreadd's) priority or CPU mask.
* The kernel thread should not inherit these properties.
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index b46131e..bbb7c94 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4488,7 +4488,7 @@ void init_idle(struct task_struct *idle, int cpu)
ftrace_graph_init_idle_task(idle, cpu);
vtime_init_idle(idle, cpu);
#if defined(CONFIG_SMP)
- sprintf(idle->comm, "%s/%d", INIT_TASK_COMM, cpu);
+ do_set_task_comm_fmt(idle, "swapper/%d", cpu);
#endif
}
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
