lkml.org 
[lkml]   [2013]   [Jan]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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...




    \
     
     \ /
      Last update: 2013-01-18 00:24    [W:4.168 / U:0.124 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site