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

Shailabh Nagar
2006-02-27T09:41:42Z

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