Re: [PATCH] Display -rt related stats in /proc/schedstat

[John Sigler <linux.kernel@free.fr>]

Ankita Garg wrote:
> On Tue, Jul 24, 2007 at 12:36:22PM +0200, John Sigler wrote:
>> Ankita Garg wrote:
>>
>>> This patch adds support to display captured -rt stats under 
>>> /proc/schedstat.
>>>
>>> Signed-off-by: Ankita Garg <ankita@in.ibm.com>
>>> --
>>> sched_stats.h |    5 ++++-
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> Index: linux-2.6.22.1/kernel/sched_stats.h
>>> ===================================================================
>>> --- linux-2.6.22.1.orig/kernel/sched_stats.h	2007-07-23 
>>> 18:15:57.000000000 +0530
>>> +++ linux-2.6.22.1/kernel/sched_stats.h	2007-07-23 
>>> 18:24:03.000000000 +0530
>>> @@ -21,12 +21,15 @@
>>>
>>> 		/* runqueue-specific stats */
>>> 		seq_printf(seq,
>>> -		    "cpu%d %lu %lu %lu %lu %lu %lu %lu %lu %lu %llu %llu 
>>> %lu",
>>> +		    "cpu%d %lu %lu %lu %lu %lu %lu %lu %lu %lu %llu %lu %lu 
>>> %lu %llu %lu",
>>> 		    cpu, rq->yld_both_empty,
>>> 		    rq->yld_act_empty, rq->yld_exp_empty, rq->yld_cnt,
>>> 		    rq->sched_switch, rq->sched_cnt, rq->sched_goidle,
>>> 		    rq->ttwu_cnt, rq->ttwu_local,
>>> 		    rq->rq_sched_info.cpu_time,
>>> +#ifdef CONFIG_PREEMPT_RT
>>> +		    rq->rto_schedule, rq->rto_wakeup, rq->rto_pulled,
>>> +#endif
>>> 		    rq->rq_sched_info.run_delay, rq->rq_sched_info.pcnt);
>>>
>>> 		seq_printf(seq, "\n");
>>
>> I know you've written a new patch that makes this one obsolete, but I 
>> wanted to discuss this specific patch.
>>
>> Originally, there were 13 conversion specifications, and your patch
>> adds 3, for a total of 16 conversion specifications.
>>
>> However, if CONFIG_PREEMPT_RT is not defined, then seq_printf is
>> called with only 13 arguments.
>>
>> Is it safe to call seq_printf with too few arguments?
> 
> So I tried to run the code disabling CONFIG_PREEMPT_RT, to see the
> behavior vsnprintf() on supplying fewer arguments. Arbit/garbage value is
> printed for conversion specifications that do not have a matching argument.
> It did not cause any issue at all beyond that point. But considering the
> correctness and quality of code, was going to post the updated patch (if
> the re-work had not happened)!

I double-checked. Calling v*printf with too few parameters also leads to 
undefined behavior.

You didn't notice any ill effects on your particular compiler / platform 
combination, but the code above remains a time bomb :-)

Once in UB land, all bets are off.

Regards.