From bea493a031fe3337f4fe5479e8e865513828ea76 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Tue, 17 Oct 2006 00:10:33 -0700 Subject: [PATCH] rt-mutex: fixup rt-mutex debug code BUG: warning at kernel/rtmutex-debug.c:125/rt_mutex_debug_task_free() (Not tainted) [] show_trace_log_lvl+0x58/0x16a [] show_trace+0xd/0x10 [] dump_stack+0x19/0x1b [] rt_mutex_debug_task_free+0x35/0x6a [] free_task+0x15/0x24 [] copy_process+0x12bd/0x1324 [] do_fork+0x42/0x113 [] sys_fork+0x19/0x1b [] syscall_call+0x7/0xb In copy_process(), dup_task_struct() also duplicates the ->pi_lock, ->pi_waiters and ->pi_blocked_on members. rt_mutex_debug_task_free() called from free_task() validates these members. However free_task() can be invoked before these members are reset for the new task. Move the initialization code before the first bail that can hit free_task(). Signed-off-by: Peter Zijlstra Acked-by: Ingo Molnar Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- kernel/fork.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'kernel/fork.c') diff --git a/kernel/fork.c b/kernel/fork.c index 7dc6140baac6..29ebb30850ed 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -984,6 +984,8 @@ static struct task_struct *copy_process(unsigned long clone_flags, if (!p) goto fork_out; + rt_mutex_init_task(p); + #ifdef CONFIG_TRACE_IRQFLAGS DEBUG_LOCKS_WARN_ON(!p->hardirqs_enabled); DEBUG_LOCKS_WARN_ON(!p->softirqs_enabled); @@ -1088,8 +1090,6 @@ static struct task_struct *copy_process(unsigned long clone_flags, p->lockdep_recursion = 0; #endif - rt_mutex_init_task(p); - #ifdef CONFIG_DEBUG_MUTEXES p->blocked_on = NULL; /* not blocked yet */ #endif -- cgit v1.2.3 From 093a8e8aecd77b2799934996a55a6838e1e2b8f3 Mon Sep 17 00:00:00 2001 From: Oleg Nesterov Date: Sat, 28 Oct 2006 10:38:51 -0700 Subject: [PATCH] taskstats_tgid_free: fix usage taskstats_tgid_free() is called on copy_process's error path. This is wrong. IF (clone_flags & CLONE_THREAD) We should not clear ->signal->taskstats, current uses it, it probably has a valid accumulated info. ELSE taskstats_tgid_init() set ->signal->taskstats = NULL, there is nothing to free. Move the callsite to __exit_signal(). We don't need any locking, entire thread group is exiting, nobody should have a reference to soon to be released ->signal. Signed-off-by: Oleg Nesterov Cc: Shailabh Nagar Cc: Balbir Singh Cc: Jay Lan Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- include/linux/taskstats_kern.h | 13 ++----------- kernel/exit.c | 1 + kernel/fork.c | 1 - 3 files changed, 3 insertions(+), 12 deletions(-) (limited to 'kernel/fork.c') diff --git a/include/linux/taskstats_kern.h b/include/linux/taskstats_kern.h index 16894b7edcc8..a437ca0d226b 100644 --- a/include/linux/taskstats_kern.h +++ b/include/linux/taskstats_kern.h @@ -49,17 +49,8 @@ static inline void taskstats_tgid_alloc(struct signal_struct *sig) static inline void taskstats_tgid_free(struct signal_struct *sig) { - struct taskstats *stats = NULL; - unsigned long flags; - - spin_lock_irqsave(&sig->stats_lock, flags); - if (sig->stats) { - stats = sig->stats; - sig->stats = NULL; - } - spin_unlock_irqrestore(&sig->stats_lock, flags); - if (stats) - kmem_cache_free(taskstats_cache, stats); + if (sig->stats) + kmem_cache_free(taskstats_cache, sig->stats); } extern void taskstats_exit_alloc(struct taskstats **, unsigned int *); diff --git a/kernel/exit.c b/kernel/exit.c index f250a5e3e281..06de6c4e8ca3 100644 --- a/kernel/exit.c +++ b/kernel/exit.c @@ -128,6 +128,7 @@ static void __exit_signal(struct task_struct *tsk) flush_sigqueue(&tsk->pending); if (sig) { flush_sigqueue(&sig->shared_pending); + taskstats_tgid_free(sig); __cleanup_signal(sig); } } diff --git a/kernel/fork.c b/kernel/fork.c index 29ebb30850ed..213326609bac 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -897,7 +897,6 @@ static inline int copy_signal(unsigned long clone_flags, struct task_struct * ts void __cleanup_signal(struct signal_struct *sig) { exit_thread_group_keys(sig); - taskstats_tgid_free(sig); kmem_cache_free(signal_cachep, sig); } -- cgit v1.2.3 From b8534d7bd89df0cd41cd47bcd6733a05ea9a691a Mon Sep 17 00:00:00 2001 From: Oleg Nesterov Date: Sat, 28 Oct 2006 10:38:53 -0700 Subject: [PATCH] taskstats: kill ->taskstats_lock in favor of ->siglock signal_struct is (mostly) protected by ->sighand->siglock, I think we don't need ->taskstats_lock to protect ->stats. This also allows us to simplify the locking in fill_tgid(). Signed-off-by: Oleg Nesterov Cc: Shailabh Nagar Cc: Balbir Singh Cc: Jay Lan Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- include/linux/sched.h | 1 - include/linux/taskstats_kern.h | 15 ++++++--------- kernel/fork.c | 2 +- kernel/taskstats.c | 16 ++++++---------- 4 files changed, 13 insertions(+), 21 deletions(-) (limited to 'kernel/fork.c') diff --git a/include/linux/sched.h b/include/linux/sched.h index 6735c1cf334c..eafe4a7b8237 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -466,7 +466,6 @@ struct signal_struct { struct pacct_struct pacct; /* per-process accounting information */ #endif #ifdef CONFIG_TASKSTATS - spinlock_t stats_lock; struct taskstats *stats; #endif }; diff --git a/include/linux/taskstats_kern.h b/include/linux/taskstats_kern.h index 664224008fb2..6562a2050a25 100644 --- a/include/linux/taskstats_kern.h +++ b/include/linux/taskstats_kern.h @@ -23,28 +23,26 @@ static inline void taskstats_exit_free(struct taskstats *tidstats) static inline void taskstats_tgid_init(struct signal_struct *sig) { - spin_lock_init(&sig->stats_lock); sig->stats = NULL; } -static inline void taskstats_tgid_alloc(struct signal_struct *sig) +static inline void taskstats_tgid_alloc(struct task_struct *tsk) { + struct signal_struct *sig = tsk->signal; struct taskstats *stats; - unsigned long flags; if (sig->stats != NULL) return; + /* No problem if kmem_cache_zalloc() fails */ stats = kmem_cache_zalloc(taskstats_cache, SLAB_KERNEL); - if (!stats) - return; - spin_lock_irqsave(&sig->stats_lock, flags); + spin_lock_irq(&tsk->sighand->siglock); if (!sig->stats) { sig->stats = stats; stats = NULL; } - spin_unlock_irqrestore(&sig->stats_lock, flags); + spin_unlock_irq(&tsk->sighand->siglock); if (stats) kmem_cache_free(taskstats_cache, stats); @@ -59,7 +57,6 @@ static inline void taskstats_tgid_free(struct signal_struct *sig) extern void taskstats_exit_alloc(struct taskstats **, unsigned int *); extern void taskstats_exit_send(struct task_struct *, struct taskstats *, int, unsigned int); extern void taskstats_init_early(void); -extern void taskstats_tgid_alloc(struct signal_struct *); #else static inline void taskstats_exit_alloc(struct taskstats **ptidstats, unsigned int *mycpu) {} @@ -71,7 +68,7 @@ static inline void taskstats_exit_send(struct task_struct *tsk, {} static inline void taskstats_tgid_init(struct signal_struct *sig) {} -static inline void taskstats_tgid_alloc(struct signal_struct *sig) +static inline void taskstats_tgid_alloc(struct task_struct *tsk) {} static inline void taskstats_tgid_free(struct signal_struct *sig) {} diff --git a/kernel/fork.c b/kernel/fork.c index 213326609bac..3da978eec791 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -830,7 +830,7 @@ static inline int copy_signal(unsigned long clone_flags, struct task_struct * ts if (clone_flags & CLONE_THREAD) { atomic_inc(¤t->signal->count); atomic_inc(¤t->signal->live); - taskstats_tgid_alloc(current->signal); + taskstats_tgid_alloc(current); return 0; } sig = kmem_cache_alloc(signal_cachep, GFP_KERNEL); diff --git a/kernel/taskstats.c b/kernel/taskstats.c index 9aeee511a463..b2efda94615a 100644 --- a/kernel/taskstats.c +++ b/kernel/taskstats.c @@ -241,11 +241,11 @@ static int fill_tgid(pid_t tgid, struct task_struct *tgidtsk, tsk = first; read_lock(&tasklist_lock); /* Start with stats from dead tasks */ - if (first->signal) { - spin_lock_irqsave(&first->signal->stats_lock, flags); + if (first->sighand) { + spin_lock_irqsave(&first->sighand->siglock, flags); if (first->signal->stats) memcpy(stats, first->signal->stats, sizeof(*stats)); - spin_unlock_irqrestore(&first->signal->stats_lock, flags); + spin_unlock_irqrestore(&first->sighand->siglock, flags); } do { @@ -276,7 +276,7 @@ static void fill_tgid_exit(struct task_struct *tsk) { unsigned long flags; - spin_lock_irqsave(&tsk->signal->stats_lock, flags); + spin_lock_irqsave(&tsk->sighand->siglock, flags); if (!tsk->signal->stats) goto ret; @@ -288,7 +288,7 @@ static void fill_tgid_exit(struct task_struct *tsk) */ delayacct_add_tsk(tsk->signal->stats, tsk); ret: - spin_unlock_irqrestore(&tsk->signal->stats_lock, flags); + spin_unlock_irqrestore(&tsk->sighand->siglock, flags); return; } @@ -464,15 +464,10 @@ void taskstats_exit_send(struct task_struct *tsk, struct taskstats *tidstats, size_t size; int is_thread_group; struct nlattr *na; - unsigned long flags; if (!family_registered || !tidstats) return; - spin_lock_irqsave(&tsk->signal->stats_lock, flags); - is_thread_group = tsk->signal->stats ? 1 : 0; - spin_unlock_irqrestore(&tsk->signal->stats_lock, flags); - rc = 0; /* * Size includes space for nested attributes @@ -480,6 +475,7 @@ void taskstats_exit_send(struct task_struct *tsk, struct taskstats *tidstats, size = nla_total_size(sizeof(u32)) + nla_total_size(sizeof(struct taskstats)) + nla_total_size(0); + is_thread_group = (tsk->signal->stats != NULL); if (is_thread_group) size = 2 * size; /* PID + STATS + TGID + STATS */ -- cgit v1.2.3