> 2024年9月1日 09:09，Kent Overstreet <kent.overstr...@linux.dev> 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 <mmpgour...@gmail.com>
>> ---
>> 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()
>
>> ret = !pcpu_read_count(lock);
>>
>> if (try && !ret) {
>> --
>> 2.45.2
