[PATCH] task delay accounting fixes

Cleanup allocation and freeing of tsk->delays used by delay accounting.
This solves two problems reported for delay accounting:

1. oops in __delayacct_blkio_ticks
http://www.uwsg.indiana.edu/hypermail/linux/kernel/0608.2/1844.html

Currently tsk->delays is getting freed too early in task exit which can
cause a NULL tsk->delays to get accessed via reading of /proc/<tgid>/stats.
 The patch fixes this problem by freeing tsk->delays closer to when
task_struct itself is freed up.  As a result, it also eliminates the use of
tsk->delays_lock which was only being used (inadequately) to safeguard
access to tsk->delays while a task was exiting.

2. Possible memory leak in kernel/delayacct.c
http://www.uwsg.indiana.edu/hypermail/linux/kernel/0608.2/1389.html

The patch cleans up tsk->delays allocations after a bad fork which was
missing earlier.

The patch has been tested to fix the problems listed above and stress
tested with rapid calls to delay accounting's taskstats command interface
(which is the other path that can access the same data, besides the /proc
interface causing the oops above).

Signed-off-by: Shailabh Nagar <nagar@watson.ibm.com>
Cc: Balbir Singh <balbir@in.ibm.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/include/linux/delayacct.h b/include/linux/delayacct.h
index 11487b6e7127ff68ec12cada74c672af72efcffb..561e2a77805c582fdebcd3d588a222d8adec9a6c 100644 (file)
--- a/include/linux/delayacct.h
+++ b/include/linux/delayacct.h
@@ -59,10 +59,14 @@ static inline void delayacct_tsk_init(struct task_struct *tsk)
                __delayacct_tsk_init(tsk);
 }
 
-static inline void delayacct_tsk_exit(struct task_struct *tsk)
+/* Free tsk->delays. Called from bad fork and __put_task_struct
+ * where there's no risk of tsk->delays being accessed elsewhere
+ */
+static inline void delayacct_tsk_free(struct task_struct *tsk)
 {
        if (tsk->delays)
-               __delayacct_tsk_exit(tsk);
+               kmem_cache_free(delayacct_cache, tsk->delays);
+       tsk->delays = NULL;
 }
 
 static inline void delayacct_blkio_start(void)
@@ -101,7 +105,7 @@ static inline void delayacct_init(void)
 {}
 static inline void delayacct_tsk_init(struct task_struct *tsk)
 {}
-static inline void delayacct_tsk_exit(struct task_struct *tsk)
+static inline void delayacct_tsk_free(struct task_struct *tsk)
 {}
 static inline void delayacct_blkio_start(void)
 {}

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 6674fc1e51bf3dfe0677633e489231a91c271dac..34ed0d99b1bd1ccde1dee28a164bfded8fb1fc37 100644 (file)
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -994,7 +994,6 @@ struct task_struct {
         */
        struct pipe_inode_info *splice_pipe;
 #ifdef CONFIG_TASK_DELAY_ACCT
-       spinlock_t delays_lock;
        struct task_delay_info *delays;
 #endif
 };

diff --git a/kernel/delayacct.c b/kernel/delayacct.c
index 57ca3730205d75f0734ac7ad418d85cced3025ef..36752f124c6a1897809edd3788a6ed88a3072ba8 100644 (file)
--- a/kernel/delayacct.c
+++ b/kernel/delayacct.c
@@ -41,24 +41,11 @@ void delayacct_init(void)
 
 void __delayacct_tsk_init(struct task_struct *tsk)
 {
-       spin_lock_init(&tsk->delays_lock);
-       /* No need to acquire tsk->delays_lock for allocation here unless
-          __delayacct_tsk_init called after tsk is attached to tasklist
-       */
        tsk->delays = kmem_cache_zalloc(delayacct_cache, SLAB_KERNEL);
        if (tsk->delays)
                spin_lock_init(&tsk->delays->lock);
 }
 
-void __delayacct_tsk_exit(struct task_struct *tsk)
-{
-       struct task_delay_info *delays = tsk->delays;
-       spin_lock(&tsk->delays_lock);
-       tsk->delays = NULL;
-       spin_unlock(&tsk->delays_lock);
-       kmem_cache_free(delayacct_cache, delays);
-}
-
 /*
  * Start accounting for a delay statistic using
  * its starting timestamp (@start)
@@ -118,8 +105,6 @@ int __delayacct_add_tsk(struct taskstats *d, struct task_struct *tsk)
        struct timespec ts;
        unsigned long t1,t2,t3;
 
-       spin_lock(&tsk->delays_lock);
-
        /* Though tsk->delays accessed later, early exit avoids
         * unnecessary returning of other data
         */
@@ -161,7 +146,6 @@ int __delayacct_add_tsk(struct taskstats *d, struct task_struct *tsk)
        spin_unlock(&tsk->delays->lock);
 
 done:
-       spin_unlock(&tsk->delays_lock);
        return 0;
 }
 

diff --git a/kernel/exit.c b/kernel/exit.c
index dba194a8d41619d689d4e53f9819bb51583cb72c..a4c19a52ce46dbee40cfa0d67e37f406cf9329ed 100644 (file)
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -908,7 +908,6 @@ fastcall NORET_TYPE void do_exit(long code)
                audit_free(tsk);
        taskstats_exit_send(tsk, tidstats, group_dead, mycpu);
        taskstats_exit_free(tidstats);
-       delayacct_tsk_exit(tsk);
 
        exit_mm(tsk);
 

diff --git a/kernel/fork.c b/kernel/fork.c
index aa36c43783cc01f296eb1c755cc4f17b51b55ccc..f9b014e3e7002e243ca8120f8f81aa33bf0f0f81 100644 (file)
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -117,6 +117,7 @@ void __put_task_struct(struct task_struct *tsk)
        security_task_free(tsk);
        free_uid(tsk->user);
        put_group_info(tsk->group_info);
+       delayacct_tsk_free(tsk);
 
        if (!profile_handoff_task(tsk))
                free_task(tsk);
@@ -1011,7 +1012,7 @@ static struct task_struct *copy_process(unsigned long clone_flags,
        retval = -EFAULT;
        if (clone_flags & CLONE_PARENT_SETTID)
                if (put_user(p->pid, parent_tidptr))
-                       goto bad_fork_cleanup;
+                       goto bad_fork_cleanup_delays_binfmt;
 
        INIT_LIST_HEAD(&p->children);
        INIT_LIST_HEAD(&p->sibling);
@@ -1277,7 +1278,8 @@ bad_fork_cleanup_policy:
 bad_fork_cleanup_cpuset:
 #endif
        cpuset_exit(p);
-bad_fork_cleanup:
+bad_fork_cleanup_delays_binfmt:
+       delayacct_tsk_free(p);
        if (p->binfmt)
                module_put(p->binfmt->module);
 bad_fork_cleanup_put_domain: