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

Jung-uk Kim jkim at FreeBSD.org
Tue Dec 12 15:37:50 PST 2006


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


More information about the cvs-src mailing list