Re: [PATCH RFC v7 00/16] fuse: fuse-over-io-uring

[Pavel Begunkov <asml.silence@gmail.com>]

On 12/6/24 11:36, Bernd Schubert wrote:
> On 12/3/24 15:32, Bernd Schubert wrote:
>> On 12/3/24 15:24, Pavel Begunkov wrote:
>>> On 11/27/24 13:40, Bernd Schubert wrote:
>>>> [I removed RFC status as the design should be in place now
>>>> and as xfstests pass. I still reviewing patches myself, though
>>>> and also repeatings tests with different queue sizes.]
>>>
>>> I left a few comments, but it looks sane. At least on the io_uring
>>> side nothing weird caught my eye. Cancellations might be a bit
>>> worrisome as usual, so would be nice to give it a good run with
>>> sanitizers.
>>
>> Thanks a lot for your reviews, new series is in preparation, will
>> send it out tomorrow to give a test run over night. I'm
>> running xfstests on a kernel that has lockdep and ASAN enabled, which
>> is why it takes around 15 hours (with/without FOPEN_DIRECT_IO).
> 
> I found a few issues myself and somehow xfstests take more
> than twice as long right with 6.13 *and a slightly different kernel
> config. Still waiting for test completion.
> 
> 
> I have a question actually regarding patch 15 that handles
> IO_URING_F_CANCEL. I think there there is a race in v7 and before,
> as the fuse entry state FRRS_WAIT might not have been reached _yet_
> and then io_uring_cmd_done() would not be called.
> Can I do it like this in fuse_uring_cancel()

A IO_URING_F_CANCEL doesn't cancel a request nor removes it
from io_uring's cancellation list, io_uring_cmd_done() does.
You might also be getting multiple IO_URING_F_CANCEL calls for
a request until the request is released.


> if (need_cmd_done) {
> 	io_uring_cmd_done(cmd, -ENOTCONN, 0, issue_flags);
> } else {
> 	/*
> 	 * We don't check for the actual state, but let io-uring
> 	 * layer handle if re-sending the IO_URING_F_CANCEL SQE is still
> 	 * needed.
> 	 */
> 	ret = -EAGAIN;
> }
> 
> I.e. lets assume umount races with IO_URING_F_CANCEL (for example umount
> triggers a daemon crash). The umount process/task could now already do
> or start to do io_uring_cmd_done and just around the same time and
> IO_URING_F_CANCEL comes in.
> My question is if io-uring knows that re-sending the
> IO_URING_F_CANCEL is still needed or will it avoid re-sending if
> io_uring_cmd_done() was already done?
> I could also add state checking in the fuse_uring_cancel function.
-- 
Pavel Begunkov