Re: [PATCH 2/2] nvme: keyring: fix conditional compilation

Arnd Bergmann
2023-10-26T14:45:34Z

On Thu, Oct 26, 2023, at 16:20, Hannes Reinecke wrote:
> On 10/26/23 15:37, Arnd Bergmann wrote:
>> On Thu, Oct 26, 2023, at 15:08, Hannes Reinecke wrote:
>>> diff --git a/drivers/nvme/common/Kconfig b/drivers/nvme/common/Kconfig
>>> index 244432e0b73d..96031016079f 100644
>>> --- a/drivers/nvme/common/Kconfig
>>> +++ b/drivers/nvme/common/Kconfig
>>> @@ -1,7 +1,7 @@
>>> # SPDX-License-Identifier: GPL-2.0-only
>>>
>>> config NVME_KEYRING
>>> - tristate
>>> + bool
>>> select KEYS
>>
>> I guess that's one way to address the link failure ;-)
>>
>> It feels like cheating to force it built-in even if
>> both target and host support is in loadable module.
>>
> Arguably.
> But the decision matrix really has only limited choices:
>
> Host Target Keyring
> n n n
> n m m
> n y y
> m n m
> m m m
> m y y
> y n y
> y m y
> y y y
>
> So we're correct in 75% of all cases :-)
> And before we trying to figure out some weird complex kconfig syntax
> to get all cases correct I prefer the easy solution.
> Plus it has the benefit that the keyring is avialable right from the
> start, so you can pre-provision keys even before nvme is loaded.

Your original version already had the logic for doing
this part right and always linking the "common" module
as built-in if needed. Maybe just replace the "PATCH 1/2"
with a different approach then.

My feeling is still that my v1 was the simplest solution,
but that does exactly the right thing in the end, but if
you absolutely want to keep the #if/#else block in the
header instead of the "if (IS_ENABLED())" checks, then
you could also get there by moving the bits that
are actually common (the MODULE_LICENSE, MODULE_AUTHOR,
module_init and module_exit tags) into a third file
that is always part of nvme-common.ko.

Arnd