Btrfs devel - Nov 2007 - [PATCH] return value checking in module init

Hi,

Thanks for the feedback on my last version of this.

Is anyone using kernel versions under 2.6.23 for development?

Also, I was going to ask about the use of labels in the classic sense of:

alloc1
if alloc1 fails
	exit
alloc2
if alloc2 fails
	goto free1
alloc3
if alloc3 fails
	goto free2
return 0

free2:
free1:
return foo

Since btrfs_destroy_cachep() just uses branching to free memory.  I'm
guessing the
feedback on this label heavy patch will explain either:

A. branching is the quick and dirty, while we're in development method.
B. I'm missing the point of how Btrfs is intending to use labels
C. something else I missed?

Apparently I got so caught up in seeing how and why memory is freed in a
certain way I forgot to free any in my last send. :-)

This patch compiles cleanly except for files I did not touch.  I created a new
file system on my external usb drive I use for experimentation and created a few
files
to test.

Anyone who can suggest other ways I can test, please let me know.

Thanks
Wyatt Banks

Patched against http://oss.oracle.com/mercurial/mason/btrfs-unstable

diff -r 454189bdd3ed extent_map.c
--- a/extent_map.c	Fri Nov 16 11:45:54 2007 -0500
+++ b/extent_map.c	Fri Nov 16 19:54:26 2007 -0500
@@ -43,17 +43,31 @@ struct extent_page_data {
	get_extent_t *get_extent;
 };

-void __init extent_map_init(void)
+int __init extent_map_init(void)
 {
	extent_map_cache = btrfs_cache_create("extent_map",
					    sizeof(struct extent_map), 0,
					    NULL);
+	if (!extent_map_cache)
+		return -ENOMEM;
	extent_state_cache = btrfs_cache_create("extent_state",
					    sizeof(struct extent_state), 0,
					    NULL);
+	if (!extent_state_cache)
+		goto free_map_cache;
	extent_buffer_cache = btrfs_cache_create("extent_buffers",
					    sizeof(struct extent_buffer), 0,
					    NULL);
+	if (!extent_buffer_cache)
+		goto free_state_cache;
+
+	return 0;
+
+free_state_cache:
+	kmem_cache_destroy(extent_state_cache);
+free_map_cache:
+	kmem_cache_destroy(extent_map_cache);
+	return -ENOMEM;
 }

 void __exit extent_map_exit(void)
diff -r 454189bdd3ed extent_map.h
--- a/extent_map.h	Fri Nov 16 11:45:54 2007 -0500
+++ b/extent_map.h	Fri Nov 16 19:54:26 2007 -0500
@@ -110,7 +110,7 @@ void free_extent_map(struct extent_map *
 void free_extent_map(struct extent_map *em);
 int extent_read_full_page(struct extent_map_tree *tree, struct page *page,
			  get_extent_t *get_extent);
-void __init extent_map_init(void);
+int __init extent_map_init(void);
 void __exit extent_map_exit(void);

 int test_range_bit(struct extent_map_tree *tree, u64 start, u64 end,
diff -r 454189bdd3ed super.c
--- a/super.c	Fri Nov 16 11:45:54 2007 -0500
+++ b/super.c	Fri Nov 16 19:54:26 2007 -0500
@@ -347,9 +347,24 @@ static int __init init_btrfs_fs(void)
	btrfs_init_transaction_sys();
	err = btrfs_init_cachep();
        if (err)
-		return err;
-	extent_map_init();
-	return register_filesystem(&btrfs_fs_type);
+		goto free_transaction_sys;
+	err = extent_map_init();
+	if (err)
+		goto free_cachep;
+
+	err = register_filesystem(&btrfs_fs_type);
+	if (err)
+		goto free_extent_map;
+	return 0;
+
+free_extent_map:
+	extent_map_exit();
+free_cachep:
+	btrfs_destroy_cachep();
+free_transaction_sys:
+	btrfs_exit_transaction_sys();
+	btrfs_exit_sysfs();
+	return err;
 }

 static void __exit exit_btrfs_fs(void)

On Fri, 16 Nov 2007 20:21:24 -0500
Wyatt Banks <wyatt@banksresearch.com> wrote:
> Hi,
> 
> Thanks for the feedback on my last version of this.
> 
> Is anyone using kernel versions under 2.6.23 for development?
> 
> Also, I was going to ask about the use of labels in the classic sense
The usage in your patch was fine, thanks for rediffing these.  I've
pushed your fix into the unstable tree.

-chris

-chris