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

Jeff Moyer
2024-03-06T15:59:30Z

"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.

Cheers,
Jeff

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