Re: [PATCH v4 2/2] change buffer_locked, so that it has acquire semantics

[Matthew Wilcox]

On Mon, Aug 01, 2022 at 06:43:55AM -0400, Mikulas Patocka wrote:
> Let's have a look at this piece of code in __bread_slow:
> 	get_bh(bh);
> 	bh->b_end_io = end_buffer_read_sync;
> 	submit_bh(REQ_OP_READ, 0, bh);
> 	wait_on_buffer(bh);
> 	if (buffer_uptodate(bh))
> 		return bh;
> Neither wait_on_buffer nor buffer_uptodate contain any memory barrier.
> Consequently, if someone calls sb_bread and then reads the buffer data,
> the read of buffer data may be executed before wait_on_buffer(bh) on
> architectures with weak memory ordering and it may return invalid data.
> 
> Fix this bug by changing the function buffer_locked to have the acquire
> semantics - so that code that follows buffer_locked cannot be moved before
> it.

I think this is the wrong approach.  Instead, buffer_set_uptodate()
should have the smp_wmb() and buffer_uptodate should have the smp_rmb().
Just like the page flags.  As I said last night.