Re: [PATCH] io_uring/rw: cleanup io_rw_done()

Keith Busch
2024-01-10T18:32:32Z

On Wed, Jan 10, 2024 at 10:09:19AM -0700, Jens Axboe wrote:
> +static inline void io_rw_done(struct kiocb *kiocb, ssize_t ret)
> +{
> + if (ret == -EIOCBQUEUED) {
> + return;
> + } else if (ret >= 0) {
> +end_io:
> + INDIRECT_CALL_2(kiocb->ki_complete, io_complete_rw_iopoll,
> + io_complete_rw, kiocb, ret);
> + } else {
> + switch (ret) {
> + case -ERESTARTSYS:
> + case -ERESTARTNOINTR:
> + case -ERESTARTNOHAND:
> + case -ERESTART_RESTARTBLOCK:
> + /*
> + * We can't just restart the syscall, since previously
> + * submitted sqes may already be in progress. Just fail
> + * this IO with EINTR.
> + */
> + ret = -EINTR;
> + WARN_ON_ONCE(1);
> + break;
> + }
> + goto end_io;
> + }
> +}

Are you just trying to get the most common two conditions at the top? A
little rearringing and you can remove the 'goto'. Maybe just my opinion,
but I find using goto for flow control harder to read if there's a
structured alternative.

static inline void io_rw_done(struct kiocb *kiocb, ssize_t ret)
{
if (ret == -EIOCBQUEUED)
return;

if (unlikely(ret < 0)) {
switch (ret) {
case -ERESTARTSYS:
case -ERESTARTNOINTR:
case -ERESTARTNOHAND:
case -ERESTART_RESTARTBLOCK:
/*
* We can't just restart the syscall, since previously
* submitted sqes may already be in progress. Just fail
* this IO with EINTR.
*/
ret = -EINTR;
WARN_ON_ONCE(1);
break;
}
}

INDIRECT_CALL_2(kiocb->ki_complete, io_complete_rw_iopoll,
io_complete_rw, kiocb, ret);
}