Re: [PATCH] proc/schedstat: Expose /proc/<pid>/schedstat if delay accounting is enabled

[Naveen N. Rao]

On 2015/06/02 09:58AM, Ingo Molnar wrote:
> 
> * Naveen N. Rao <naveen.n.rao@xxxxxxxxxxxxxxxxxx> wrote:
> 
> > On 2015/05/29 11:54AM, Ingo Molnar wrote:
> > > 
> > > > On a related note, even though sum_exec_runtime is available 
> > > > unconditionally, I dump all zeroes in my patch if 
> > > > !sched_info_on() to make it clear that some of the fields are 
> > > > not available. Is this ok or should be display sum_exec_runtime 
> > > > regardless of sched_info?
> > > 
> > > So I'd suggest printing -1 for non-available fields, that should be unambigous 
> > > enough and makes it also possible to write out 0 in some cases.
> > 
> > Per Documentation/scheduler/sched-stats.txt (and the linked latency.c there), 
> > user-space seems to be expecting unsigned values here. Would displaying -1 here 
> > be ok?
> 
> Probably not (the code is silly, why doesn't it split up the string and use 
> atol()?) - hopefully real user-space is better? Can you try some real, packaged up 
> tools that read schedstats, to see whether they work with -1?

Hi Ingo,
Sorry for the delay - I had been off on vacation.

I see that quite a few packages are using /proc/<pid>/schedstat - pcp, 
systemd, dstat, android, among others. While most of these seem to be 
splitting up the fields properly, they are using a variant of 
strtoull(), which returns ULLONG_MAX for -1, and none of these check for 
that condition. If any of the tools use the value read to report total 
execution time or run delay, it will be incorrect.

At this point, I feel it is better to display all the three fields in 
schedstat only if sched_info_on() is true, as explained above. What do 
you suggest?


Thanks,
Naveen

--
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/