Re: [PATCH] Statclock aliasing by LAPIC

On Friday 15 January 2010 10:06:04 am Attilio Rao wrote:
2009/11/30 John Baldwin <jhb@xxxxxxxxxxx>:
On Friday 27 November 2009 6:42:50 pm Attilio Rao wrote:
Handling all the three clocks (hardclock, softclock, profclock) within
the LAPIC can lead to aliasing for the softclock and profclock because
hz is sized to fit mainly hardclock. The fashion to handle all of them
within the LAPIC was introduced in 2005 and before than the softclock
and profclock were supposed to be handled in the rtc. Right now, too,
there is the necessary support to handle profclock and statclock in
atrtc which gets enabled if the LAPIC signals it can't take in charge
the three clocks.
The proposed patch reverts the situation preferring the atrtc to
handle the statclock and profclock (then a different source from the
LAPIC) and then avoid the aliasing problem:

http://www.freebsd.org/~attilio/Sandvine/STABLE_8/statclock_aliasing/statclock_aliasing3.diff

In this patch, lapic_setup_clock() has been changed in order to return
a three-states variable which identified if the LAPIC got in charge
all the three clocks, just the hardclock or none of them (the current
situation is just none/all) and the rtc handling runs subsequently.
A tunable as been added to force LAPI to get in charge all the three
clocks if needed.
In ia32 atrtc compiling is linked to atpic compiling, so a compile
time flag has been added to check if atpic is not present and in case
force LAPIC to take in charge all the three clocks (which is still
better than the 'safe belt values' still present in the rtc code).

Please note that statclock and profclock are widely used in our kernel
(rusage is, for example, statclock driven) and fixing this would
result in specific improvements (as a several-reported wrong CPU usage
statistic in top).
This bug has been found by Sandvine Incorporated.

Reviews, comments and testing are welcome.

Presumably in the RTC case lapic_timer_hz should always be hz and not some
multiple of hz. Also, did you check to make sure all the lock is present? I
think at one point I changed the locking for the RTC and/or ISA timer to just
use critical_enter/exit so that UP machines running an SMP kernel wouldn't pay
the locking overhead since the code was only used on UP machines. It may very
well be that I only changed that in a development branch though and not in
HEAD.

I still see clock_lock in place (and no particular critical section
code in that paths) or you meant to say that the clock_lock doesn't
still provide enough protection alone?

I think I misremembered. I have had local patches (possibly committed?) to
make the atpic driver just use critical_enter/exit, but I don't think I
changed the clock drivers.

--
John Baldwin
_______________________________________________
freebsd-arch@xxxxxxxxxxx mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@xxxxxxxxxxx"

Relevant Pages

  • Re: [PATCH] Statclock aliasing by LAPIC
    ... and profclock were supposed to be handled in the rtc. ... atrtc which gets enabled if the LAPIC signals it can't take in charge ... all the three clocks, just the hardclock or none of them (the current ... situation is just none/all) and the rtc handling runs subsequently. ...
    (freebsd-arch)
  • Re: [PATCH] Statclock aliasing by LAPIC
    ... and profclock were supposed to be handled in the rtc. ... atrtc which gets enabled if the LAPIC signals it can't take in charge ... all the three clocks, just the hardclock or none of them (the current ... situation is just none/all) and the rtc handling runs subsequently. ...
    (freebsd-arch)
  • [PATCH] Statclock aliasing by LAPIC
    ... Handling all the three clocks (hardclock, softclock, profclock) within ...
    (freebsd-arch)