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 <pbutsy...@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.
>>
>>
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
