On Sun, Sep 01, 2024 at 10:50:37AM GMT, Alan Huang wrote: > > 2024年9月1日 09:09，Kent Overstreet <[email protected]> wrote： > > > > On Tue, Aug 27, 2024 at 11:14:48PM GMT, Alan Huang wrote: > >> If the reader acquires the read lock and then the writer enters the slow > >> path, while the reader proceeds to the unlock path, the following scenario > >> can occur without the change: > >> > >> writer: pcpu_read_count(lock) return 1 (so __do_six_trylock will return 0) > >> reader: this_cpu_dec(*lock->readers) > >> reader: smp_mb() > >> reader: state = atomic_read(&lock->state) (there is no waiting flag set) > >> writer: six_set_bitmask() > >> > >> then the writer will sleep forever. > >> > >> Signed-off-by: Alan Huang <[email protected]> > >> --- > >> fs/bcachefs/six.c | 12 +++++++++--- > >> 1 file changed, 9 insertions(+), 3 deletions(-) > >> > >> diff --git a/fs/bcachefs/six.c b/fs/bcachefs/six.c > >> index 3a494c5d1247..b3583c50ef04 100644 > >> --- a/fs/bcachefs/six.c > >> +++ b/fs/bcachefs/six.c > >> @@ -169,11 +169,17 @@ static int __do_six_trylock(struct six_lock *lock, > >> enum six_lock_type type, > >> ret = -1 - SIX_LOCK_write; > >> } > >> } else if (type == SIX_LOCK_write && lock->readers) { > >> - if (try) { > >> + if (try) > >> atomic_add(SIX_LOCK_HELD_write, &lock->state); > >> - smp_mb__after_atomic(); > >> - } > >> > >> + /* > >> + * Make sure atomic_add happens before pcpu_read_count and > >> + * six_set_bitmask in slow path happens before pcpu_read_count. > >> + * > >> + * Paired with the smp_mb() in read lock fast path (per-cpu mode) > >> + * and the one before atomic_read in read unlock path. > >> + */ > >> + smp_mb(); > > > > What is this barrier ordering? > > > > This is only a change in the !try case, and we didn't do anything before > > checking pcpu_read_count() - except way back in six_lock_slowpath() > > where we set SIX_LOCK_HELD_write, but that does have a barrier. > > Make sure six_set_bitmask(lock, SIX_LOCK_WAITING_read << type) > > happens before pcpu_read_count()
that's in another thread, a memory barrier orders things in the same thread
