Re: [PATCH 3/3] common/rc: force enable io_uring in _require_io_uring

Zorro Lang
2024-03-06T19:56:47Z

On Wed, Mar 06, 2024 at 10:59:25AM -0500, Jeff Moyer wrote:
> "Darrick J. Wong" <djwong@kernel.org> writes:
>
> > On Wed, Mar 06, 2024 at 05:19:35PM +0800, Zorro Lang wrote:
> >> If kernel supports io_uring, userspace still can/might disable that
> >> supporting by set /proc/sys/kernel/io_uring_disabled=2. Let's set
> >> it to 0, to always enable io_uring (ignore error if there's not
> >> that file).
> >>
> >> Signed-off-by: Zorro Lang <zlang@kernel.org>
> >> ---
> >> common/rc | 3 +++
> >> 1 file changed, 3 insertions(+)
> >>
> >> diff --git a/common/rc b/common/rc
> >> index 50dde313..966c92e3 100644
> >> --- a/common/rc
> >> +++ b/common/rc
> >> @@ -2317,6 +2317,9 @@ _require_aiodio()
> >> # this test requires that the kernel supports IO_URING
> >> _require_io_uring()
> >> {
> >> + # Force enable io_uring if kernel supports it
> >> + sysctl -w kernel.io_uring_disabled=0 &> /dev/null
> >
> > _require_XXX functions are supposed to be predicates that _notrun the
> > test if XXX isn't configured or available. Shouldn't this be:
> >
> > local io_uring_knob="$(sysctl --values kernel.io_uring_disabled)"
> > test "$io_uring_knob" -ne 0 && _notrun "io_uring disabled by admin"
>
> That sounds like a good option to me.
>
> > Alternately -- if it _is_ ok to turn this knob, then there should be a
> > cleanup method to put it back after the test.
>
> I think it would be better not to change the setting, especially if the
> admin had disabled it.

As a testing environment/system, even if fstests doesn't touch the io_uring_disabled
sysctl, the testers should care about that before doing his test. So the fstests
users might need to change the io_uring_disabled=0 manually, or do it in their CI
test scripts. Maybe we should leave this job to the users.

Thanks,
Zorro

>
> Cheers,
> Jeff
>
> >
> > --D
> >
> >> +
> >> $here/src/feature -R
> >> case $? in
> >> 0)
> >> --
> >> 2.43.0
> >>
> >>
>
>