Skip to contentSkip to conversation

ocfs2: fix module_init error handling

PATCH In review Akinobu Mita 3 participants 4 messages Mar 4, 2007 – Mar 6, 2007 OCFS2

lore
Share & tools
lore

Akinobu Mita

author
#0 lore raw

PATCH 1/1 ocfs2: fix module_init error handling

From: Akinobu Mita <[email protected]>
Subject: [PATCH] ocfs2: fix module_init error handling

Fix error handling in module_init and make module_init() return correct error code.

Signed-off-by Akinobu Mita <[email protected]>
Cc Mark Fasheh <[email protected]>
Cc Kurt Hackel <[email protected]>
---
 fs/ocfs2/super.c |   42 +++++++++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 15 deletions(-)
fs/ocfs2/super.c +27 -15
Index: 2.6-mm/fs/ocfs2/super.c
===================================================================
@@ -806,46 +806,58 @@ static int __init ocfs2_init(void)
 
 	ocfs2_print_version();
 
-	if (init_ocfs2_extent_maps())
-		return -ENOMEM;
+	status = init_ocfs2_extent_maps();
+	if (status)
+		goto err_extent_maps;
 
 	status = init_ocfs2_uptodate_cache();
 	if (status < 0) {
 		mlog_errno(status);
-		goto leave;
+		goto err_uptodate_cache;
 	}
 
 	status = ocfs2_initialize_mem_caches();
 	if (status < 0) {
 		mlog_errno(status);
-		goto leave;
+		goto err_mem_caches;
 	}
 
 	ocfs2_wq = create_singlethread_workqueue("ocfs2_wq");
 	if (!ocfs2_wq) {
 		status = -ENOMEM;
-		goto leave;
+		goto err_workqueue;
 	}
 
 	ocfs2_debugfs_root = debugfs_create_dir("ocfs2", NULL);
 	if (!ocfs2_debugfs_root) {
 		status = -EFAULT;
 		mlog(ML_ERROR, "Unable to create ocfs2 debugfs root.\n");
+		goto err_debugfs;
 	}
 
-leave:
-	if (status < 0) {
-		ocfs2_free_mem_caches();
-		exit_ocfs2_uptodate_cache();
-		exit_ocfs2_extent_maps();
-	}
+	status = register_filesystem(&ocfs2_fs_type);
+	if (status)
+		goto err_filesystem;
 
 	mlog_exit(status);
 
-	if (status >= 0) {
-		return register_filesystem(&ocfs2_fs_type);
-	} else
-		return -1;
+	return 0;
+
+err_filesystem:
+	debugfs_remove(ocfs2_debugfs_root);
+err_debugfs:
+	flush_workqueue(ocfs2_wq);
+	destroy_workqueue(ocfs2_wq);
+err_workqueue:
+	ocfs2_free_mem_caches();
+err_mem_caches:
+	exit_ocfs2_uptodate_cache();
+err_uptodate_cache:
+	exit_ocfs2_extent_maps();
+err_extent_maps:
+	mlog_exit(status);
+
+	return status;
 }
 
 static void __exit ocfs2_exit(void)
#1 lore raw
Quoted 5 lines · Akinobu Mita

From: Akinobu Mita <[email protected]>
Subject: [PATCH] ocfs2: fix module_init error handling

Fix error handling in module_init and make module_init() return correct error code.

What are you fixing specifically? This looks like you are
reorganizing the exit path. While stylistic differences can exist on the choice of function exit, claiming there is a "fix" means there is an actual bug. Please clarify the bug so we can evaluate your change.

fs/ocfs2/super.c static int __init ocfs2_init(void) · line 842

Quoted 11 lines
fs/ocfs2/super.c +3 -6
-leave:
-	if (status < 0) {
-		ocfs2_free_mem_caches();
-		exit_ocfs2_uptodate_cache();
-		exit_ocfs2_extent_maps();
-	}
+	status = register_filesystem(&ocfs2_fs_type);
+	if (status)
+		goto err_filesystem;
 
 	mlog_exit(status);

Also, this change suppresses some paths to mlog_exit(), which
impacts people tracing.
If there is an actual bug you are fixing, please tell us. We'll
work with you to correct it.

Joel

Signature · 9 lines
Life's Little Instruction Book #451

	"Don't be afraid to say, 'I'm sorry.'"

Joel Becker
Principal Software Developer
Oracle
E-mail: [email protected]
Phone: (650) 506-8127
author
#2 lore raw

2007/3/6, Joel Becker <[email protected]>:

Quoted 11 lines · Akinobu Mita
Quoted 5 lines

From: Akinobu Mita <[email protected]>
Subject: [PATCH] ocfs2: fix module_init error handling

Fix error handling in module_init and make module_init() return correct error code.

What are you fixing specifically? This looks like you are
reorganizing the exit path. While stylistic differences can exist on the choice of function exit, claiming there is a "fix" means there is an actual bug. Please clarify the bug so we can evaluate your change.

OK. There are three problems (these are not likely to happen)

- There is no error handling when register_filesystem() fails (several slab caches, debugfs directory, and workqueue will not be cleaned)

- workqueue will not be cleaned if debugfs_create_dir() returns error.

- kmem_cache_destroy() with NULL oops will happen if ocfs2_free_mem_caches(), exit_ocfs2_uptodate_cache(), or exit_ocfs2_extent_maps() return error.

#3 lore raw
Quoted 10 lines · Akinobu Mita

OK. There are three problems (these are not likely to happen)

- There is no error handling when register_filesystem() fails (several slab caches, debugfs directory, and workqueue will not be cleaned)

- workqueue will not be cleaned if debugfs_create_dir() returns error.

- kmem_cache_destroy() with NULL oops will happen if ocfs2_free_mem_caches(), exit_ocfs2_uptodate_cache(), or exit_ocfs2_extent_maps() return error.

Ok, both those patches are in ocfs2.git now.

	--Mark

--
Mark Fasheh
Senior Software Developer, Oracle
[email protected]