Messages in this thread | | | Date | Sun, 25 Mar 2007 19:19:21 +0400 (MSD) | From | malc <> | Subject | Re: [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/
| |