| Message ID | 20230808124445.980419-1-liwang@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | io_uring: enable I/O Uring before testing | expand |
Li Wang <liwang@redhat.com> writes: > Given that the upstream kernel is going to introduce io_uring_disabled > knob which disables the creation of new io_uring instances system-wide. > > The new sysctl is designed to let a user with root on the machine > enable and disable io_uring system-wide at runtime without requiring > a kernel recompilation or a reboot. > > See: https://patchwork.kernel.org/project/io-uring/patch/20230630151003.3622786-2-matteorizzo@google.com/ > > Without this patch, LTP/io_uring test complains: > > io_uring01.c:82: TFAIL: io_uring_setup() failed: EPERM (1) > io_uring02.c:213: TBROK: io_uring_setup() failed: EPERM (1) Just to be clear, with the above kernel patch applied io_uring is enabled by default. You shouldn't need to set the sysctl parameter unless io_uring is explicitly disabled by the administrator (that can be accomplished via the kernel command line, sysfs, or via sysctl.conf). Cheers, Jeff > > Reproted-by: Jeff Moyer <jmoyer@redhat.com> > Signed-off-by: Li Wang <liwang@redhat.com> > --- > testcases/kernel/syscalls/io_uring/io_uring01.c | 5 +++++ > testcases/kernel/syscalls/io_uring/io_uring02.c | 5 +++++ > 2 files changed, 10 insertions(+) > > diff --git a/testcases/kernel/syscalls/io_uring/io_uring01.c b/testcases/kernel/syscalls/io_uring/io_uring01.c > index 70151bb85..ab1ec00d6 100644 > --- a/testcases/kernel/syscalls/io_uring/io_uring01.c > +++ b/testcases/kernel/syscalls/io_uring/io_uring01.c > @@ -264,5 +264,10 @@ static struct tst_test test = { > .bufs = (struct tst_buffers []) { > {&iov, .iov_sizes = (int[]){BLOCK_SZ, -1}}, > {} > + }, > + .save_restore = (const struct tst_path_val[]) { > + {"/proc/sys/kernel/io_uring_disabled", "0", > + TST_SR_SKIP_MISSING | TST_SR_TCONF_RO}, > + {} > } > }; > diff --git a/testcases/kernel/syscalls/io_uring/io_uring02.c b/testcases/kernel/syscalls/io_uring/io_uring02.c > index c5c770074..c9d4bbcb1 100644 > --- a/testcases/kernel/syscalls/io_uring/io_uring02.c > +++ b/testcases/kernel/syscalls/io_uring/io_uring02.c > @@ -255,6 +255,11 @@ static struct tst_test test = { > TST_CAP(TST_CAP_REQ, CAP_SYS_CHROOT), > {} > }, > + .save_restore = (const struct tst_path_val[]) { > + {"/proc/sys/kernel/io_uring_disabled", "0", > + TST_SR_SKIP_MISSING | TST_SR_TCONF_RO}, > + {} > + }, > .tags = (const struct tst_tag[]) { > {"linux-git", "9392a27d88b9"}, > {"linux-git", "ff002b30181d"},
Hi Jeff, Thanks for comments. On Tue, Aug 8, 2023 at 9:17 PM Jeff Moyer <jmoyer@redhat.com> wrote: > Li Wang <liwang@redhat.com> writes: > > > Given that the upstream kernel is going to introduce io_uring_disabled > > knob which disables the creation of new io_uring instances system-wide. > > > > The new sysctl is designed to let a user with root on the machine > > enable and disable io_uring system-wide at runtime without requiring > > a kernel recompilation or a reboot. > > > > See: > https://patchwork.kernel.org/project/io-uring/patch/20230630151003.3622786-2-matteorizzo@google.com/ > > > > Without this patch, LTP/io_uring test complains: > > > > io_uring01.c:82: TFAIL: io_uring_setup() failed: EPERM (1) > > io_uring02.c:213: TBROK: io_uring_setup() failed: EPERM (1) > > Just to be clear, with the above kernel patch applied io_uring is > enabled by default. You shouldn't need to set the sysctl parameter > unless io_uring is explicitly disabled by the administrator (that can be > accomplished via the kernel command line, sysfs, or via sysctl.conf). > Yes, but it won't be harmful to set the parameter even if it's enabled by default, LTP uses save_restore field to manage sysfs knob unified, it guarantees the tests can really get performed whatever io_uring is enabled or disabled. I would keep the patch as it is unless you insist or others have an objection. > > Cheers, > Jeff > > > > > Reproted-by: Jeff Moyer <jmoyer@redhat.com> > > Signed-off-by: Li Wang <liwang@redhat.com> > > --- > > testcases/kernel/syscalls/io_uring/io_uring01.c | 5 +++++ > > testcases/kernel/syscalls/io_uring/io_uring02.c | 5 +++++ > > 2 files changed, 10 insertions(+) > > > > diff --git a/testcases/kernel/syscalls/io_uring/io_uring01.c > b/testcases/kernel/syscalls/io_uring/io_uring01.c > > index 70151bb85..ab1ec00d6 100644 > > --- a/testcases/kernel/syscalls/io_uring/io_uring01.c > > +++ b/testcases/kernel/syscalls/io_uring/io_uring01.c > > @@ -264,5 +264,10 @@ static struct tst_test test = { > > .bufs = (struct tst_buffers []) { > > {&iov, .iov_sizes = (int[]){BLOCK_SZ, -1}}, > > {} > > + }, > > + .save_restore = (const struct tst_path_val[]) { > > + {"/proc/sys/kernel/io_uring_disabled", "0", > > + TST_SR_SKIP_MISSING | TST_SR_TCONF_RO}, > > + {} > > } > > }; > > diff --git a/testcases/kernel/syscalls/io_uring/io_uring02.c > b/testcases/kernel/syscalls/io_uring/io_uring02.c > > index c5c770074..c9d4bbcb1 100644 > > --- a/testcases/kernel/syscalls/io_uring/io_uring02.c > > +++ b/testcases/kernel/syscalls/io_uring/io_uring02.c > > @@ -255,6 +255,11 @@ static struct tst_test test = { > > TST_CAP(TST_CAP_REQ, CAP_SYS_CHROOT), > > {} > > }, > > + .save_restore = (const struct tst_path_val[]) { > > + {"/proc/sys/kernel/io_uring_disabled", "0", > > + TST_SR_SKIP_MISSING | TST_SR_TCONF_RO}, > > + {} > > + }, > > .tags = (const struct tst_tag[]) { > > {"linux-git", "9392a27d88b9"}, > > {"linux-git", "ff002b30181d"}, > >
Li Wang <liwang@redhat.com> writes: > Hi Jeff, > > Thanks for comments. > > On Tue, Aug 8, 2023 at 9:17 PM Jeff Moyer <jmoyer@redhat.com> wrote: > >> Li Wang <liwang@redhat.com> writes: >> >> > Given that the upstream kernel is going to introduce io_uring_disabled >> > knob which disables the creation of new io_uring instances system-wide. >> > >> > The new sysctl is designed to let a user with root on the machine >> > enable and disable io_uring system-wide at runtime without requiring >> > a kernel recompilation or a reboot. >> > >> > See: >> https://patchwork.kernel.org/project/io-uring/patch/20230630151003.3622786-2-matteorizzo@google.com/ >> > >> > Without this patch, LTP/io_uring test complains: >> > >> > io_uring01.c:82: TFAIL: io_uring_setup() failed: EPERM (1) >> > io_uring02.c:213: TBROK: io_uring_setup() failed: EPERM (1) >> >> Just to be clear, with the above kernel patch applied io_uring is >> enabled by default. You shouldn't need to set the sysctl parameter >> unless io_uring is explicitly disabled by the administrator (that can be >> accomplished via the kernel command line, sysfs, or via sysctl.conf). >> > > Yes, but it won't be harmful to set the parameter even if it's enabled by > default, > LTP uses save_restore field to manage sysfs knob unified, it guarantees the > tests can really get performed whatever io_uring is enabled or disabled. > > I would keep the patch as it is unless you insist or others have an > objection. I agree with the patch. I just think the description should be updated, as it implies that, without the patch, the test will fail. This is not the case for an upstream kernel. Thanks! Jeff > > >> >> Cheers, >> Jeff >> >> > >> > Reproted-by: Jeff Moyer <jmoyer@redhat.com> >> > Signed-off-by: Li Wang <liwang@redhat.com> >> > --- >> > testcases/kernel/syscalls/io_uring/io_uring01.c | 5 +++++ >> > testcases/kernel/syscalls/io_uring/io_uring02.c | 5 +++++ >> > 2 files changed, 10 insertions(+) >> > >> > diff --git a/testcases/kernel/syscalls/io_uring/io_uring01.c >> b/testcases/kernel/syscalls/io_uring/io_uring01.c >> > index 70151bb85..ab1ec00d6 100644 >> > --- a/testcases/kernel/syscalls/io_uring/io_uring01.c >> > +++ b/testcases/kernel/syscalls/io_uring/io_uring01.c >> > @@ -264,5 +264,10 @@ static struct tst_test test = { >> > .bufs = (struct tst_buffers []) { >> > {&iov, .iov_sizes = (int[]){BLOCK_SZ, -1}}, >> > {} >> > + }, >> > + .save_restore = (const struct tst_path_val[]) { >> > + {"/proc/sys/kernel/io_uring_disabled", "0", >> > + TST_SR_SKIP_MISSING | TST_SR_TCONF_RO}, >> > + {} >> > } >> > }; >> > diff --git a/testcases/kernel/syscalls/io_uring/io_uring02.c >> b/testcases/kernel/syscalls/io_uring/io_uring02.c >> > index c5c770074..c9d4bbcb1 100644 >> > --- a/testcases/kernel/syscalls/io_uring/io_uring02.c >> > +++ b/testcases/kernel/syscalls/io_uring/io_uring02.c >> > @@ -255,6 +255,11 @@ static struct tst_test test = { >> > TST_CAP(TST_CAP_REQ, CAP_SYS_CHROOT), >> > {} >> > }, >> > + .save_restore = (const struct tst_path_val[]) { >> > + {"/proc/sys/kernel/io_uring_disabled", "0", >> > + TST_SR_SKIP_MISSING | TST_SR_TCONF_RO}, >> > + {} >> > + }, >> > .tags = (const struct tst_tag[]) { >> > {"linux-git", "9392a27d88b9"}, >> > {"linux-git", "ff002b30181d"}, >> >>
On Wed, Aug 9, 2023 at 9:46 PM Jeff Moyer <jmoyer@redhat.com> wrote: > Li Wang <liwang@redhat.com> writes: > > > Hi Jeff, > > > > Thanks for comments. > > > > On Tue, Aug 8, 2023 at 9:17 PM Jeff Moyer <jmoyer@redhat.com> wrote: > > > >> Li Wang <liwang@redhat.com> writes: > >> > >> > Given that the upstream kernel is going to introduce io_uring_disabled > >> > knob which disables the creation of new io_uring instances > system-wide. > >> > > >> > The new sysctl is designed to let a user with root on the machine > >> > enable and disable io_uring system-wide at runtime without requiring > >> > a kernel recompilation or a reboot. > >> > > >> > See: > >> > https://patchwork.kernel.org/project/io-uring/patch/20230630151003.3622786-2-matteorizzo@google.com/ > >> > > >> > Without this patch, LTP/io_uring test complains: > >> > > >> > io_uring01.c:82: TFAIL: io_uring_setup() failed: EPERM (1) > >> > io_uring02.c:213: TBROK: io_uring_setup() failed: EPERM (1) > >> > >> Just to be clear, with the above kernel patch applied io_uring is > >> enabled by default. You shouldn't need to set the sysctl parameter > >> unless io_uring is explicitly disabled by the administrator (that can be > >> accomplished via the kernel command line, sysfs, or via sysctl.conf). > >> > > > > Yes, but it won't be harmful to set the parameter even if it's enabled by > > default, > > LTP uses save_restore field to manage sysfs knob unified, it guarantees > the > > tests can really get performed whatever io_uring is enabled or disabled. > > > > I would keep the patch as it is unless you insist or others have an > > objection. > > I agree with the patch. I just think the description should be updated, > as it implies that, without the patch, the test will fail. This is not > the case for an upstream kernel. > Ah, sorry for the unclear description, I actually was not mean that. anyway, I rewrote it and pushed. Thanks for your reveiw, Jeff.
diff --git a/testcases/kernel/syscalls/io_uring/io_uring01.c b/testcases/kernel/syscalls/io_uring/io_uring01.c index 70151bb85..ab1ec00d6 100644 --- a/testcases/kernel/syscalls/io_uring/io_uring01.c +++ b/testcases/kernel/syscalls/io_uring/io_uring01.c @@ -264,5 +264,10 @@ static struct tst_test test = { .bufs = (struct tst_buffers []) { {&iov, .iov_sizes = (int[]){BLOCK_SZ, -1}}, {} + }, + .save_restore = (const struct tst_path_val[]) { + {"/proc/sys/kernel/io_uring_disabled", "0", + TST_SR_SKIP_MISSING | TST_SR_TCONF_RO}, + {} } }; diff --git a/testcases/kernel/syscalls/io_uring/io_uring02.c b/testcases/kernel/syscalls/io_uring/io_uring02.c index c5c770074..c9d4bbcb1 100644 --- a/testcases/kernel/syscalls/io_uring/io_uring02.c +++ b/testcases/kernel/syscalls/io_uring/io_uring02.c @@ -255,6 +255,11 @@ static struct tst_test test = { TST_CAP(TST_CAP_REQ, CAP_SYS_CHROOT), {} }, + .save_restore = (const struct tst_path_val[]) { + {"/proc/sys/kernel/io_uring_disabled", "0", + TST_SR_SKIP_MISSING | TST_SR_TCONF_RO}, + {} + }, .tags = (const struct tst_tag[]) { {"linux-git", "9392a27d88b9"}, {"linux-git", "ff002b30181d"},
Given that the upstream kernel is going to introduce io_uring_disabled knob which disables the creation of new io_uring instances system-wide. The new sysctl is designed to let a user with root on the machine enable and disable io_uring system-wide at runtime without requiring a kernel recompilation or a reboot. See: https://patchwork.kernel.org/project/io-uring/patch/20230630151003.3622786-2-matteorizzo@google.com/ Without this patch, LTP/io_uring test complains: io_uring01.c:82: TFAIL: io_uring_setup() failed: EPERM (1) io_uring02.c:213: TBROK: io_uring_setup() failed: EPERM (1) Reproted-by: Jeff Moyer <jmoyer@redhat.com> Signed-off-by: Li Wang <liwang@redhat.com> --- testcases/kernel/syscalls/io_uring/io_uring01.c | 5 +++++ testcases/kernel/syscalls/io_uring/io_uring02.c | 5 +++++ 2 files changed, 10 insertions(+)