Re: [PATCH for-5.0] aio-posix: signal-proof fdmon-io_uring

[Stefan Hajnoczi]

On Wed, Apr 08, 2020 at 12:06:03PM +0200, Stefano Garzarella wrote:
> On Wed, Apr 08, 2020 at 10:11:39AM +0100, Stefan Hajnoczi wrote:
> > The io_uring_enter(2) syscall returns with errno=EINTR when interrupted
> > by a signal.  Retry the syscall in this case.
> > 
> > It's essential to do this in the io_uring_submit_and_wait() case.  My
> > interpretation of the Linux v5.5 io_uring_enter(2) code is that it
> > shouldn't affect the io_uring_submit() case, but there is no guarantee
> > this will always be the case.  Let's check for -EINTR around both APIs.
> > 
> > Note that the liburing APIs have -errno return values.
> > 
> > Signed-off-by: Stefan Hajnoczi <address@hidden>
> > ---
> >  util/fdmon-io_uring.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> The patch LGTM:
> 
> Reviewed-by: Stefano Garzarella <address@hidden>
> 
> Not related to this patch, looking at block/io_uring.c, should we retry
> if the io_uring_submit() fails with EINTR?
> 
> I mean something like this:
> 
> diff --git a/block/io_uring.c b/block/io_uring.c
> index a3142ca989..9765681f7c 100644
> --- a/block/io_uring.c
> +++ b/block/io_uring.c
> @@ -231,7 +231,7 @@ static int ioq_submit(LuringState *s)
>          trace_luring_io_uring_submit(s, ret);
>          /* Prevent infinite loop if submission is refused */
>          if (ret <= 0) {
> -            if (ret == -EAGAIN) {
> +            if (ret == -EAGAIN || ret == -EINTR) {
>                  continue;
>              }
>              break;

Thanks!

I didn't do that for the reason described in the commit message.
Philippe also mentioned that EINTR isn't listed on the io_uring_enter(2)
man page, although that is a bug because it does occur with
IORING_ENTER_GETEVENTS and min_complete > 0.

Feel free to send a separate patch.  It's probably not something that
needs to go into QEMU 5.0.

Stefan

signature.asc
Description: PGP signature