diff mbox series

btrfs: add helper to exit_btrfs_fs

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

Commit Message

Anand Jain Oct. 14, 2022, 12:17 p.m. UTC
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(-)

Comments

David Sterba Oct. 17, 2022, 5:59 p.m. UTC | #1
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.
Anand Jain Oct. 19, 2022, 4:27 p.m. UTC | #2
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 mbox series

Patch

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