Re: [PATCH 1/4] module: Add module_subinit{_noexit} and module_subeixt helper macros

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 27/07/2024 22:52, Theodore Ts'o wrote:
On Fri, Jul 26, 2024 at 11:09:02AM -0700, Christoph Hellwig wrote:
On Fri, Jul 26, 2024 at 01:58:00PM -0400, Theodore Ts'o wrote:
Yeah, that's my reaction as well.  This only saves 50 lines of code in
ext4, and that includes unrelated changes such as getting rid of "int
i" and putting the declaration into the for loop --- "for (int i =
...").  Sure, that saves two lines of code, but yay?

If the ordering how the functions gets called is based on the magic
ordering in the Makefile, I'm not sure this actually makes the code
clearer, more robust, and easier to maintain for the long term.
So you two object to kernel initcalls for the same reason and would
rather go back to calling everything explicitly?
I don't oject to kernel initcalls which don't have any
interdependencies and where ordering doesn't matter.
1. Previous version implementation: array mode (see link 1) :
   Advantages:
   - Few changes, simple principle, easy to understand code.
   Disadvantages:
   - Each modified module needs to maintain an array, more code.

2. Current implementation: explicit call subinit in initcall (see link 2) :
   Advantages:
   - Direct use of helpes macros, the subinit call sequence is
     intuitive, and the implementation is relatively simple.
   Disadvantages:
   - helper macros need to be implemented compared to array mode.

3. Only one module_subinit per file (not implemented, see link 3) :
   Advantage:
   - No need to display to call subinit.
   Disadvantages:
   - Magic order based on Makefile makes code more fragile,
   - Make sure that each file has only one module_subinit,
   - It is not intuitive to know which subinits the module needs
     and in what order (grep and Makefile are required),
   - With multiple subinits per module, it would be difficult to
     define module_{subinit, subexit} by MODULE, and difficult to
     rollback when initialization fails (I haven't found a good way
     to do this yet).


Personally, I prefer the implementation of method two.

Links:
[1]: https://lore.kernel.org/all/20240711074859.366088-4-youling.tang@xxxxxxxxx/ [2]: https://lore.kernel.org/all/20240723083239.41533-2-youling.tang@xxxxxxxxx/
[3]: https://lore.kernel.org/all/ZqKreStOD-eRkKZU@xxxxxxxxxxxxx/

Thanks,
Youling.




[Index of Archives]     [Linux Kernel]     [Kernel Newbies]     [x86 Platform Driver]     [Netdev]     [Linux Wireless]     [Netfilter]     [Bugtraq]     [Linux Filesystems]     [Yosemite Discussion]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]

  Powered by Linux