On 25.12.2021 02:08, Namjae Jeon wrote: > 2021-12-24 21:31 GMT+09:00, Vasily Averin <vvs@xxxxxxxxxxxxx>: >> 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? > No, Each commands are handled by ksmbd-io kworkers. In this case ksmbd can do not require async lock processing and you can drop my previous patches. Is there any difference where thread will be blocked: inside ->lock() function of exported filesystem or in ksmbd_vfs_posix_lock_wait? I think it isn't. Thank you, Vasily Averin
