lkml.org 
[lkml]   [2007]   [Mar]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch] sched: accurate user accounting
On Mon, 26 Mar 2007, Con Kolivas wrote:

> On Monday 26 March 2007 00:57, malc wrote:
>> On Mon, 26 Mar 2007, Con Kolivas wrote:
>>> On Sunday 25 March 2007 23:06, malc wrote:
>>>> On Sun, 25 Mar 2007, Con Kolivas wrote:
>>>>> On Sunday 25 March 2007 21:46, Con Kolivas wrote:
>>>>>> On Sunday 25 March 2007 21:34, malc wrote:
>>>>>>> On Sun, 25 Mar 2007, Ingo Molnar wrote:
>>>>>>>> * Con Kolivas <kernel@kolivas.org> wrote:
>>>>>>>>> For an rsdl 0.33 patched kernel. Comments? Overhead worth it?
>>>>
>>>> [..snip..]
>>>>
>>>>> ---
>>>>> Currently we only do cpu accounting to userspace based on what is
>>>>> actually happening precisely on each tick. The accuracy of that
>>>>> accounting gets progressively worse the lower HZ is. As we already keep
>>>>> accounting of nanosecond resolution we can accurately track user cpu,
>>>>> nice cpu and idle cpu if we move the accounting to update_cpu_clock
>>>>> with a nanosecond cpu_usage_stat entry. This increases overhead
>>>>> slightly but avoids the problem of tick aliasing errors making
>>>>> accounting unreliable.
>>>>>
>>>>> Signed-off-by: Con Kolivas <kernel@kolivas.org>
>>>>> Signed-off-by: Ingo Molnar <mingo@elte.hu>
>>>>
>>>> [..snip..]
>>>>
>>>> Forgot to mention. Given that this goes into the kernel, shouldn't
>>>> Documentation/cpu-load.txt be amended/removed?
>>>
>>> Yes that's a good idea. Also there should be a sanity check because
>>> sometimes for some reason noone's been able to explain to me sched_clock
>>> gives a value which doesn't make sense (time appears to have gone
>>> backwards) and that will completely ruin the accounting from then on.
>>
>> After running this new kernel for a while i guess i have hit this issue:
>> http://www.boblycat.org/~malc/apc/bad-load.png
>>
>> Top and icewm's monitor do show incredibly huge load while in reality
>> nothing like that is really happening. Both ad-hoc and `/proc/stat' (idle)
>> show normal CPU utilization (7% since i'm doing some A/V stuff in the
>> background)
>
> Yes I'd say you hit the problem I described earlier. When playing with
> sched_clock() I found it gave some "interesting" results fairly infrequently.
> They could lead to ridiculous accounting mistakes.
>
> So before we go any further with this patch, can you try the following one and
> see if this simple sanity check is enough?

Sure (compiling the kernel now), too bad old axiom that testing can not
confirm absence of bugs holds.

I have one nit and one request from clarification. Question first (i
admit i haven't looked at the surroundings of the patch maybe things
would have been are self evident if i did):

What this patch amounts to is that the accounting logic is moved from
timer interrupt to the place where scheduler switches task (or something
to that effect)?

[..snip..]

> * These are the 'tuning knobs' of the scheduler:
> @@ -3017,8 +3018,53 @@ EXPORT_PER_CPU_SYMBOL(kstat);
> static inline void
> update_cpu_clock(struct task_struct *p, struct rq *rq, unsigned long long now)
> {
> - p->sched_time += now - p->last_ran;
> + struct cpu_usage_stat *cpustat = &kstat_this_cpu.cpustat;
> + cputime64_t time_diff;
> +
> p->last_ran = rq->most_recent_timestamp = now;
> + /* Sanity check. It should never go backwards or ruin accounting */
> + if (unlikely(now < p->last_ran))
> + return;
> + time_diff = now - p->last_ran;

A nit. Anything wrong with:

time_diff = now - p->last_ran;
if (unlikeley (LESS_THAN_ZERO (time_diff))
return;

[..snip..]

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

\
 
 \ /
  Last update: 2007-03-25 17:23    [W:1.129 / U:0.144 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site