netdev - Re: [RFC][BNX2X]: New driver for Broadcom 10Gb Ethernet.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1186091301.18322.46.camel@dell>
Date:	Thu, 02 Aug 2007 14:48:21 -0700
From:	"Michael Chan" <mchan@...adcom.com>
To:	"Michael Buesch" <mb@...sch.de>
cc:	"David Miller" <davem@...emloft.net>, jeff@...zik.org,
	"netdev" <netdev@...r.kernel.org>, eliezert@...adcom.com,
	lusinsky@...adcom.com, eilong@...adcom.com
Subject: Re: [RFC][BNX2X]: New driver for Broadcom 10Gb Ethernet.

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 majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html