Re: [PATCH] io_uring: Add IO_U_F_DEVICE_ERROR to identify error types

[Minwoo Im <minwoo.im.dev@gmail.com>]

On 24-09-26 18:54:35, Jens Axboe wrote:
> On 9/26/24 3:33 PM, Minwoo Im wrote:
> > On 24-09-26 15:06:49, Jens Axboe wrote:
> >> On 9/26/24 12:08 PM, Vincent Fu wrote:
> >>> On 9/26/24 13:48, Jens Axboe wrote:
> >>>> On 9/25/24 6:41 PM, Minwoo Im wrote:
> >>>>> diff --git a/io_u.h b/io_u.h
> >>>>> index ab93d50f967e..20afad667ee1 100644
> >>>>> --- a/io_u.h
> >>>>> +++ b/io_u.h
> >>>>> @@ -22,6 +22,7 @@ enum {
> >>>>>       IO_U_F_BARRIER        = 1 << 6,
> >>>>>       IO_U_F_VER_LIST        = 1 << 7,
> >>>>>       IO_U_F_PATTERN_DONE    = 1 << 8,
> >>>>> +    IO_U_F_DEVICE_ERROR    = 1 << 9,
> >>>>>   };
> >>>>>     /*
> >>>>
> >>>> The patches you sent should've been a series, how are they supposed to
> >>>> both apply when you add an item here for each of them as if the other
> >>>> one doesn't exist?
> >>>>
> >>>> I'll fix it up, but for the future, if patches depend on each other, it
> >>>> should be a series. Please check if everything works when it's pushed
> >>>> out, which should be shortly.
> >>>>
> >>>> And since I'm on a plane and this doesn't appear to want to send, when
> >>>> you do see it, please also add HOWTO additions similar to the fio.1
> >>>> additions you made.
> >>>>
> >>>
> >>> Also, Minwoo, we are seeing some build failures with your patches with
> >>> compilers rejecting abs(io_u->error) since error is unsigned. Please
> >>> fix that up as well.
> >>
> >> Doh yes, I fixed that up now.
> > 
> > Jens,
> > 
> > How about the following one for the fix?  (checked with clang)
> > 
> > diff --git a/engines/io_uring.c b/engines/io_uring.c
> > index 6c07c1011c40..85cebf8371cf 100644
> > --- a/engines/io_uring.c
> > +++ b/engines/io_uring.c
> > @@ -589,7 +589,7 @@ ret:
> >                 io_u_set(td, io_u, IO_U_F_DEVICE_ERROR);
> >         else
> >                 io_u_clear(td, io_u, IO_U_F_DEVICE_ERROR);
> > -       io_u->error = io_u->error;
> > +       io_u->error = abs((int)io_u->error);
> >         return io_u;
> >  }
> > 
> > io_u->error is unsigned, but it can have negative value as an errno.
> > io_u->error should be converted to a positive one especially when it's
> > an errno.  If we don't, it will be like Unknown error:
> > 
> > fio: io_u error on file /dev/ng0n1: Unknown error -22: write offset=429916160, buflen=1048576
> 
> Yeah I think that looks good. Can you send this as a patch against the
> current tree? I did do a fixup, but way too jet lagged and it's
> obviously garbage.

Sure, I will, Thanks!