netdev - Re: [PATCH 1/1] bnx2x: Tx barriers and locks

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]

Date:	Tue, 2 Mar 2010 08:18:44 -0800
From:	"Michael Chan" <mchan@...adcom.com>
To:	"'Stanislaw Gruszka'" <sgruszka@...hat.com>,
	"Vladislav Zolotarov" <vladz@...adcom.com>
cc:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"Eilon Greenstein" <eilong@...adcom.com>,
	"Matthew Carlson" <mcarlson@...adcom.com>
Subject: Re: [PATCH 1/1] bnx2x: Tx barriers and locks

Stanislaw Gruszka wrote:

> On Tue, Mar 02, 2010 at 04:50:59AM -0800, Vladislav Zolotarov wrote:
> > Stanislaw barrier() is not a memory barrier - it's a
> compiler barrier. I don't think removing it from
> bnx2x_tx_avail() will improve anything. If u think I'm wrong,
> could u, pls., provide a specific example.
>
> Only improvement is removing confusing code, And comment like
> "Tell compiler that prod and cons can change" is even more
> confusing. If you think I'm wrong, just tell as why that
> barrier is needed :)

The barrier (compiler barrier at least) is required in
bnx2x_tx_avail().  The status block index can be updated by DMA and
the compiler doesn't know it (because it is considered wrong to
declare the status block as volatile).  Near the end of
bnx2x_start_xmit() where we call bnx2x_tx_avail() twice.  It is
possible that the compiler will optimize it and not look at the
status block in memory the second time.

--
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