[Devel] [PATCH] fs/fuse: move FUSE_S_FAIL_IMMEDIATELY check before kio req send

[Kirill Tkhai]

On 24.01.2019 12:17, Pavel Butsykin wrote:
> Yes, I missed this synchronization idea, the check and list_add should
> be together, will fix.

Also, __fuse_request_send() may need to be fixed, since it does not look
as having appropriate in already existing code (I haven't checked deeply).
 
> On 24.01.2019 11:45, Kirill Tkhai wrote:
>> On 23.01.2019 20:22, Pavel Butsykin wrote:
>>>
>>> 23.01.2019 16:55, Kirill Tkhai пишет:
>>>> On 23.01.2019 14:49, Pavel Butsykin wrote:
>>>>> Fuse file with FUSE_S_FAIL_IMMEDIATELY state should not allow to execute new
>>>>> requests. But in case of kio requests it doesn't work because the status check
>>>>> is located behind kio.op->req_send(). To fix this let's move the status check
>>>>> before kio.op->req_send().
>>>>>
>>>>> Note: We can drop hunk with req->end(fc, req) in __fuse_request_send() because
>>>>> it was only needed to clenup kio setattr request after pcs_kio_setattr_handle().
>>>>>
>>>>> Signed-off-by: Pavel Butsykin <pbutsykin at virtuozzo.com>
>>>> Why is this safe?
>>>>
>>>> After you move the check out of fc->lock, it becomes racy, and fuse_invalidate_files()
>>>> may become work not as expected.
>>>
>>> test_bit is atomic operation. Which type of race do you mean?
>>
>> fuse_request_send_background()                                 fuse_invalidate_files()
>>    test_bit(FUSE_S_FAIL_IMMEDIATELY, &req->ff->ff_state))
>>
>>                                                                   spin_lock(&fc->lock);
>>                                                                   list_for_each_entry(ff, &fi->rw_files, rw_entry)
>>                                                                     set_bit(FUSE_S_FAIL_IMMEDIATELY, &ff->ff_state);
>>                                                                   spin_unlock(&fc->lock);
>>
>>                                                                   spin_lock(&fc->lock);
>>                                                                   fuse_kill_requests(fc, inode, &fc->bg_queue);
>>                                                                   spin_unlock(&fc->lock);
>>
>>
>>    spin_lock(&fc->lock);
>>    if (fc->connected) {
>>      fuse_request_send_background_locked(fc, req); <-- queuing request after fuse_invalidate_files() thinks that
>>                                                        requests for all immediate files already killed.
>>
>>