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

[Jeff Moyer <jmoyer@redhat.com>]

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