Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 27 Sep 2019 17:57:30 +0200
From:   Peter Zijlstra <peterz@infradead.org>
To:     Andrea Parri <parri.andrea@gmail.com>
Cc:     David Howells <dhowells@redhat.com>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Will Deacon <will@kernel.org>,
        "Paul E. McKenney" <paulmck@linux.ibm.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Linux List Kernel Mailing <linux-kernel@vger.kernel.org>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        Nick Desaulniers <ndesaulniers@google.com>,
        jose.marchesi@oracle.com
Subject: Re: Do we need to correct barriering in circular-buffers.rst?
Message-ID: <20190927155730.GA11194@worktop.programming.kicks-ass.net>
References: <CAHk-=wj85tOp8WjcUp6gwstp4Cg2WT=p209S=fOzpWAgqqQPKg@mail.gmail.com>
 <20190915145905.hd5xkc7uzulqhtzr@willie-the-truck>
 <25289.1568379639@warthog.procyon.org.uk>
 <28447.1568728295@warthog.procyon.org.uk>
 <20190917170716.ud457wladfhhjd6h@willie-the-truck>
 <15228.1568821380@warthog.procyon.org.uk>
 <5385.1568901546@warthog.procyon.org.uk>
 <20190923144931.GC2369@hirez.programming.kicks-ass.net>
 <20190927095107.GA13098@andrea>
 <20190927124929.GB4643@worktop.programming.kicks-ass.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190927124929.GB4643@worktop.programming.kicks-ass.net>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Sep 27, 2019 at 02:49:29PM +0200, Peter Zijlstra wrote:
> On Fri, Sep 27, 2019 at 11:51:07AM +0200, Andrea Parri wrote:
> 
> > For the record, the LKMM doesn't currently model "order" derived from
> > control dependencies to a _plain_ access (even if the plain access is
> > a write): in particular, the following is racy (as far as the current
> > LKMM is concerned):
> > 
> > C rb
> > 
> > { }
> > 
> > P0(int *tail, int *data, int *head)
> > {
> > 	if (READ_ONCE(*tail)) {
> > 		*data = 1;
> > 		smp_wmb();
> > 		WRITE_ONCE(*head, 1);
> > 	}
> > }
> > 
> > P1(int *tail, int *data, int *head)
> > {
> > 	int r0;
> > 	int r1;
> > 
> > 	r0 = READ_ONCE(*head);
> > 	smp_rmb();
> > 	r1 = *data;
> > 	smp_mb();
> > 	WRITE_ONCE(*tail, 1);
> > }
> > 
> > Replacing the plain "*data = 1" with "WRITE_ONCE(*data, 1)" (or doing
> > s/READ_ONCE(*tail)/smp_load_acquire(tail)) suffices to avoid the race.
> > Maybe I'm short of imagination this morning...  but I can't currently
> > see how the compiler could "break" the above scenario.
> 
> The compiler; if sufficiently smart; is 'allowed' to change P0 into
> something terrible like:
> 
> 	*data = 1;
> 	if (*tail) {
> 		smp_wmb();
> 		*head = 1;
> 	} else
> 		*data = 0;
> 
> 
> (assuming it knows *data was 0 from a prior store or something)
> 
> Using WRITE_ONCE() defeats this because volatile indicates external
> visibility.

The much simpler solution might be writing it like:

	if (READ_ONCE(*tail) {
		barrier();
		*data = 1;
		smp_wmb();
		WRITE_ONCE(*head, 1);
	}

which I don't think the compiler is allowed to mess up.