Re: [PATCH 2/3] module: make structure definitions always visible

[Daniel Gomez <da.gomez@kernel.org>]

On 11/07/2025 08.29, Thomas WeiÃschuh wrote:
> On Mon, Jul 07, 2025 at 09:11:05PM +0200, Daniel Gomez wrote:
>> On 12/06/2025 16.53, Thomas WeiÃschuh wrote:
>>> To write code that works with both CONFIG_MODULES=y and CONFIG_MODULES=n
>>> it is convenient to use "if (IS_ENABLED(CONFIG_MODULES))" over raw #ifdef.
>>> The code will still fully typechecked but the unreachable parts are
>>> discarded by the compiler. This prevents accidental breakage when a certain
>>> kconfig combination was not specifically tested by the developer.
>>> This pattern is already supported to some extend by module.h defining
>>> empty stub functions if CONFIG_MODULES=n.
>>> However some users of module.h work on the structured defined by module.h.
>>>
>>> Therefore these structure definitions need to be visible, too.
>>
>> We are missing here which structures are needed. + we are making more things
>> visible than what we actually need.
>>
>>>
>>> Many structure members are still gated by specific configuration settings.
>>> The assumption for those is that the code using them will be gated behind
>>> the same configuration setting anyways.
>>
>> I think code and kconfig need to reflect the actual dependencies. For example,
>> if CONFIG_LIVEPATCH depends on CONFIG_MODULES, we need to specify that in
>> Kconfig with depends on, as well as keep the code gated by these 2 configs with
>> ifdef/IS_ENABLED.
> 
> If CONFIG_LIVEPATCH depends on CONFIG_MODULES in kconfig then
> IS_ENABLED(CONFIG_LIVEPATCH) will depend on CONFIG_MODULES automatically.
> There is no need for another explicit IS_ENABLED(CONFIG_MODULES).

This makes sense to me. My assessment before to reflect in code what we have in
kconfig does not scale. Thanks.

> 
>>>
>>> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
>>> ---
>>>  include/linux/module.h | 23 ++++++++++++-----------
>>>  1 file changed, 12 insertions(+), 11 deletions(-)

{...}

>>
>> After the patch, mod_tree_node is not needed externally.
> 
> Can you explain what you mean with "not needed externally"?
> 'struct mod_tree_node' is only ever used by core module code.
> It is only public because it is embedded in the public 'struct module'

But only when MODULES_TREE_LOOKUP is enabled. Now, all kernels (regardless of
that config) will define mod_tree_node data structure.

However, Petr already stated that is harmless to do so. I was trying here to
not be useless.

With that, changes look good to me:

Reviewed-by: Daniel Gomez <da.gomez@samsung.com>