Re: [Patch 2/7] Add sysctl for schedstats

[Shailabh Nagar]

Nick Piggin wrote:

<snip>

> > +int schedstats_sysctl_handler(ctl_table *table, int write, struct 
> > file *filp,
> > +            void __user *buffer, size_t *lenp, loff_t *ppos)
> > +{
> > +    int ret, prev = schedstats_sysctl;
> > +    struct task_struct *g, *t;
> > +
> > +    ret = proc_dointvec(table, write, filp, buffer, lenp, ppos);
> > +    if ((ret != 0) || (prev == schedstats_sysctl))
> > +        return ret;
> > +    if (schedstats_sysctl) {
> > +        read_lock(&tasklist_lock);
> > +        do_each_thread(g, t) {
> > +            memset(&t->sched_info, 0, sizeof(t->sched_info));
> > +        } while_each_thread(g, t);
> > +        read_unlock(&tasklist_lock);
> > +    }
> > +    schedstats_set(schedstats_sysctl);
>
>
> You don't clear the rq's schedstats stuff here.

Good point.

> And clearing this at all is not really needed for the schedstats 
> interface.
> You have a timestamp and a set of accumulated values, so it is easy to 
> work
> out deltas. So do you need this?

Not clearing the stats will mean userspace has to distinguish between 
the tasks
that are hanging around from before the last turn off, and the ones 
created after
wards. Any delta taken across an interval where schedstats was turned 
off will
give the impression a task was sleeping during the interval (and hence 
show it had
a lesser average wait time than it might actually have experienced). 

--Shailabh
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/