From 6892b75e60557a48c01d57ba320419a9e2ce9846 Mon Sep 17 00:00:00 2001 From: Ingo Molnar Date: Wed, 13 Feb 2008 14:02:36 +0100 Subject: [PATCH 1/8] sched: make early bootup sched_clock() use safer do not call sched_clock() too early. Not only might rq->idle not be set up - but pure per-cpu data might not be accessible either. this solves an ia64 early bootup hang with CONFIG_PRINTK_TIME=y. Tested-by: Tony Luck Acked-by: Tony Luck Acked-by: David S. Miller Signed-off-by: Ingo Molnar --- kernel/sched.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/kernel/sched.c b/kernel/sched.c index b387a8de26a5..7286ccb01082 100644 --- a/kernel/sched.c +++ b/kernel/sched.c @@ -668,6 +668,8 @@ const_debug unsigned int sysctl_sched_nr_migrate = 32; */ unsigned int sysctl_sched_rt_period = 1000000; +static __read_mostly int scheduler_running; + /* * part of the period that we allow rt tasks to run in us. * default: 0.95s @@ -689,14 +691,16 @@ unsigned long long cpu_clock(int cpu) unsigned long flags; struct rq *rq; - local_irq_save(flags); - rq = cpu_rq(cpu); /* * Only call sched_clock() if the scheduler has already been * initialized (some code might call cpu_clock() very early): */ - if (rq->idle) - update_rq_clock(rq); + if (unlikely(!scheduler_running)) + return 0; + + local_irq_save(flags); + rq = cpu_rq(cpu); + update_rq_clock(rq); now = rq->clock; local_irq_restore(flags); @@ -7284,6 +7288,8 @@ void __init sched_init(void) * During early bootup we pretend to be a normal task: */ current->sched_class = &fair_sched_class; + + scheduler_running = 1; } #ifdef CONFIG_DEBUG_SPINLOCK_SLEEP From 70eee74b70c1a8485ec5f2bafa13dbc66fab6e02 Mon Sep 17 00:00:00 2001 From: Balbir Singh Date: Fri, 22 Feb 2008 13:25:53 +0530 Subject: [PATCH 2/8] sched: remove duplicate code from sched_fair.c pick_task_entity() duplicates existing code. This functionality can be easily obtained using rb_last(). Avoid code duplication by using rb_last(). Signed-off-by: Balbir Singh Signed-off-by: Ingo Molnar --- kernel/sched_fair.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c index 6c091d6e159d..7abad50d935f 100644 --- a/kernel/sched_fair.c +++ b/kernel/sched_fair.c @@ -202,16 +202,13 @@ static struct sched_entity *__pick_next_entity(struct cfs_rq *cfs_rq) static inline struct sched_entity *__pick_last_entity(struct cfs_rq *cfs_rq) { - struct rb_node **link = &cfs_rq->tasks_timeline.rb_node; - struct sched_entity *se = NULL; - struct rb_node *parent; - - while (*link) { - parent = *link; - se = rb_entry(parent, struct sched_entity, run_node); - link = &parent->rb_right; - } + struct rb_node *last; + struct sched_entity *se; + last = rb_last(&cfs_rq->tasks_timeline); + if (!last) + return NULL; + se = rb_entry(last, struct sched_entity, run_node); return se; } From 7eee3e677d6e2e9007afcd7d79b0715525aa552e Mon Sep 17 00:00:00 2001 From: Ingo Molnar Date: Fri, 22 Feb 2008 10:32:21 +0100 Subject: [PATCH 3/8] sched: clean up __pick_last_entity() a bit Signed-off-by: Ingo Molnar --- kernel/sched_fair.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c index 7abad50d935f..c8e6492c5925 100644 --- a/kernel/sched_fair.c +++ b/kernel/sched_fair.c @@ -202,14 +202,12 @@ static struct sched_entity *__pick_next_entity(struct cfs_rq *cfs_rq) static inline struct sched_entity *__pick_last_entity(struct cfs_rq *cfs_rq) { - struct rb_node *last; - struct sched_entity *se; + struct rb_node *last = rb_last(&cfs_rq->tasks_timeline); - last = rb_last(&cfs_rq->tasks_timeline); if (!last) return NULL; - se = rb_entry(last, struct sched_entity, run_node); - return se; + + return rb_entry(last, struct sched_entity, run_node); } /************************************************************** From 67ca7bde2e9d3516b5ae0188330ad1059ac03f38 Mon Sep 17 00:00:00 2001 From: Harvey Harrison Date: Fri, 15 Feb 2008 09:56:36 -0800 Subject: [PATCH 4/8] sched: fix signedness warnings in sched.c Unsigned long values are always assigned to switch_count, make it unsigned long. kernel/sched.c:3897:15: warning: incorrect type in assignment (different signedness) kernel/sched.c:3897:15: expected long *switch_count kernel/sched.c:3897:15: got unsigned long * kernel/sched.c:3921:16: warning: incorrect type in assignment (different signedness) kernel/sched.c:3921:16: expected long *switch_count kernel/sched.c:3921:16: got unsigned long * Signed-off-by: Harvey Harrison Signed-off-by: Ingo Molnar --- kernel/sched.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/sched.c b/kernel/sched.c index 7286ccb01082..f06950c8a6ce 100644 --- a/kernel/sched.c +++ b/kernel/sched.c @@ -3889,7 +3889,7 @@ pick_next_task(struct rq *rq, struct task_struct *prev) asmlinkage void __sched schedule(void) { struct task_struct *prev, *next; - long *switch_count; + unsigned long *switch_count; struct rq *rq; int cpu; From 2d07b255c7b8a9723010e5c74778e058dc05162e Mon Sep 17 00:00:00 2001 From: Harvey Harrison Date: Fri, 15 Feb 2008 09:56:34 -0800 Subject: [PATCH 5/8] sched: add declaration of sched_tail to sched.h Avoids sparse warnings: kernel/sched.c:2170:17: warning: symbol 'schedule_tail' was not declared. Should it be static? Avoids the need for an external declaration in arch/um/process.c Signed-off-by: Harvey Harrison Signed-off-by: Ingo Molnar --- arch/um/kernel/process.c | 2 -- include/linux/sched.h | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/arch/um/kernel/process.c b/arch/um/kernel/process.c index fc50d2f959d1..e8cb9ff183e9 100644 --- a/arch/um/kernel/process.c +++ b/arch/um/kernel/process.c @@ -128,8 +128,6 @@ void *get_current(void) return current; } -extern void schedule_tail(struct task_struct *prev); - /* * This is called magically, by its address being stuffed in a jmp_buf * and being longjmp-d to. diff --git a/include/linux/sched.h b/include/linux/sched.h index e217d188a102..9c17e828d6d4 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -242,6 +242,7 @@ struct task_struct; extern void sched_init(void); extern void sched_init_smp(void); +extern asmlinkage void schedule_tail(struct task_struct *prev); extern void init_idle(struct task_struct *idle, int cpu); extern void init_idle_bootup_task(struct task_struct *idle); From ae0027869db7d28563cd783865fab04ffd18419c Mon Sep 17 00:00:00 2001 From: Hiroshi Shimamoto Date: Thu, 14 Feb 2008 10:26:24 -0800 Subject: [PATCH 6/8] latencytop: fix kernel panic while reading latency proc file Reading /proc//latency or /proc//task//latency could cause NULL pointer dereference. In lstats_open(), get_proc_task() can return NULL, in which case the kernel will oops at lstats_show_proc() because m->private is NULL. When get_proc_task() returns NULL, the kernel should return -ENOENT. This can be reproduced by the following script. while : do date bash -c 'ls > ls.$$' & pid=$! cat /proc/$pid/latency & cat /proc/$pid/latency & cat /proc/$pid/latency & cat /proc/$pid/latency done Signed-off-by: Hiroshi Shimamoto Signed-off-by: Ingo Molnar --- fs/proc/base.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/proc/base.c b/fs/proc/base.c index 96ee899d6502..989e3078d7af 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -350,6 +350,8 @@ static int lstats_open(struct inode *inode, struct file *file) struct seq_file *m; struct task_struct *task = get_proc_task(inode); + if (!task) + return -ENOENT; ret = single_open(file, lstats_show_proc, NULL); if (!ret) { m = file->private_data; From d6643d12cb0885d06a1491b16c1476abcbd53d40 Mon Sep 17 00:00:00 2001 From: Hiroshi Shimamoto Date: Thu, 14 Feb 2008 10:27:00 -0800 Subject: [PATCH 7/8] latencytop: fix memory leak on latency proc file At lstats_open(), calling get_proc_task() gets task struct, but it never put. put_task_struct() should be called when releasing. Signed-off-by: Hiroshi Shimamoto Signed-off-by: Ingo Molnar --- fs/proc/base.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/fs/proc/base.c b/fs/proc/base.c index 989e3078d7af..85e06e498078 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -360,6 +360,15 @@ static int lstats_open(struct inode *inode, struct file *file) return ret; } +static int lstats_release(struct inode *inode, struct file *file) +{ + struct seq_file *m = file->private_data; + struct task_struct *task = m->private; + + put_task_struct(task); + return single_release(inode, file); +} + static ssize_t lstats_write(struct file *file, const char __user *buf, size_t count, loff_t *offs) { @@ -378,7 +387,7 @@ static const struct file_operations proc_lstats_operations = { .read = seq_read, .write = lstats_write, .llseek = seq_lseek, - .release = single_release, + .release = lstats_release, }; #endif From 13d77c37cab2bb906022309e1e7182c327e49916 Mon Sep 17 00:00:00 2001 From: Hiroshi Shimamoto Date: Wed, 20 Feb 2008 16:53:29 -0800 Subject: [PATCH 8/8] latencytop: change /proc task_struct access method Change getting task_struct by get_proc_task() at read or write time, and returns -ESRCH if get_proc_task() returns NULL. This is same behavior as other /proc files. Signed-off-by: Hiroshi Shimamoto Signed-off-by: Ingo Molnar --- fs/proc/base.c | 40 ++++++++++++---------------------------- 1 file changed, 12 insertions(+), 28 deletions(-) diff --git a/fs/proc/base.c b/fs/proc/base.c index 85e06e498078..91a1bd67ac1d 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -314,9 +314,12 @@ static int proc_pid_schedstat(struct task_struct *task, char *buffer) static int lstats_show_proc(struct seq_file *m, void *v) { int i; - struct task_struct *task = m->private; - seq_puts(m, "Latency Top version : v0.1\n"); + struct inode *inode = m->private; + struct task_struct *task = get_proc_task(inode); + if (!task) + return -ESRCH; + seq_puts(m, "Latency Top version : v0.1\n"); for (i = 0; i < 32; i++) { if (task->latency_record[i].backtrace[0]) { int q; @@ -341,43 +344,24 @@ static int lstats_show_proc(struct seq_file *m, void *v) } } + put_task_struct(task); return 0; } static int lstats_open(struct inode *inode, struct file *file) { - int ret; - struct seq_file *m; - struct task_struct *task = get_proc_task(inode); - - if (!task) - return -ENOENT; - ret = single_open(file, lstats_show_proc, NULL); - if (!ret) { - m = file->private_data; - m->private = task; - } - return ret; -} - -static int lstats_release(struct inode *inode, struct file *file) -{ - struct seq_file *m = file->private_data; - struct task_struct *task = m->private; - - put_task_struct(task); - return single_release(inode, file); + return single_open(file, lstats_show_proc, inode); } static ssize_t lstats_write(struct file *file, const char __user *buf, size_t count, loff_t *offs) { - struct seq_file *m; - struct task_struct *task; + struct task_struct *task = get_proc_task(file->f_dentry->d_inode); - m = file->private_data; - task = m->private; + if (!task) + return -ESRCH; clear_all_latency_tracing(task); + put_task_struct(task); return count; } @@ -387,7 +371,7 @@ static const struct file_operations proc_lstats_operations = { .read = seq_read, .write = lstats_write, .llseek = seq_lseek, - .release = lstats_release, + .release = single_release, }; #endif