[PATCH] btrfs: add helper to exit_btrfs_fs

[PATCH] btrfs: add helper to exit_btrfs_fs

[Anand Jain]

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(-)

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);
-- 
2.33.1

Re: [PATCH] btrfs: add helper to exit_btrfs_fs

[David Sterba]

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.

Re: [PATCH] btrfs: add helper to exit_btrfs_fs

[Anand Jain]

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