Re: [PATCH v3 RFC liburing 2/4] src/{queue,register,setup}: Don't use `__sys_io_uring*`

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Oct 2, 2021 at 3:12 PM Louvian Lyndal <louvianlyndal@xxxxxxxxx> wrote:
>
> On 10/2/21 8:48 AM, Ammar Faizi wrote:
> > @@ -158,7 +142,7 @@ int io_uring_register_files_tags(struct io_uring *ring,
> >               break;
> >       } while (1);
> >  
> > -     return ret < 0 ? -errno : ret;
> > +     return (ret < 0) ? ret : 0;
> >   }
>
> This is wrong, you changed the logic, should've been "return ret;".
> Not all successful call returns 0.
>
> >  
> >   int io_uring_register_files(struct io_uring *ring, const int *files,
> > @@ -167,12 +151,12 @@ int io_uring_register_files(struct io_uring *ring, const int *files,
> >       int ret, did_increase = 0;
> >  
> >       do {
> > -             ret = __sys_io_uring_register(ring->ring_fd,
> > -                                           IORING_REGISTER_FILES, files,
> > -                                           nr_files);
> > +             ret = ____sys_io_uring_register(ring->ring_fd,
> > +                                             IORING_REGISTER_FILES, files,
> > +                                             nr_files);
> >               if (ret >= 0)
> >                       break;
> > -             if (errno == EMFILE && !did_increase) {
> > +             if (ret == -EMFILE && !did_increase) {
> >                       did_increase = 1;
> >                       increase_rlimit_nofile(nr_files);
> >                       continue;
> > @@ -180,55 +164,44 @@ int io_uring_register_files(struct io_uring *ring, const int *files,
> >               break;
> >       } while (1);
> >  
> > -     return ret < 0 ? -errno : ret;
> > +     return (ret < 0) ? ret : 0;
> >   }
>
> The same thing here!
>
> --
> Louvian Lyndal

Ack, that changed the logic. I missed that, will fix this for v4.

Not an excuse, but the test did not fail from my end.

Looking at test/test_update_multiring.c:71 on test_update_multiring():
```
  if (io_uring_register_files(r1, fds, 10) ||
      io_uring_register_files(r2, fds, 10) ||
      io_uring_register_files(r3, fds, 10)) {
        fprintf(stderr, "%s: register files failed\n", __FUNCTION__);
        goto err;
  }
```
Based on this one, this function is expected to return 0. When it
returns non zero value, it will go to err. So it won't really change
the situation.

In anyway, it's a fact that I changed the logic, I admit that and will
get it fixed.

Thanks!

-- 
Ammar Faizi



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux