Messages in this thread |  | | Date | Thu, 17 Jan 2013 16:36:38 -0600 | From | Nathan Zimmer <> | Subject | Re: [PATCH RESEND 1/4] sched: /proc/sched_stat fails on very very large machines. |
| |
On 01/16/2013 03:53 PM, Andrew Morton wrote: > On Tue, 15 Jan 2013 15:46:09 -0600 > Nathan Zimmer <nzimmer@sgi.com> wrote: > >> On systems with 4096 cores doing a cat /proc/sched_stat fails. >> We are trying to push all the data into a single kmalloc buffer. >> The issue is on these very large machines all the data will not fit in 4mb. >> >> A better solution is to not us the single_open mechanism but to provide >> our own seq_operations. >> >> The output should be identical to previous version and thus not need the >> version number. >> >> ... >> >> index 903ffa9..33a85c9 100644 >> --- a/kernel/sched/stats.c >> +++ b/kernel/sched/stats.c >> @@ -21,9 +21,13 @@ static int show_schedstat(struct seq_file *seq, void *v) >> if (mask_str == NULL) >> return -ENOMEM; >> >> - seq_printf(seq, "version %d\n", SCHEDSTAT_VERSION); >> - seq_printf(seq, "timestamp %lu\n", jiffies); >> - for_each_online_cpu(cpu) { >> + if (v == (void *)1) { > The magic-numbers-in-pointers at least need comments, please. Or nice > and meaningful #defines. > >> + seq_printf(seq, "version %d\n", SCHEDSTAT_VERSION); >> + seq_printf(seq, "timestamp %lu\n", jiffies); > The code leaks the memory at mask_str here. > >> + } else { >> + >> + cpu = (unsigned long)(v - 2); >> + >> struct rq *rq = cpu_rq(cpu); >> #ifdef CONFIG_SMP >> struct sched_domain *sd; >> @@ -72,35 +76,64 @@ static int show_schedstat(struct seq_file *seq, void *v) >> } >> rcu_read_unlock(); >> #endif >> + kfree(mask_str); >> } >> - kfree(mask_str); >> return 0; >> } > Undoing this change will fix the leak. > > The schedstats code (both the original and after the patch) appears to > be racy against cpu hotplug? What prevents the rq from vanishing while > we're playing with it? > > Looking at other usages people seem to be quite willing to just read a variable here and there without locking. The structure is a percpu structure so I don't believe rq will vanish, perhaps the backing data might become meaningless though...
|  |