All of lore.kernel.org
 help / color / mirror / Atom feed
From: Youling Tang <youling.tang@linux.dev>
To: Christoph Hellwig <hch@infradead.org>
Cc: Arnd Bergmann <arnd@arndb.de>,
	Luis Chamberlain <mcgrof@kernel.org>, Chris Mason <clm@fb.com>,
	Josef Bacik <josef@toxicpanda.com>,
	David Sterba <dsterba@suse.com>,
	tytso@mit.edu, Andreas Dilger <adilger.kernel@dilger.ca>,
	Jaegeuk Kim <jaegeuk@kernel.org>, Chao Yu <chao@kernel.org>,
	linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-modules@vger.kernel.org, linux-btrfs@vger.kernel.org,
	linux-ext4@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net,
	Youling Tang <tangyouling@kylinos.cn>
Subject: Re: [PATCH 1/4] module: Add module_subinit{_noexit} and module_subeixt helper macros
Date: Thu, 25 Jul 2024 11:01:33 +0800	[thread overview]
Message-ID: <895360e3-97bb-4188-a91d-eaca3302bd43@linux.dev> (raw)
In-Reply-To: <ZqEhMCjdFwC3wF4u@infradead.org>

On 24/07/2024 23:43, Christoph Hellwig wrote:
> On Wed, Jul 24, 2024 at 09:57:05AM +0800, Youling Tang wrote:
>> module_init(initfn)/module_exit(exitfn) has two definitions (via MODULE):
>> - buindin: uses do_initcalls() to iterate over the contents of the specified
>>    section and executes all initfn functions in the section in the order in
>>    which they are stored (exitfn is not required).
>>
>> - ko: run do_init_module(mod)->do_one_initcall(mod->init) to execute initfn
>>    of the specified module.
>>
>> If we change module_subinit to something like this, not called in
>> module_init,
>> Not only do we want to ensure that exit is executed in reverse order of
>> init, but we also want to ensure the order of init.
> Yes.
>
>> This does not guarantee the order in which init will be executed (although
>> the init/exit order will remain the same)
> Hmm, so the normal built-in initcalls depend on the link order, but when
> they are in the same file, the compiler can reorder them before we even
> get to the linker.
>
> I wonder what a good syntax would be to still avoid the boilerplate
> code.  We'd probably need one macro to actually define the init/exit
> table in a single statement so that it can't be reordered, but that
> would lose the ability to actually declare the module subinit/exit
> handlers in multiple files, which really is the biggest win of this
> scheme as it allows to keep the functions static instead of exposing
> them to other compilation units.
>
> And in fact even in your three converted file systems, most
> subinit/exit handler are in separate files, so maybe instead
> enforcing that there is just one per file and slightly refactoring
> the code so that this is the case might be the best option?
- It doesn't feel good to have only one subinit/exit in a file.
   Assuming that there is only one file in each file, how do we
   ensure that the files are linked in order?(Is it sorted by *.o
   in the Makefile?)

- Even if the order of each init is linked correctly, then the
   runtime will be iterated through the .subinitcall.init section,
   which executes each initfn in sequence (similar to do_initcalls),
   which means that no other code can be inserted between each subinit.


If module_subinit is called in module_init, other code can be inserted
between subinit, similar to the following:

```
static int __init init_example(void)
{
     module_subinit(inita, exita);

     otherthing...

     module_subinit(initb, exitb);

     return 0;
}

module_init(init_example);
```

IMHO, module_subinit() might be better called in module_init().

Thanks,
Youling.

WARNING: multiple messages have this Message-ID (diff)
From: Youling Tang <youling.tang@linux.dev>
To: Christoph Hellwig <hch@infradead.org>
Cc: linux-arch@vger.kernel.org, Youling Tang <tangyouling@kylinos.cn>,
	linux-f2fs-devel@lists.sourceforge.net, tytso@mit.edu,
	Arnd Bergmann <arnd@arndb.de>, Josef Bacik <josef@toxicpanda.com>,
	linux-kernel@vger.kernel.org, Chris Mason <clm@fb.com>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Andreas Dilger <adilger.kernel@dilger.ca>,
	linux-btrfs@vger.kernel.org, David Sterba <dsterba@suse.com>,
	Jaegeuk Kim <jaegeuk@kernel.org>,
	linux-ext4@vger.kernel.org, linux-modules@vger.kernel.org
