On Thu, 2007-08-02 at 00:06 +0200, Michael Buesch wrote:
> +static inline u32 bnx2x_tx_avail(struct bnx2x_fastpath *fp)
>
> Too big for inlining.
>
> > +{
> > + u16 used;
> > + u32 prod = fp->tx_bd_prod;
> > + u32 cons = fp->tx_bd_cons;
> > +
> > + smp_mb();
>
> This barrier needs a comment. Why is it there? And why SMP only?
bnx2 and tg3 have similar logic to tell the compiler that prod and cons
can change. Strictly speaking, we can just use barrier(). The barrier
is also not placed correctly and should be:
/* Tell compiler that prod and cons can change. */
barrier();
prod = fp->tx_bd_prod;
cons = fp->tx_bd_cons;
...
>
> > + fp->tx_pkt_cons = sw_cons;
> > + fp->tx_bd_cons = bd_cons;
> > +
> > + smp_mb();
>
> Please add a comment why we need a SMP MB here.
This is again similar to logic in tg3 and bnx2 and the comments in tg3
are:
/* Need to make the tx_cons update visible to tg3_start_xmit()
* before checking for netif_queue_stopped(). Without the
* memory barrier, there is a small possibility that tg3_start_xmit()
* will miss it and cause the queue to be stopped forever.
*/
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
