![]() | |||||||||||
Messages in this thread |
Balbir Singh wrote:
> Hi, Andrew,
>
> This patch fixes a race in per-task-delay-accounting. This race
> was reported by Jay Lan. I tested the patch using
> cerebrus test control system for eight hours with getdelays running on
> the side (for both push and pull of delay statistics).
>
> It fixed the problem that Jay Lan saw.
>
> Here's an explanation of the race condition
>
> Consider tasks of the same thread group exiting, lets call them T1 and T2
>
>
> T1 T2
>
> CPU0 CPU1
> ===== =====
>
> do_exit()
> ... do_exit()
> taskstats_exit_send() ...
> taskstats_exit_send()
> fill_tgid()
> delayacct_add_tsk()
> delayacct_tsk_exit() ....
> __delayacct_add_tsk()
>
>
> While T1 is yet to be removed from the thread group. T1->delays is set to NULL
> between delayacct_add_tsk() and __delayacct_add_tsk() call.
>
> When T2 looks for threads in the thread group, it finds T1 and tries to
> collect stats for it. When we get to the spin_lock() in __delayacct_add_tsk(),
> we find T1->delays is a NULL pointer.
>
> Signed-off-by: Balbir Singh <balbir@in.ibm.com>
> ---
>
> include/linux/taskstats_kern.h | 1 +
> kernel/delayacct.c | 5 ++++-
> kernel/taskstats.c | 5 ++++-
> 3 files changed, 9 insertions(+), 2 deletions(-)
>
> diff -puN kernel/taskstats.c~per-task-delay-accounting-fix-exit-race kernel/taskstats.c
> --- linux-2.6.17-rc6/kernel/taskstats.c~per-task-delay-accounting-fix-exit-race 2006-06-20 13:49:29.000000000 +0530
> +++ linux-2.6.17-rc6-balbir/kernel/taskstats.c 2006-06-20 13:49:29.000000000 +0530
> @@ -25,7 +25,7 @@
> static DEFINE_PER_CPU(__u32, taskstats_seqnum) = { 0 };
> static int family_registered = 0;
> kmem_cache_t *taskstats_cache;
> -static DEFINE_MUTEX(taskstats_exit_mutex);
> +DEFINE_MUTEX(taskstats_exit_mutex);
>
> static struct genl_family family = {
> .id = GENL_ID_GENERATE,
> @@ -193,6 +193,7 @@ static int taskstats_send_stats(struct s
> if (rc < 0)
> return rc;
>
> + mutex_lock(&taskstats_exit_mutex);
> if (info->attrs[TASKSTATS_CMD_ATTR_PID]) {
> u32 pid = nla_get_u32(info->attrs[TASKSTATS_CMD_ATTR_PID]);
> rc = fill_pid(pid, NULL, &stats);
> @@ -218,6 +219,7 @@ static int taskstats_send_stats(struct s
> goto err;
> }
>
> + mutex_unlock(&taskstats_exit_mutex);
> nla_nest_end(rep_skb, na);
>
> return send_reply(rep_skb, info->snd_pid, TASKSTATS_MSG_UNICAST);
> @@ -226,6 +228,7 @@ nla_put_failure:
> return genlmsg_cancel(rep_skb, reply);
> err:
> nlmsg_free(rep_skb);
> + mutex_unlock(&taskstats_exit_mutex);
> return rc;
> }
>
> diff -puN kernel/delayacct.c~per-task-delay-accounting-fix-exit-race kernel/delayacct.c
> --- linux-2.6.17-rc6/kernel/delayacct.c~per-task-delay-accounting-fix-exit-race 2006-06-20 13:49:29.000000000 +0530
> +++ linux-2.6.17-rc6-balbir/kernel/delayacct.c 2006-06-20 13:49:29.000000000 +0530
> @@ -48,8 +48,11 @@ void __delayacct_tsk_init(struct task_st
>
> void __delayacct_tsk_exit(struct task_struct *tsk)
> {
> - kmem_cache_free(delayacct_cache, tsk->delays);
> + struct task_delay_info *delays = tsk->delays;
> + mutex_lock(&taskstats_exit_mutex);
> tsk->delays = NULL;
> + mutex_unlock(&taskstats_exit_mutex);
> + kmem_cache_free(delayacct_cache, delays);
> }
>
> /*
> diff -puN include/linux/taskstats_kern.h~per-task-delay-accounting-fix-exit-race include/linux/taskstats_kern.h
> --- linux-2.6.17-rc6/include/linux/taskstats_kern.h~per-task-delay-accounting-fix-exit-race 2006-06-20 13:49:29.000000000 +0530
> +++ linux-2.6.17-rc6-balbir/include/linux/taskstats_kern.h 2006-06-20 13:49:29.000000000 +0530
> @@ -17,6 +17,7 @@ enum {
>
> #ifdef CONFIG_TASKSTATS
> extern kmem_cache_t *taskstats_cache;
> +extern struct mutex taskstats_exit_mutex;
>
> static inline void taskstats_exit_alloc(struct taskstats **ptidstats,
> struct taskstats **ptgidstats)
> _
>
The Cc's got messed up (a really odd name lookup happened at the mail
server).
Sorry
Balbir Singh,
Linux Technology Center,
IBM Software Labs
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
| ||||||||||
| Last update: 2006-06-21 08:55 [from the cache] ©2003-2008 | |||||||||||