From owner-cvs-src@FreeBSD.ORG Wed Dec 13 00:04:15 2006 Return-Path: X-Original-To: cvs-src@FreeBSD.org Delivered-To: cvs-src@FreeBSD.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 025B416A415; Wed, 13 Dec 2006 00:04:15 +0000 (UTC) (envelope-from oleg@lath.rinet.ru) Received: from lath.rinet.ru (lath.rinet.ru [195.54.192.90]) by mx1.FreeBSD.org (Postfix) with ESMTP id ED73D43CD0; Wed, 13 Dec 2006 00:02:29 +0000 (GMT) (envelope-from oleg@lath.rinet.ru) Received: from lath.rinet.ru (localhost [127.0.0.1]) by lath.rinet.ru (8.13.8/8.13.8) with ESMTP id kBD03rtO096132 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 13 Dec 2006 03:03:53 +0300 (MSK) (envelope-from oleg@lath.rinet.ru) Received: (from oleg@localhost) by lath.rinet.ru (8.13.8/8.13.8/Submit) id kBD03rcq096131; Wed, 13 Dec 2006 03:03:53 +0300 (MSK) (envelope-from oleg) Date: Wed, 13 Dec 2006 03:03:53 +0300 From: Oleg Bulyzhin To: Jung-uk Kim Message-ID: <20061213000352.GH91560@lath.rinet.ru> References: <200612111800.kBBI0Zv0047909@repoman.freebsd.org> <20061212222549.GD91560@lath.rinet.ru> <200612121824.45643.jkim@FreeBSD.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200612121824.45643.jkim@FreeBSD.org> User-Agent: Mutt/1.5.13 (2006-08-11) Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/dev/bge if_bge.c X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 13 Dec 2006 00:04:15 -0000 On Tue, Dec 12, 2006 at 06:24:43PM -0500, Jung-uk Kim wrote: > On Tuesday 12 December 2006 05:25 pm, Oleg Bulyzhin wrote: > > On Mon, Dec 11, 2006 at 06:00:35PM +0000, Jung-uk Kim wrote: > > > jkim 2006-12-11 18:00:35 UTC > > > > > > FreeBSD src repository > > > > > > Modified files: > > > sys/dev/bge if_bge.c > > > Log: > > > - Correct collision counter for BCM5705+. This register is > > > read/clear. - Correct RX packet drop counter for BCM5705+. This > > > register is read/clear and it wraps very quickly under heavy > > > packet drops because only the lower ten bits are valid according > > > to the documentation. However, it seems few more bits are > > > actually valid and the rest bits are always zeros[1]. Therefore, > > > we don't mask them off here. To get accurate packet drop count, > > > we need to check the register from bge_rxeof(). It is commented > > > out for now, not to penalize normal operation. Actual > > > performance impact should be measured later. > > > - Correct integer casting from u_long to uint32_t. Casting is > > > not really needed for all supported platforms but we better do > > > this correctly[2]. > > > > > > Tested by: bde[1] > > > Suggested by: bde[2] > > > > > > Revision Changes Path > > > 1.158 +13 -10 src/sys/dev/bge/if_bge.c > > > > I didnt get the point of your u_long -> uint32_t changes. > > As i can see your change will cause more often wraps on 64bit > > archs: In rev. 1.153 you have converted cnt to uint32_t. Since cnt > > is stored in sc->bge_* counters you have shortened(on 64bit archs) > > them as well. > > Depending on the chip types, we use different methods to get the > stats. However, both methods read only 32-bit register/memory. > Therefore, there's no reason to use u_long at all and integer math is > cheap. While hardware counters are 32bit long, interface & driver's counters are 64bit long on 64bit chips. So you can keep numbers even if hardware counter wraps around. uint32_t & u_long are the same on i386 and u_long arithmetic is not slower than int one for 64 bit chips. Anyway we need few arithmetic operations once per second, so it's unimportant. > > > P.S. Your current change is unclear to me too: since ifp counters > > and sc->_bge_ are both u_long i can not see any reason of > > converting (u_long) cast to (uint32_t) one. > > To cut the long story short, it does not really matter as I said in > the log but it is required if sizeof(uint32_t) < sizeof(int). Could you please explain this. > > Jung-uk Kim -- Oleg. ================================================================ === Oleg Bulyzhin -- OBUL-RIPN -- OBUL-RIPE -- oleg@rinet.ru === ================================================================