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

[Kirill Tkhai]

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.