Date: Fri, 3 Dec 2010 19:38:09 -0500 From: Jung-uk Kim <jkim@FreeBSD.org> To: freebsd-current@FreeBSD.org Cc: Andriy Gapon <avg@freebsd.org> Subject: Re: non-invariant tsc and cputicker Message-ID: <201012031938.12684.jkim@FreeBSD.org> In-Reply-To: <4CF98192.3050909@freebsd.org> References: <4CF92852.20705@freebsd.org> <201012031504.02532.jkim@FreeBSD.org> <4CF98192.3050909@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Friday 03 December 2010 06:47 pm, Andriy Gapon wrote: > on 03/12/2010 22:03 Jung-uk Kim said the following: > > On Friday 03 December 2010 01:14 pm, Andriy Gapon wrote: > >> on 03/12/2010 20:05 Jung-uk Kim said the following: > >>> On Friday 03 December 2010 12:26 pm, Andriy Gapon wrote: > >>>> FreeBSD uses cpu_ticks [function pointer] in a few places for > >>>> a few things like process CPU time accounting. On x86 > >>>> cpu_ticks always points to rdtsc. If TSC is not invariant that > >>>> leads to incorrect accounting of "CPU ticks". The code > >>>> pretends to try to handle changing cpufreq levels, but does > >>>> that incorrectly. > >>> > >>> Arg... Probably it is my fault. :-( > >>> > >>>> I think that we could use a selected timecounter instead of > >>>> "raw" TSC if the latter is not invariant. In this case > >>>> cpu_ticks calls would be slightly costlier, but always > >>>> correct. > >>>> > >>>> The change is quite trivial: > >>>> http://people.freebsd.org/~avg/tsc-cputicker.diff > >>>> > >>>> What do you think? > >>> > >>> Why don't we just fix it properly? > >> > >> Patch? :-) > > > > Attached. > > I fail to see how this corrects the calculations (cpu tick > accumulation) in !invariant_tsc case. Sorry, I interpreted your problem backwards. :-( > >> It seems that it is not too trivial to do and is prone to error > >> accumulation given how the ticks are added up. > >> Besides, why using a timecounter would not be a proper fix? > > > > Well, it is not that simple, unfortunately. Because init_TSC() > > is called very early, your patch will select dummy timecounter as > > a CPU ticker if my memory serves. It is very hard to implement > > right on x86 arch. :-( > > I don't think that init_TSC() is called earlier than the code that > probes CPU features. After all, presence of TSC is another CPU > feature. If my understanding is correct, your patch uses the dummy timecounter until a real timecounter is chosen. When a real timecounter is set, tc_cpu_ticks() changes the frequency naturally. How are you going to solve this problem? What should we do when a user set a new timecounter hardware via "sysctl kern.timecounter.hardware"? I don't think it is any better than current code. Am I missing something? :-( Jung-uk Kim
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201012031938.12684.jkim>