Re: [PATCH] kernel: make module.c itself more explicitly non-modular

[Paul Gortmaker]

On 2015-08-26 12:06 AM, Rusty Russell wrote:
> Paul Gortmaker <paul.gortmaker@xxxxxxxxxxxxx> writes:
>> The Kconfig currently controlling compilation of this code is:
>>
>> menuconfig MODULES
>>    bool "Enable loadable module support"
>>
>> ...meaning that it currently is not being built as a module by anyone.
>> No surprise here, since modular support being a module would be an
>> interesting chicken before the egg problem.
>>
>> Lets remove the use of module_init in this code so that when reading
>> the file, there is less doubt that it is builtin-only.
>>
>> Since module_init translates to device_initcall in the non-modular
>> case, the init ordering remains unchanged with this commit.  However
>> one could argue that fs_initcall makes more sense for proc stuff,
>> and we can change the initcall order later and watch for fallout
>> if so desired.
> 
> This patch is just weird; is this part of something larger you are
> trying to do?

Yes, it is part of a larger cleanup; for subsystems with more than
one patch I created a 0/N explanatory note; such as:

https://lkml.kernel.org/r/1440459295-21814-1-git-send-email-paul.gortmaker@xxxxxxxxxxxxx
https://lkml.kernel.org/r/1437530538-5078-1-git-send-email-paul.gortmaker@xxxxxxxxxxxxx

and others. Apologies for the lack of context on this single patch.
 
> I would argue that module_init() should be the default; it implies
> no dependencies on the initialization, and it's a common pattern.

To summarize briefly, module_init forces everything into one
initcall priority bin, it encourages people to write module_exit
functions that are never used, and it can make the code appear
inconsistent with the Kconfig and/or Makefile settings.  So I'd
hope you'd agree that there is value in not using module_init
in code that can never be modular.

Thanks,
Paul.
--

> 
> Cheers,
> Rusty.
> 
>> We can't of course delete the module.h include in this case since it
>> is used all through the rest of the file.
>>
>> Cc: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
>> Signed-off-by: Paul Gortmaker <paul.gortmaker@xxxxxxxxxxxxx>
>> ---
>>
>> [I was undecided as to whether we should do this in one step
>>  or two, i.e. instead just make the change to fs_initcall here
>>  and now, and so went with the more cautious/granular approach.]
>>
>>  kernel/module.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/module.c b/kernel/module.c
>> index 8f051a106676..7750bdcb12fc 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -3947,7 +3947,7 @@ static int __init proc_modules_init(void)
>>  	proc_create("modules", 0, NULL, &proc_modules_operations);
>>  	return 0;
>>  }
>> -module_init(proc_modules_init);
>> +device_initcall(proc_modules_init);
>>  #endif
>>  
>>  /* Given an address, look for it in the module exception tables. */
>> -- 
>> 2.5.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/