On 1/10/24 11:32 AM, Keith Busch wrote:
> 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);
> }
This does look nicer! I'll fold that in, thanks Keith.
--
Jens Axboe
