Message ID | 7a7b7128-ba62-59e1-552b-3b1fd6b1eb50@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | ltp/fsstress: don't fail on io_uring ENOSYS | expand |
On Thu, Jan 28, 2021 at 03:31:40PM -0600, Eric Sandeen wrote: > We might have URING #defined at build time, but be running on a kernel > which does not support it. > > For that reason, we should not exit with an error if > io_uring_queue_init() fails with ENOSYS. We can just note the lack of > support and skip all future io_uring operations. > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > --- > > diff --git a/ltp/fsstress.c b/ltp/fsstress.c > index 22df5e38..73751935 100644 > --- a/ltp/fsstress.c > +++ b/ltp/fsstress.c > @@ -35,6 +35,7 @@ io_context_t io_ctx; > #include <liburing.h> > #define URING_ENTRIES 1 > struct io_uring ring; > +bool have_io_uring; /* to indicate runtime availability */ > #endif > #include <sys/syscall.h> > #include <sys/xattr.h> > @@ -706,9 +707,15 @@ int main(int argc, char **argv) > } > #endif > #ifdef URING > + have_io_uring = true; > + /* If ENOSYS, just ignore uring, other errors are fatal. */ Yes, I thought about if we should do this since rhel8 kernel removed io_uring support from kernel, but left userspace liburing. But if we do this for io_uring, should we do the same check the others which can be disabled from kernel? Likes: AIO? Thanks, Zorro > if (io_uring_queue_init(URING_ENTRIES, &ring, 0)) { > - fprintf(stderr, "io_uring_queue_init failed\n"); > - exit(1); > + if (errno == ENOSYS) { > + have_io_uring = false; > + } else { > + fprintf(stderr, "io_uring_queue_init failed\n"); > + exit(1); > + } > } > #endif > for (i = 0; !loops || (i < loops); i++) > @@ -720,7 +727,8 @@ int main(int argc, char **argv) > } > #endif > #ifdef URING > - io_uring_queue_exit(&ring); > + if (have_io_uring) > + io_uring_queue_exit(&ring); > #endif > cleanup_flist(); > free(freq_table); > @@ -2208,6 +2216,9 @@ do_uring_rw(int opno, long r, int flags) > struct iovec iovec; > int iswrite = (flags & (O_WRONLY | O_RDWR)) ? 1 : 0; > > + if (!have_io_uring) > + return; > + > init_pathname(&f); > if (!get_fname(FT_REGFILE, r, &f, NULL, NULL, &v)) { > if (v) >
On Fri, Jan 29, 2021 at 04:10:44PM +0800, Zorro Lang wrote: > On Thu, Jan 28, 2021 at 03:31:40PM -0600, Eric Sandeen wrote: > > We might have URING #defined at build time, but be running on a kernel > > which does not support it. > > > > For that reason, we should not exit with an error if > > io_uring_queue_init() fails with ENOSYS. We can just note the lack of > > support and skip all future io_uring operations. > > > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > > --- > > > > diff --git a/ltp/fsstress.c b/ltp/fsstress.c > > index 22df5e38..73751935 100644 > > --- a/ltp/fsstress.c > > +++ b/ltp/fsstress.c > > @@ -35,6 +35,7 @@ io_context_t io_ctx; > > #include <liburing.h> > > #define URING_ENTRIES 1 > > struct io_uring ring; > > +bool have_io_uring; /* to indicate runtime availability */ > > #endif > > #include <sys/syscall.h> > > #include <sys/xattr.h> > > @@ -706,9 +707,15 @@ int main(int argc, char **argv) > > } > > #endif > > #ifdef URING > > + have_io_uring = true; > > + /* If ENOSYS, just ignore uring, other errors are fatal. */ > > Yes, I thought about if we should do this since rhel8 kernel removed io_uring > support from kernel, but left userspace liburing. But if we do this for io_uring, > should we do the same check the others which can be disabled from kernel? Likes: AIO? io_uring is a relative new interface, and it's quite possible that some distros don't support it. aio has been there for a long time, and is very unlikely disabled. If we really need to do the same check for aio, we could do it in another patch I guess. Thanks, Eryu
On Sun, Jan 31, 2021 at 10:30:07PM +0800, Eryu Guan wrote: > On Fri, Jan 29, 2021 at 04:10:44PM +0800, Zorro Lang wrote: > > On Thu, Jan 28, 2021 at 03:31:40PM -0600, Eric Sandeen wrote: > > > We might have URING #defined at build time, but be running on a kernel > > > which does not support it. > > > > > > For that reason, we should not exit with an error if > > > io_uring_queue_init() fails with ENOSYS. We can just note the lack of > > > support and skip all future io_uring operations. > > > > > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > > > --- > > > > > > diff --git a/ltp/fsstress.c b/ltp/fsstress.c > > > index 22df5e38..73751935 100644 > > > --- a/ltp/fsstress.c > > > +++ b/ltp/fsstress.c > > > @@ -35,6 +35,7 @@ io_context_t io_ctx; > > > #include <liburing.h> > > > #define URING_ENTRIES 1 > > > struct io_uring ring; > > > +bool have_io_uring; /* to indicate runtime availability */ > > > #endif > > > #include <sys/syscall.h> > > > #include <sys/xattr.h> > > > @@ -706,9 +707,15 @@ int main(int argc, char **argv) > > > } > > > #endif > > > #ifdef URING > > > + have_io_uring = true; > > > + /* If ENOSYS, just ignore uring, other errors are fatal. */ > > > > Yes, I thought about if we should do this since rhel8 kernel removed io_uring > > support from kernel, but left userspace liburing. But if we do this for io_uring, > > should we do the same check the others which can be disabled from kernel? Likes: AIO? > > io_uring is a relative new interface, and it's quite possible that some > distros don't support it. aio has been there for a long time, and is > very unlikely disabled. If we really need to do the same check for aio, > we could do it in another patch I guess. OK, I just have this one question, this patch looks good to me. Thanks, Zorro > > Thanks, > Eryu >
diff --git a/ltp/fsstress.c b/ltp/fsstress.c index 22df5e38..73751935 100644 --- a/ltp/fsstress.c +++ b/ltp/fsstress.c @@ -35,6 +35,7 @@ io_context_t io_ctx; #include <liburing.h> #define URING_ENTRIES 1 struct io_uring ring; +bool have_io_uring; /* to indicate runtime availability */ #endif #include <sys/syscall.h> #include <sys/xattr.h> @@ -706,9 +707,15 @@ int main(int argc, char **argv) } #endif #ifdef URING + have_io_uring = true; + /* If ENOSYS, just ignore uring, other errors are fatal. */ if (io_uring_queue_init(URING_ENTRIES, &ring, 0)) { - fprintf(stderr, "io_uring_queue_init failed\n"); - exit(1); + if (errno == ENOSYS) { + have_io_uring = false; + } else { + fprintf(stderr, "io_uring_queue_init failed\n"); + exit(1); + } } #endif for (i = 0; !loops || (i < loops); i++) @@ -720,7 +727,8 @@ int main(int argc, char **argv) } #endif #ifdef URING - io_uring_queue_exit(&ring); + if (have_io_uring) + io_uring_queue_exit(&ring); #endif cleanup_flist(); free(freq_table); @@ -2208,6 +2216,9 @@ do_uring_rw(int opno, long r, int flags) struct iovec iovec; int iswrite = (flags & (O_WRONLY | O_RDWR)) ? 1 : 0; + if (!have_io_uring) + return; + init_pathname(&f); if (!get_fname(FT_REGFILE, r, &f, NULL, NULL, &v)) { if (v)
We might have URING #defined at build time, but be running on a kernel which does not support it. For that reason, we should not exit with an error if io_uring_queue_init() fails with ENOSYS. We can just note the lack of support and skip all future io_uring operations. Signed-off-by: Eric Sandeen <sandeen@redhat.com> ---