Re: [PATCH] bnx2x: fix occasional statistics off-by-4GB error

[Eric Dumazet <eric.dumazet@gmail.com>]

On Sun, 2013-03-17 at 22:53 +0200, Eilon Greenstein wrote:

> This is not such a trivial issue - the HW/FW is guaranteeing the atomic
> read and this is why we can always use 32b variables.
> 

I am glad you qualify the following code as 'trivial'

/* difference = minuend - subtrahend */
#define DIFF_64(d_hi, m_hi, s_hi, d_lo, m_lo, s_lo) \
        do { \
                if (m_lo < s_lo) { \
                        /* underflow */ \
                        d_hi = m_hi - s_hi; \
                        if (d_hi > 0) { \
                                /* we can 'loan' 1 */ \
                                d_hi--; \
                                d_lo = m_lo + (UINT_MAX - s_lo) + 1; \
                        } else { \
                                /* m_hi <= s_hi */ \
                                d_hi = 0; \
                                d_lo = 0; \
                        } \
                } else { \
                        /* m_lo >= s_lo */ \
                        if (m_hi < s_hi) { \
                                d_hi = 0; \
                                d_lo = 0; \
                        } else { \
                                /* m_hi >= s_hi */ \
                                d_hi = m_hi - s_hi; \
                                d_lo = m_lo - s_lo; \
                        } \
                } \
        } while (0)


All the macros and _hi/_ho fields found in
drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.h
are really badly designed.

It hurts so much the common sense its not even funny.

All you really needed is to have normal host structures containing u64
fields, and you only needed one single helper to convert from hw u32:u32
to host u64.

Because gcc handles u64 operations very well, even on 32bit hosts, and
it knows about the carry flag.