From owner-freebsd-arch@FreeBSD.ORG Fri Jan 15 15:25:32 2010 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 352B91065676; Fri, 15 Jan 2010 15:25:32 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id E3A4C8FC14; Fri, 15 Jan 2010 15:25:31 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 7A0CE46B29; Fri, 15 Jan 2010 10:25:31 -0500 (EST) Received: from jhbbsd.localnet (smtp.hudson-trading.com [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPA id 935268A024; Fri, 15 Jan 2010 10:25:30 -0500 (EST) From: John Baldwin To: Attilio Rao Date: Fri, 15 Jan 2010 10:25:06 -0500 User-Agent: KMail/1.12.1 (FreeBSD/7.2-CBSD-20091231; KDE/4.3.1; amd64; ; ) References: <3bbf2fe10911271542h2b179874qa0d9a4a7224dcb2f@mail.gmail.com> <200911301305.30572.jhb@freebsd.org> <3bbf2fe11001150706y765159a2jbd37c7ae4cf378f0@mail.gmail.com> In-Reply-To: <3bbf2fe11001150706y765159a2jbd37c7ae4cf378f0@mail.gmail.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <201001151025.06251.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Fri, 15 Jan 2010 10:25:30 -0500 (EST) X-Virus-Scanned: clamav-milter 0.95.1 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.6 required=4.2 tests=AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: FreeBSD Arch , Ed Maste Subject: Re: [PATCH] Statclock aliasing by LAPIC X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 15 Jan 2010 15:25:32 -0000 On Friday 15 January 2010 10:06:04 am Attilio Rao wrote: > 2009/11/30 John Baldwin : > > 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