On 22.12.2021 18:17, Vasily Averin wrote: > On 22.12.2021 11:58, Namjae Jeon wrote: >> 2021-12-22 15:51 GMT+09:00, Vasily Averin <vvs@xxxxxxxxxxxxx>: >>> On 22.12.2021 08:25, Namjae Jeon wrote: >>>> 2021-12-22 13:32 GMT+09:00, Vasily Averin <vvs@xxxxxxxxxxxxx>: >>>>> On 22.12.2021 05:50, Namjae Jeon wrote: >>>>>> 2021-12-21 22:08 GMT+09:00, Vasily Averin <vvs@xxxxxxxxxxxxx>: >>>>>>> On 21.12.2021 15:02, Namjae Jeon wrote: >>>>>>>> 2021-12-19 18:34 GMT+09:00, Vasily Averin <vvs@xxxxxxxxxxxxx>: >>>>>>>>> To avoid possible deadlock ksmbd should process locks >>>>>>>>> asynchronously. >>>>>>>>> Callers expecting vfs_file_locks() to return asynchronously should >>>>>>>>> only >>>>>>>>> use F_SETLK, not F_SETLKW. >>>>>>>> Should I check this patch instead of >>>>>>>> [PATCH] ksmbd: force "fail immediately" flag on fs with its own >>>>>>>> ->lock >>>>>>>> ? >>>>>>> >>>>>>> no, these patches are independent and both ones are required. >>>>>>> current patch fixes incorrect kernel thread behaviour: >>>>>>> kernel threads should not use F_SETLKW for locking requests. >>>>>> How does this patch work? posix_lock_file in vfs_lock_file() does not >>>>>> use >>>>>> cmd. >>>>>> And your patch still leaves FL_SLEEP. >>>>> >>>>> "use F_SETLK, not F_SETLKW" was copy-pasted from requirement described >>>>> in >>>>> comment above vfs_lock_file(). >>>>> >>>>> posix_lock_file() is not used in all ->lock() functions, and use >>>>> F_SETLKW >>>>> forces some of affected filesystem use blocking locks: >>>> What I'm saying is that when we apply "ksmbd: force "fail immediately" >>>> flag on fs with its own ->lock ", this patch is meaningless. How is >>>> ->lock() with F_SETLKW called? >>> >>> I got your point finally, >>> yes, you are right, now this cannot happen. >>> However I'm going to fix all affected filesystems and then revert >>> "ksmbd: force "fail immediately" flag on fs with its own ->lock" >>> When this happen and ksmbd will still use IS_SETLKW it will trigger the >>> problems described below. >> If so, You can include one patch(this patch + revert patch) in patch >> series for fixing ->lock of all filesystem. I've checked how smb2_lock() handles FILE_LOCK_DEFERRED returned by vfs_lock_file() call. It seems for me, working thread will be blocked in ksmbd_vfs_posix_lock_wait(), so whole ksmbd server still can deadlock. Am I wrong probably? Thank you, Vasily Averin
