Message ID | a1e9999846422fc971c5bb5f79d517de6530a55a.1665745675.git.anand.jain@oracle.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | btrfs: add helper to exit_btrfs_fs | expand |
On Fri, Oct 14, 2022 at 08:17:26PM +0800, Anand Jain wrote: > The module exit function exit_btrfs_fs() is duplicating a section of code > in init_btrfs_fs(). So add a helper function to remove the duplicate code. > Also, remove the comment about the function's .text section, which > doesn't make sense. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > --- > David, > This patch is on top of Qu's patch on the mailing list. > btrfs: make btrfs module init/exit match their sequence > > This patch passed, make mrproper, compile with defconfig and Oracle Linux config. > and, Module load/unload. > I suggested this change in the review comment, but it wasn't going anywhere. > Instead, I found sending a patch is more productive. Please, keep my SOB. So there's the for() { } duplication but we need to be careful about the sections. Can we rather use some inlining tricks to force the common code to be inlined and avoid dealing with the sections explicitly? Eg. using __always_inline.
On 18/10/2022 01:59, David Sterba wrote: > On Fri, Oct 14, 2022 at 08:17:26PM +0800, Anand Jain wrote: >> The module exit function exit_btrfs_fs() is duplicating a section of code >> in init_btrfs_fs(). So add a helper function to remove the duplicate code. >> Also, remove the comment about the function's .text section, which >> doesn't make sense. >> >> Signed-off-by: Anand Jain <anand.jain@oracle.com> >> --- >> David, >> This patch is on top of Qu's patch on the mailing list. >> btrfs: make btrfs module init/exit match their sequence >> >> This patch passed, make mrproper, compile with defconfig and Oracle Linux config. >> and, Module load/unload. >> I suggested this change in the review comment, but it wasn't going anywhere. >> Instead, I found sending a patch is more productive. Please, keep my SOB. > > So there's the for() { } duplication but we need to be careful about the > sections. Can we rather use some inlining tricks to force the common > code to be inlined and avoid dealing with the sections explicitly? Eg. > using __always_inline. Yeah, must avoid section mismatch, explicitly making it inline is better. Sent v2. (sorry for the delay in response). Thanks, Anand
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 2356b744828d..0c48bf562aa8 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -2791,7 +2791,7 @@ static const struct init_sequence mod_init_seq[] = { static bool mod_init_result[ARRAY_SIZE(mod_init_seq)]; -static void __exit exit_btrfs_fs(void) +static void btrfs_exit_btrfs_fs(void) { int i; @@ -2804,6 +2804,11 @@ static void __exit exit_btrfs_fs(void) } } +static void __exit exit_btrfs_fs(void) +{ + btrfs_exit_btrfs_fs(); +} + static int __init init_btrfs_fs(void) { int ret; @@ -2812,26 +2817,13 @@ static int __init init_btrfs_fs(void) for (i = 0; i < ARRAY_SIZE(mod_init_seq); i++) { ASSERT(!mod_init_result[i]); ret = mod_init_seq[i].init_func(); - if (ret < 0) - goto error; + if (ret < 0) { + btrfs_exit_btrfs_fs(); + return ret; + } mod_init_result[i] = true; } return 0; - -error: - /* - * If we call exit_btrfs_fs() it would cause section mismatch. - * As init_btrfs_fs() belongs to .init.text, while exit_btrfs_fs() - * belongs to .exit.text. - */ - for (i = ARRAY_SIZE(mod_init_seq) - 1; i >= 0; i--) { - if (!mod_init_result[i]) - continue; - if (mod_init_seq[i].exit_func) - mod_init_seq[i].exit_func(); - mod_init_result[i] = false; - } - return ret; } late_initcall(init_btrfs_fs);
The module exit function exit_btrfs_fs() is duplicating a section of code in init_btrfs_fs(). So add a helper function to remove the duplicate code. Also, remove the comment about the function's .text section, which doesn't make sense. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- David, This patch is on top of Qu's patch on the mailing list. btrfs: make btrfs module init/exit match their sequence This patch passed, make mrproper, compile with defconfig and Oracle Linux config. and, Module load/unload. I suggested this change in the review comment, but it wasn't going anywhere. Instead, I found sending a patch is more productive. Please, keep my SOB. fs/btrfs/super.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-)