From: "Michael Chan" <mchan@broadcom.com>
To: "Michael Buesch" <mb@bu3sch.de>
Cc: "David Miller" <davem@davemloft.net>,
jeff@garzik.org, "netdev" <netdev@vger.kernel.org>,
eliezert@broadcom.com, lusinsky@broadcom.com,
eilong@broadcom.com
Subject: Re: [RFC][BNX2X]: New driver for Broadcom 10Gb Ethernet.
Date: Thu, 02 Aug 2007 14:48:21 -0700 [thread overview]
Message-ID: <1186091301.18322.46.camel@dell> (raw)
In-Reply-To: <200708020006.13457.mb@bu3sch.de>
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.
*/
next prev parent reply other threads:[~2007-08-02 20:54 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-08-01 8:31 [RFC][BNX2X]: New driver for Broadcom 10Gb Ethernet Michael Chan
2007-08-01 22:06 ` Michael Buesch
2007-08-01 22:28 ` Roland Dreier
2007-08-01 22:50 ` Jeff Garzik
2007-08-02 18:13 ` Eliezer Tamir
2007-08-02 18:09 ` Eliezer Tamir
2007-08-02 21:48 ` Michael Chan [this message]
2007-08-07 22:15 ` Jeff Garzik
2007-08-07 22:20 ` Michael Buesch
2007-08-07 23:04 ` Christoph Hellwig
2007-08-07 23:08 ` David Miller
2007-08-08 8:40 ` Michael Buesch
2007-08-07 23:04 ` Roland Dreier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1186091301.18322.46.camel@dell \
--to=mchan@broadcom.com \
--cc=davem@davemloft.net \
--cc=eilong@broadcom.com \
--cc=eliezert@broadcom.com \
--cc=jeff@garzik.org \
--cc=lusinsky@broadcom.com \
--cc=mb@bu3sch.de \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).