Whamcloud - gitweb
LU-14713 llite: mend the trunc_sem_up_write()
authorBobi Jam <bobijam@whamcloud.com>
Tue, 3 Aug 2021 06:38:46 +0000 (14:38 +0800)
committerAndreas Dilger <adilger@whamcloud.com>
Fri, 14 Jan 2022 06:07:12 +0000 (06:07 +0000)
commitc172c2e3bcb5c4c47de01149069aa46e8aeb30e0
tree41156cc372bedc95b66112d71114f47735e56c38
parente26bd88b3075e1bd048930aebaadea173c5d7e69
LU-14713 llite: mend the trunc_sem_up_write()

The original lli_trunc_sem replace change (commit e5914a61ac) fixed a
lock scenario:

  t1 (page fault)          t2 (dio read)              t3 (truncate)
|- vm_mmap_pgoff()       |- vvp_io_read_start()     |- vvp_io_setattr
 |- down_write(mmap_sem)  |- down_read(trunc_sem)            _start()
  |- do_map()              |- ll_direct_IO_impl()
   |- vvp_io_fault_start    |- ll_get_user_pages()

                                                     |- down_write(
                             |- down_read(mmap_sem)        trunc_sem)
    |- down_read(trunc_sem)

t1 waits for read semaphore of trunc_sem which is hindered by t3,
since t3 is waiting for the write semaphore while t2 take its read
semaphore, and t2 is waiting for mmap_sem which has been taken by t1,
and a deadlock ensues.

commit e5914a61ac changes the down_read(trunc_sem) to
trunc_sem_down_read_nowait() in page fault path, to make it ignore
that there is a down_write(trunc_sem) waiting, just takes the read
semaphore if no writer has taken the semaphore, and breaks the
deadlock.

But there is a delicacy in using wake_up_var(), wake_up_var()->
__wake_up_bit()->waitqueue_active() locklessly test for waiters on the
queue, and if it's called without explicit smp_mb() it's possible for
the waitqueue_active() to ge hoisted before the condition store such
that we'll observe an empty wait list and the waiter might not
observe the condition, and the waiter won't get woke up whereafter.

Lustre-change: https://review.whamcloud.com/43844
Lustre-commit: 39745c8b5493159bbca62add54ca9be7cac6564f

Fixes: e5914a61ac ("LU-12460 llite: replace lli_trunc_sem")
Change-Id: Ifdda2c1c8a4171466be1723923c136e84de8ce0e
Signed-off-by: Bobi Jam <bobijam@whamcloud.com>
Reviewed-by: Patrick Farrell <pfarrell@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-on: https://review.whamcloud.com/46014
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Bobi Jam <bobijam@hotmail.com>
lustre/llite/llite_internal.h