netdev - Re: [RFC PATCH] bnx2x: fix tx queue locking and memory barriers

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: <31550.1268241570@redhat.com>
Date:	Wed, 10 Mar 2010 17:19:30 +0000
From:	David Howells <dhowells@...hat.com>
To:	Stanislaw Gruszka <sgruszka@...hat.com>,
	Vladislav Zolotarov <vladz@...adcom.com>,
	David Miller <davem@...emloft.net>
Cc:	dhowells@...hat.com, paulmck@...ux.vnet.ibm.com,
	netdev@...r.kernel.org, Eilon Greenstein <eilong@...adcom.com>
Subject: Re: [RFC PATCH] bnx2x: fix tx queue locking and memory barriers

David Howells <dhowells@...hat.com> wrote:

> > -	barrier(); /* Tell compiler that prod and cons can change */
> > +	/* prod and cons can change on other cpu, want to see
> > +	   consistend available space and queue (stop/running) state */
> > +	smp_mb();
> > +
> >  	prod = fp->tx_bd_prod;
> >  	cons = fp->tx_bd_cons;
> 
> I suspect that this isn't what you want.
> 
> The barrier() didn't tell the compiler that fp->tx_bd_prod and fp->tx_bd_cons
> could change.  What it did was to say that the accesses to those two variables
> must be performed after all the other accesses issued by that CPU prior to the
> barrier - at least as far as the compiler is concerned.
> 
> You don't need to separate the reads of tx_bd_prod and tx_bd_cons above with a
> memory barrier.  They aren't ever altered in the same place.

Having said that, you might need a memory barrier before reading tx_bd_prod in
the consumer if the producer waggles a flag in memory to indicate to the
consumer that it should consume, and a memory barrier in the producer before
waggling that flag:

	[producer]
	...
	smp_wmb(); /* commit buffer contents before incrementing index */
	fp->tx_bd_prod = TX_BD(bd_prod + 1);
	smp_wmb(); /* commit increment index before prodding consumer */
	prod_consumer();

	[consumer]
	check_prod_flag();
	smp_rmb(); /* read producer index after checking prod flag */
	bd_prod = fp->tx_bd_prod;
	bd_cons = fp->tx_bd_cons;
	smp_read_barrier_depends(); /* read index before reading contents */

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