Re: [PATCH] nvmet: support reservation feature

[Jens Axboe <axboe@kernel.dk>]

On 1/10/24 1:31 AM, Chaitanya Kulkarni wrote:
>>>> @@ -904,6 +906,16 @@ static u16 nvmet_parse_io_cmd(struct nvmet_req 
>>>> *req)
>>>>            return ret;
>>>>        }
>>>>    +    ret = nvmet_pr_check_cmd_access(req);
>>>> +    if (unlikely(ret)) {
>>>> +        req->error_loc = offsetof(struct nvme_common_command, opcode);
>>>> +        return ret;
>>>> +    }
>>>> +
>>>> +    ret = nvmet_parse_pr_cmd(req);
>>>> +    if (!ret)
>>>> +        return ret;
>>>> +
>>> Can we make this feature configurable via Kconfig? If someone doesn't
>>> want to
>>> use PR, they will have to bear the cost of these checks in the fast 
>>> path.
>>
>> Yeah, I have added a resv_enable in configfs, the default is false, 
>> one can
>>
>> make reservation enable before enable namespace.
> 
> Why can't we make it KConfig option ? Is there any particular reason
> for not doing that ? That will also allow user to avoid kernel
> compilation of code if they want to turn it off.

Kernel config options generally don't fix anything, as distros enable
everything anyway. It's mostly used as a cop-out if adding something and
people complain that it's slowing things down, and then the answer is
"well you can just not enable it". Not useful when distros blindly
enable everything anyway, it solves absolutely nothing.

Either it's cheap enough that it should just be generally available, or
it should be written in such a way that it is cheap enough.

-- 
Jens Axboe