Re: io_uring_prep_openat_direct() and link/drain

[Jens Axboe <axboe@kernel.dk>]

On 3/29/22 1:30 PM, Miklos Szeredi wrote:
> On Tue, 29 Mar 2022 at 20:40, Jens Axboe <axboe@kernel.dk> wrote:
>>
>> On 3/29/22 12:31 PM, Miklos Szeredi wrote:
>>> On Tue, 29 Mar 2022 at 20:26, Jens Axboe <axboe@kernel.dk> wrote:
>>>>
>>>> On 3/29/22 12:21 PM, Miklos Szeredi wrote:
>>>>> On Tue, 29 Mar 2022 at 19:04, Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>
>>>>>> On 3/29/22 10:08 AM, Jens Axboe wrote:
>>>>>>> On 3/29/22 7:20 AM, Miklos Szeredi wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> I'm trying to read multiple files with io_uring and getting stuck,
>>>>>>>> because the link and drain flags don't seem to do what they are
>>>>>>>> documented to do.
>>>>>>>>
>>>>>>>> Kernel is v5.17 and liburing is compiled from the git tree at
>>>>>>>> 7a3a27b6a384 ("add tests for nonblocking accept sockets").
>>>>>>>>
>>>>>>>> Without those flags the attached example works some of the time, but
>>>>>>>> that's probably accidental since ordering is not ensured.
>>>>>>>>
>>>>>>>> Adding the drain or link flags make it even worse (fail in casese that
>>>>>>>> the unordered one didn't).
>>>>>>>>
>>>>>>>> What am I missing?
>>>>>>>
>>>>>>> I don't think you're missing anything, it looks like a bug. What you
>>>>>>> want here is:
>>>>>>>
>>>>>>> prep_open_direct(sqe);
>>>>>>> sqe->flags |= IOSQE_IO_LINK;
>>>>>>> ...
>>>>>>> prep_read(sqe);
>>>>>
>>>>> So with the below merge this works.   But if instead I do
>>>>>
>>>>> prep_open_direct(sqe);
>>>>>  ...
>>>>> prep_read(sqe);
>>>>> sqe->flags |= IOSQE_IO_DRAIN;
> 
> And this doesn't work either:
> 
> prep_open_direct(sqe);
> sqe->flags |= IOSQE_IO_LINK;
> ...
> prep_read(sqe);
> sqe->flags |= IOSQE_IO_LINK;
> ...
> prep_open_direct(sqe);
> sqe->flags |= IOSQE_IO_LINK;
> ...
> prep_read(sqe);
> 
> Yeah, the link is not needed for the read (unless the fixed file slot
> is to be reused), but link/drain should work as general ordering
> instructions, not just in special cases.

Not disagreeing with that, it had nothing to do with any assumptions
like that, it was just a bug in the change I made in terms of when
things got punted async.

Can you try and repull the branch? I rebased the top two, so reset to
v5.17 first and then pull it.

BTW, I would not recommend using DRAIN, it's very expensive compared to
just a link. Particularly, using just a single link between the
open+read makes sense, and should be efficient. Don't link between all
of them, that's creating a more expensive dependency that doesn't make
any sense for your use case either. Should they both work? Of course,
and I think they will in the current branch. Merely stating that they
make no sense other than as an exercise-the-logic test case.

-- 
Jens Axboe