cvs commit: src/sys/dev/bge if_bge.c

[Oleg Bulyzhin]

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 at rinet.ru ===
================================================================