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

Arnd Bergmann
2023-10-27T08:12:47Z

On Fri, Oct 27, 2023, at 08:01, Hannes Reinecke wrote:
> On 10/27/23 07:21, Christoph Hellwig wrote:
>> On Thu, Oct 26, 2023 at 04:20:12PM +0200, Hannes Reinecke wrote:
>>> 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.
>>
>> in the 75% of cases that don't really matter, as 99% of all setups
>> have nvme and nvmet built modular, and for that you now build code
>> into the kernel for no good reason at all.
>>
>> FYI, what's I've done a lot in the past for such simple helper is
>> to not have a Kconfig symbol at all, but let the Makefile handle
>> it.
>>
>> A
>>
>> obj-$(CONFIG_MOD1) += mod1.o mod-common.o
>> obj-$(CONFIG_MOD2) += mod2.o mod-common.o
>>
>> will actually do the right thing here without much complicated
>> boilerplate.

Right, this is what I meant referring to the usual Kconfig
or Makefile logic. The example above needs an extra conditional
here to deal with the keyring code being turned on or off
altogether in addition to being used from any combination
of built-in or modular host/target modules, but none of this
should require unusual hacks.

> In principle. Unfortunately I have to initialize the keyring, and that
> can be done only once.
> I see if I can come up with a different solution.

If the keyring has to be initialized first, I think the safest
way is to move it to a different initcall level, to avoid
relying on link order for the everything-built-in case. The
other cases are handled automatically once you get it to
link properly, as module load order takes care of the
all-modular case, and with the keyring built-in it also comes
before any modules.

Arnd