Date: Tue, 12 Dec 2006 18:24:43 -0500 From: Jung-uk Kim <jkim@FreeBSD.org> To: Oleg Bulyzhin <oleg@FreeBSD.org> Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/dev/bge if_bge.c Message-ID: <200612121824.45643.jkim@FreeBSD.org> In-Reply-To: <20061212222549.GD91560@lath.rinet.ru> References: <200612111800.kBBI0Zv0047909@repoman.freebsd.org> <20061212222549.GD91560@lath.rinet.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
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. > 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). Jung-uk Kim
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200612121824.45643.jkim>