2024年9月1日 11:10，Kent Overstreet <kent.overstr...@linux.dev> wrote：
>
> On Sun, Sep 01, 2024 at 10:50:37AM GMT, Alan Huang wrote:
>>> 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()
>
> that's in another thread, a memory barrier orders things in the same
> thread
six_set_bitmask(lock, SIX_LOCK_WAITING_read << type) is called before
__do_six_trylock()