Subject: Re: [f2fs-dev] [PATCH 1/4] module: Add module_subinit{_noexit} and module_subeixt helper macros
Date: Thu, 25 Jul 2024 11:01:33 +0800	[thread overview]
Message-ID: <895360e3-97bb-4188-a91d-eaca3302bd43@linux.dev> (raw)
In-Reply-To: <ZqEhMCjdFwC3wF4u@infradead.org>

On 24/07/2024 23:43, Christoph Hellwig wrote:
> On Wed, Jul 24, 2024 at 09:57:05AM +0800, Youling Tang wrote:
>> module_init(initfn)/module_exit(exitfn) has two definitions (via MODULE):
>> - buindin: uses do_initcalls() to iterate over the contents of the specified
>>    section and executes all initfn functions in the section in the order in
>>    which they are stored (exitfn is not required).
>>
>> - ko: run do_init_module(mod)->do_one_initcall(mod->init) to execute initfn
>>    of the specified module.
>>
>> If we change module_subinit to something like this, not called in
>> module_init,
>> Not only do we want to ensure that exit is executed in reverse order of
>> init, but we also want to ensure the order of init.
> Yes.
>
>> This does not guarantee the order in which init will be executed (although
>> the init/exit order will remain the same)
> Hmm, so the normal built-in initcalls depend on the link order, but when
> they are in the same file, the compiler can reorder them before we even
> get to the linker.
>
> I wonder what a good syntax would be to still avoid the boilerplate
> code.  We'd probably need one macro to actually define the init/exit
> table in a single statement so that it can't be reordered, but that
> would lose the ability to actually declare the module subinit/exit
> handlers in multiple files, which really is the biggest win of this
> scheme as it allows to keep the functions static instead of exposing
> them to other compilation units.
>
> And in fact even in your three converted file systems, most
> subinit/exit handler are in separate files, so maybe instead
> enforcing that there is just one per file and slightly refactoring
> the code so that this is the case might be the best option?
- It doesn't feel good to have only one subinit/exit in a file.
   Assuming that there is only one file in each file, how do we
   ensure that the files are linked in order?(Is it sorted by *.o
   in the Makefile?)

- Even if the order of each init is linked correctly, then the
   runtime will be iterated through the .subinitcall.init section,
   which executes each initfn in sequence (similar to do_initcalls),
   which means that no other code can be inserted between each subinit.


If module_subinit is called in module_init, other code can be inserted
between subinit, similar to the following:

```
static int __init init_example(void)
{
     module_subinit(inita, exita);

     otherthing...

     module_subinit(initb, exitb);

     return 0;
}

module_init(init_example);
```

IMHO, module_subinit() might be better called in module_init().

