From owner-svn-src-head@FreeBSD.ORG Tue Oct 28 09:55:34 2008 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 38E68106567E; Tue, 28 Oct 2008 09:55:34 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail06.syd.optusnet.com.au (mail06.syd.optusnet.com.au [211.29.132.187]) by mx1.freebsd.org (Postfix) with ESMTP id C5C318FC18; Tue, 28 Oct 2008 09:55:33 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from c122-106-151-199.carlnfd1.nsw.optusnet.com.au (c122-106-151-199.carlnfd1.nsw.optusnet.com.au [122.106.151.199]) by mail06.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id m9S9tSkT006706 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 28 Oct 2008 20:55:30 +1100 Date: Tue, 28 Oct 2008 20:55:28 +1100 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: John Baldwin In-Reply-To: <200810271411.11813.jhb@freebsd.org> Message-ID: <20081028202701.E85964@delplex.bde.org> References: <200810261858.m9QIw4YV091893@svn.freebsd.org> <200810271411.11813.jhb@freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: svn-src-head@freebsd.org, Maxim Sobolev , svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r184293 - in head/sys: amd64/amd64 i386/i386 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Oct 2008 09:55:34 -0000 On Mon, 27 Oct 2008, John Baldwin wrote: > On Sunday 26 October 2008 02:58:04 pm Maxim Sobolev wrote: >> Author: sobomax >> Date: Sun Oct 26 18:58:04 2008 >> New Revision: 184293 >> URL: http://svn.freebsd.org/changeset/base/184293 >> >> Log: >> Fix division by zero panic if kern.hz less than 32. >> >> MFC after: 1 day > > This is wrong. In the case you are worried about here, lapic_timer_hz is less > than 128. There is no way you are going to fire stathz 128 times per second > from a timer running at < 128 hz. You are effectively running stathz at > lapic_timer_hz, so I would just set stathz = lapic_timer_hz in this case. stathz needs to be about 128 to work as intended, at least for SCHED_4BSD. > Also, I would drop the extra {}'s to match style(9) as well as the existing > style of the file. I noticed this bug in the main commit too. Also, hz = 10 cannot work on i386 without lapic_timer, since the i8254 timer has a maximum interrupt period of 55 ms and thus a minimum frequency of 18.2 Hz. Attempts to set it to 10 Hz cause similar bugs to the ones here -- the best approximation of 18.2 is (supposed to be) used, but the system is not informed about the enormous error in this approximation and still thinks that 10 Hz is used. I use hz = stathz = 100 = lapic_timer_hz on all systems with lapic timer, mainly to avoid the large changes to scale all the timers perfectly. (This makes profhz = lapic_timer_hz = 100 more broken than before. Perfect scaling rarely matters, and asynchronicity of the timers is broken anyway.) hz != 100 is still supported in a simple way that makes stathz = 100 (and lapic_timer_hz unnecessarily large -- 1000) if hz < 100: lapic_timer_hz = howmany(imax(hz, 100), hz) * hz; stathz = lapic_timer_hz / (lapic_timer_hz < 100 ? 1 : lapic_timer_hz / 100); >> --- head/sys/amd64/amd64/local_apic.c Sun Oct 26 17:20:37 2008 (r184292) >> +++ head/sys/amd64/amd64/local_apic.c Sun Oct 26 18:58:04 2008 (r184293) >> @@ -401,7 +401,11 @@ lapic_setup_clock(void) >> lapic_timer_hz = hz * 2; >> else >> lapic_timer_hz = hz * 4; >> - stathz = lapic_timer_hz / (lapic_timer_hz / 128); >> + if (lapic_timer_hz < 128) { >> + stathz = 128; >> + } else { >> + stathz = lapic_timer_hz / (lapic_timer_hz / 128); >> + } Use of if/else instead a conditional expression as in the above may be another style bug here. Bruce