在 2019年8月16日,01:07,Jens Axboe <ax...@kernel.dk> 写道:

> 
> On 8/14/19 3:35 AM, Jackie Liu wrote:
>> Suppose there are three IOs here, and their order is as follows:
>> 
>> Submit:
>>      [1] IO_LINK
>>          |
>>          |---  [2] IO_LINK | IO_DRAIN
>>                    |
>>                    |- [3] NORMAL_IO
>> 
>> In theory, they all need to be inserted into the Link-list, but flag
>> IO_DRAIN we have, io[2] and io[3] will be inserted into the defer_list,
>> and finally, io[3] and io[2] will be processed at the same time.
>> 
>> Now, it is directly forbidden to pass these two flags at the same time.
>> 
>> Fixes: 9e645e1105c ("io_uring: add support for sqe links")
>> Signed-off-by: Jackie Liu <liuyu...@kylinos.cn>
>> ---
>>  fs/io_uring.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>> 
>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>> index d542f1c..05ee628 100644
>> --- a/fs/io_uring.c
>> +++ b/fs/io_uring.c
>> @@ -2074,10 +2074,13 @@ static void io_submit_sqe(struct io_ring_ctx *ctx, 
>> struct sqe_submit *s,
>>  {
>>      struct io_uring_sqe *sqe_copy;
>>      struct io_kiocb *req;
>> +    unsigned int flags;
>>      int ret;
>> 
>> +    flags = READ_ONCE(s->sqe->flags);
>>      /* enforce forwards compatibility on users */
>> -    if (unlikely(s->sqe->flags & ~SQE_VALID_FLAGS)) {
>> +    if (unlikely((flags & ~SQE_VALID_FLAGS) ||
>> +                 (flags & (IOSQE_IO_DRAIN | IOSQE_IO_LINK)))) {
> 
> This doesn't look right, as any setting of either DRAIN or LINK would now
> fail?
> 
> Did you mean something ala:
> 
>       if ((flags & (IOSQE_IO_DRAIN | IOSQE_IO_LINK)) ==
>           (IOSQE_IO_DRAIN | IOSQE_IO_LINK)) {
>               ... fail ...
>       }
> 
> which makes me worried that you didn't test this at all...
> 
> -- 
> Jens Axboe

Oh, yes, it's my fault, I just simulated it in my head, thank you for pointing 
out.
I think I'd add an [RFC PATCH] next time. 

For this issue, I have two solutions, first is this, just avoid passing DRAIN 
and LINK at
the same time; second is allow, let the SQE following LINK inherit the DRAIN 
flag, but
It's more complicated, I prefer the first one.

I will rewrite this patch later, with a real test. Thanks again.

--
Jackie Liu



Reply via email to