Thanks,
Youling.


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

  reply	other threads:[~2024-07-25  3:01 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-23  8:32 [PATCH 0/4] Add module_subinit{_noexit} and module_subeixt helper macros Youling Tang
2024-07-23  8:32 ` [f2fs-dev] " Youling Tang
2024-07-23  8:32 ` [PATCH 1/4] module: " Youling Tang
2024-07-23  8:32   ` [f2fs-dev] " Youling Tang
2024-07-23  9:58   ` Mika Penttilä
2024-07-23  9:58     ` [f2fs-dev] " Mika Penttilä
2024-07-24  1:20     ` Youling Tang
2024-07-24  1:20       ` [f2fs-dev] " Youling Tang
2024-07-23 14:33   ` Christoph Hellwig
2024-07-23 14:33     ` [f2fs-dev] " Christoph Hellwig
2024-07-24  1:57     ` Youling Tang
2024-07-24  1:57       ` [f2fs-dev] " Youling Tang
2024-07-24 15:43       ` Christoph Hellwig
2024-07-24 15:43         ` [f2fs-dev] " Christoph Hellwig
2024-07-25  3:01         ` Youling Tang [this message]
2024-07-25  3:01           ` Youling Tang
2024-07-25 14:39           ` Christoph Hellwig
2024-07-25 14:39             ` [f2fs-dev] " Christoph Hellwig
2024-07-25 15:30             ` Arnd Bergmann
2024-07-25 15:30               ` [f2fs-dev] " Arnd Bergmann
2024-07-25 15:34               ` Christoph Hellwig
2024-07-25 15:34                 ` [f2fs-dev] " Christoph Hellwig
2024-07-25 17:14                 ` Goffredo Baroncelli
2024-07-25 17:14                   ` [f2fs-dev] " Goffredo Baroncelli via Linux-f2fs-devel
2024-07-25 19:46                   ` Christoph Hellwig
2024-07-25 19:46                     ` [f2fs-dev] " Christoph Hellwig
2024-07-26  8:54                     ` Youling Tang
2024-07-26  8:54                       ` [f2fs-dev] " Youling Tang
2024-07-26 14:04                       ` Christoph Hellwig
2024-07-26 14:04                         ` [f2fs-dev] " Christoph Hellwig
2024-07-26 15:22                         ` David Sterba
2024-07-26 15:22                           ` [f2fs-dev] " David Sterba
2024-07-26 17:58                           ` Theodore Ts'o
2024-07-26 17:58                             ` [f2fs-dev] " Theodore Ts'o
2024-07-26 18:09                             ` Christoph Hellwig
2024-07-26 18:09                               ` [f2fs-dev] " Christoph Hellwig
2024-07-26 22:45                               ` David Sterba
2024-07-26 22:45                                 ` [f2fs-dev] " David Sterba
2024-07-27 14:52                               ` Theodore Ts'o
2024-07-27 14:52                                 ` [f2fs-dev] " Theodore Ts'o
2024-07-29  1:46                                 ` Youling Tang
2024-07-29  1:46                                   ` [f2fs-dev] " Youling Tang
2024-07-29  2:44                                   ` Theodore Ts'o
2024-07-29  2:44                                     ` [f2fs-dev] " Theodore Ts'o
2024-07-29  3:01                                     ` Youling Tang
2024-07-29  3:01                                       ` [f2fs-dev] " Youling Tang
2024-07-29 18:57                                     ` Christoph Hellwig
2024-07-29 18:57                                       ` [f2fs-dev] " Christoph Hellwig
2024-07-23  8:32 ` [PATCH 2/4] btrfs: Use " Youling Tang
2024-07-23  8:32   ` [f2fs-dev] " Youling Tang
2024-07-23 22:24   ` kernel test robot
2024-07-23 22:24     ` [f2fs-dev] " kernel test robot
2024-07-24  6:29     ` Youling Tang
2024-07-24  6:29       ` [f2fs-dev] " Youling Tang
2024-07-23  8:32 ` [PATCH 3/4] ext4: Use module_{subinit, subexit} " Youling Tang
2024-07-23  8:32   ` [f2fs-dev] " Youling Tang
2024-07-23  8:32 ` [PATCH 4/4] f2fs: Use module_{subinit, subeixt} " Youling Tang
2024-07-23  8:32   ` [f2fs-dev] " Youling Tang
2024-07-23 18:51   ` kernel test robot
2024-07-23 18:51     ` [f2fs-dev] " kernel test robot
2024-07-24  2:14     ` Youling Tang
2024-07-24  2:14       ` [f2fs-dev] " Youling Tang
2024-07-23 21:31   ` kernel test robot
2024-07-23 21:31     ` [f2fs-dev] " kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=895360e3-97bb-4188-a91d-eaca3302bd43@linux.dev \
    --to=youling.tang@linux.dev \
    --cc=adilger.kernel@dilger.ca \
    --cc=arnd@arndb.de \
    --cc=chao@kernel.org \
    --cc=clm@fb.com \
    --cc=dsterba@suse.com \
    --cc=hch@infradead.org \
    --cc=jaegeuk@kernel.org \
    --cc=josef@toxicpanda.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=tangyouling@kylinos.cn \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.