lkml.org 
[lkml]   [2022]   [Jul]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] Add a read memory barrier to wait_on_buffer


On Sun, 31 Jul 2022, Ard Biesheuvel wrote:

> This has little to do with speculation, so better to drop this S bomb
> from your commit message. This is about concurrency and weak memory
> ordering.

Yes.

> This doesn't seem like a very robust fix to me, tbh - I suppose this
> makes the symptom you encountered go away, but the underlying issue
> remains afaict.
>
> Given that the lock and uptodate fields etc are just bits in a
> bitfield, wouldn't it be better to use cmpxchg() with acquire/release
> semantics (as appropriate) to manage these bits?

The kernel already uses clear_bit_unlock, test_and_set_bit_lock and
wait_on_bit_lock_io to manage the BH_Lock bit - and they have
acquire/release semantics.

The only problem is that test_bit doesn't provide any memory barriers.
Should we add the barrier to buffer_locked() instead of wait_on_buffer()?
Perhaps it would fix more bugs - in reiserfs, there's this piece of code:

if (buffer_locked(bh)) {
spin_unlock(lock);
wait_on_buffer(bh);
spin_lock(lock);
}
if (!buffer_uptodate(bh)) {
ret = -EIO;
}

or this:
if (buffer_locked(bh)) {
int depth;
PROC_INFO_INC(sb, scan_bitmap.wait);
depth = reiserfs_write_unlock_nested(sb);
__wait_on_buffer(bh);
reiserfs_write_lock_nested(sb, depth);
}
BUG_ON(!buffer_uptodate(bh));
BUG_ON(atomic_read(&bh->b_count) == 0);

That assumes that buffer_locked provides a barrier.

Mikulas

\
 
 \ /
  Last update: 2022-07-31 15:41    [W:0.079 / U:0.540 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site