jbd/2: eliminate code duplication and gracefully handle cache initialisation failure

jbd/2: eliminate code duplication and gracefully handle cache initialisation failure

[Duane Griffin]

 fs/jbd/journal.c  |  149 +++++++++++++++++++++++-------------------------
 fs/jbd/revoke.c   |  164 +++++++++++++++++++++++------------------------------
 fs/jbd2/journal.c |  154 ++++++++++++++++++++++++-------------------------
 fs/jbd2/revoke.c  |  164 +++++++++++++++++++++++------------------------------
 4 files changed, 291 insertions(+), 340 deletions(-)

This is version 2 of the jdb/2 patches to eliminate code duplication and
gracefully handle cace initialisation failure.

Changes since V1:
 * Check whether caches are initialised in journal_init_common. If not then
   attempt to initialise them once before failing.
 * Addressed all Andreas Dilger's comments.
 * Modified the initialisation/destruction methods to match Christoph Hellwig's
   suggestions.
 * Rolled the if/assertion and debugfs changes into the main patch.

Andrew, please drop the earlier versions of these patches and replace them with
these ones, thanks.

Cheers,
Duane.

-- 
"I never could learn to drink that blood and call it wine" - Bob Dylan

[PATCH] jbd: eliminate duplicated code in revocation table init/destroy functions

[Duane Griffin]

The revocation table initialisation/destruction code is repeated for each of
the two revocation tables stored in the journal. Refactoring the duplicated
code into functions is tidier, simplifies the logic in initialisation in
particular, and slightly reduces the code size.

There should not be any functional change.

Signed-off-by: Duane Griffin <duaneg@dghda.com>
---
 fs/jbd/revoke.c |  127 ++++++++++++++++++++++---------------------------------
 1 files changed, 51 insertions(+), 76 deletions(-)

diff --git a/fs/jbd/revoke.c b/fs/jbd/revoke.c
index ad2eacf..1ef5c80 100644
--- a/fs/jbd/revoke.c
+++ b/fs/jbd/revoke.c
@@ -195,109 +195,84 @@ void journal_destroy_revoke_caches(void)
 	revoke_table_cache = NULL;
 }
 
-/* Initialise the revoke table for a given journal to a given size. */
-
-int journal_init_revoke(journal_t *journal, int hash_size)
+static struct jbd_revoke_table_s *journal_init_revoke_table(int hash_size)
 {
-	int shift, tmp;
+	int shift = 0;
+	int tmp = hash_size;
+	struct jbd_revoke_table_s *table;
 
-	J_ASSERT (journal->j_revoke_table[0] == NULL);
+	table = kmem_cache_alloc(revoke_table_cache, GFP_KERNEL);
+	if (!table)
+		goto out;
 
-	shift = 0;
-	tmp = hash_size;
 	while((tmp >>= 1UL) != 0UL)
 		shift++;
 
-	journal->j_revoke_table[0] = kmem_cache_alloc(revoke_table_cache, GFP_KERNEL);
-	if (!journal->j_revoke_table[0])
-		return -ENOMEM;
-	journal->j_revoke = journal->j_revoke_table[0];
-
-	/* Check that the hash_size is a power of two */
-	J_ASSERT(is_power_of_2(hash_size));
-
-	journal->j_revoke->hash_size = hash_size;
-
-	journal->j_revoke->hash_shift = shift;
-
-	journal->j_revoke->hash_table =
+	table->hash_size = hash_size;
+	table->hash_shift = shift;
+	table->hash_table =
 		kmalloc(hash_size * sizeof(struct list_head), GFP_KERNEL);
-	if (!journal->j_revoke->hash_table) {
-		kmem_cache_free(revoke_table_cache, journal->j_revoke_table[0]);
-		journal->j_revoke = NULL;
-		return -ENOMEM;
+	if (!table->hash_table) {
+		kmem_cache_free(revoke_table_cache, table);
+		table = NULL;
+		goto out;
 	}
 
 	for (tmp = 0; tmp < hash_size; tmp++)
-		INIT_LIST_HEAD(&journal->j_revoke->hash_table[tmp]);
+		INIT_LIST_HEAD(&table->hash_table[tmp]);
 
-	journal->j_revoke_table[1] = kmem_cache_alloc(revoke_table_cache, GFP_KERNEL);
-	if (!journal->j_revoke_table[1]) {
-		kfree(journal->j_revoke_table[0]->hash_table);
-		kmem_cache_free(revoke_table_cache, journal->j_revoke_table[0]);
-		return -ENOMEM;
+out:
+	return table;
+}
+
+static void journal_destroy_revoke_table(struct jbd_revoke_table_s *table)
+{
+	int i;
+	struct list_head *hash_list;
+
+	for (i = 0; i < table->hash_size; i++) {
+		hash_list = &table->hash_table[i];
+		J_ASSERT(list_empty(hash_list));
 	}
 
-	journal->j_revoke = journal->j_revoke_table[1];
+	kfree(table->hash_table);
+	kmem_cache_free(revoke_table_cache, table);
+}
 
-	/* Check that the hash_size is a power of two */
+/* Initialise the revoke table for a given journal to a given size. */
+int journal_init_revoke(journal_t *journal, int hash_size)
+{
+	J_ASSERT(journal->j_revoke_table[0] == NULL);
 	J_ASSERT(is_power_of_2(hash_size));
 
-	journal->j_revoke->hash_size = hash_size;
+	journal->j_revoke_table[0] = journal_init_revoke_table(hash_size);
+	if (!journal->j_revoke_table[0])
+		goto fail0;
 
-	journal->j_revoke->hash_shift = shift;
+	journal->j_revoke_table[1] = journal_init_revoke_table(hash_size);
+	if (!journal->j_revoke_table[1])
+		goto fail1;
 
-	journal->j_revoke->hash_table =
-		kmalloc(hash_size * sizeof(struct list_head), GFP_KERNEL);
-	if (!journal->j_revoke->hash_table) {
-		kfree(journal->j_revoke_table[0]->hash_table);
-		kmem_cache_free(revoke_table_cache, journal->j_revoke_table[0]);
-		kmem_cache_free(revoke_table_cache, journal->j_revoke_table[1]);
-		journal->j_revoke = NULL;
-		return -ENOMEM;
-	}
-
-	for (tmp = 0; tmp < hash_size; tmp++)
-		INIT_LIST_HEAD(&journal->j_revoke->hash_table[tmp]);
+	journal->j_revoke = journal->j_revoke_table[1];
 
 	spin_lock_init(&journal->j_revoke_lock);
 
 	return 0;
-}
 
-/* Destoy a journal's revoke table.  The table must already be empty! */
+fail1:
+	journal_destroy_revoke_table(journal->j_revoke_table[0]);
+fail0:
+	return -ENOMEM;
+}
 
+/* Destroy a journal's revoke table.  The table must already be empty! */
 void journal_destroy_revoke(journal_t *journal)
 {
-	struct jbd_revoke_table_s *table;
-	struct list_head *hash_list;
-	int i;
-
-	table = journal->j_revoke_table[0];
-	if (!table)
-		return;
-
-	for (i=0; i<table->hash_size; i++) {
-		hash_list = &table->hash_table[i];
-		J_ASSERT (list_empty(hash_list));
-	}
-
-	kfree(table->hash_table);
-	kmem_cache_free(revoke_table_cache, table);
-	journal->j_revoke = NULL;
-
-	table = journal->j_revoke_table[1];
-	if (!table)
-		return;
-
-	for (i=0; i<table->hash_size; i++) {
-		hash_list = &table->hash_table[i];
-		J_ASSERT (list_empty(hash_list));
-	}
-
-	kfree(table->hash_table);
-	kmem_cache_free(revoke_table_cache, table);
 	journal->j_revoke = NULL;
+	if (journal->j_revoke_table[0])
+		journal_destroy_revoke_table(journal->j_revoke_table[0]);
+	if (journal->j_revoke_table[1])
+		journal_destroy_revoke_table(journal->j_revoke_table[1]);
 }
 
 
-- 
1.5.3.7

[PATCH] jbd2: eliminate duplicated code in revocation table init/destroy functions

[Duane Griffin]

The revocation table initialisation/destruction code is repeated for each of
the two revocation tables stored in the journal. Refactoring the duplicated
code into functions is tidier, simplifies the logic in initialisation in
particular, and slightly reduces the code size.

There should not be any functional change.

Signed-off-by: Duane Griffin <duaneg@dghda.com>
---
 fs/jbd2/revoke.c |  127 ++++++++++++++++++++++--------------------------------
 1 files changed, 51 insertions(+), 76 deletions(-)

diff --git a/fs/jbd2/revoke.c b/fs/jbd2/revoke.c
index df36f42..07e4703 100644
--- a/fs/jbd2/revoke.c
+++ b/fs/jbd2/revoke.c
@@ -196,109 +196,84 @@ void jbd2_journal_destroy_revoke_caches(void)
 	jbd2_revoke_table_cache = NULL;
 }
 
-/* Initialise the revoke table for a given journal to a given size. */
-
-int jbd2_journal_init_revoke(journal_t *journal, int hash_size)
+static struct jbd2_revoke_table_s *jbd2_journal_init_revoke_table(int hash_size)
 {
-	int shift, tmp;
+	int shift = 0;
+	int tmp = hash_size;
+	struct jbd2_revoke_table_s *table;
 
-	J_ASSERT (journal->j_revoke_table[0] == NULL);
+	table = kmem_cache_alloc(jbd2_revoke_table_cache, GFP_KERNEL);
+	if (!table)
+		goto out;
 
-	shift = 0;
-	tmp = hash_size;
 	while((tmp >>= 1UL) != 0UL)
 		shift++;
 
-	journal->j_revoke_table[0] = kmem_cache_alloc(jbd2_revoke_table_cache, GFP_KERNEL);
-	if (!journal->j_revoke_table[0])
-		return -ENOMEM;
-	journal->j_revoke = journal->j_revoke_table[0];
-
-	/* Check that the hash_size is a power of two */
-	J_ASSERT(is_power_of_2(hash_size));
-
-	journal->j_revoke->hash_size = hash_size;
-
-	journal->j_revoke->hash_shift = shift;
-
-	journal->j_revoke->hash_table =
+	table->hash_size = hash_size;
+	table->hash_shift = shift;
+	table->hash_table =
 		kmalloc(hash_size * sizeof(struct list_head), GFP_KERNEL);
-	if (!journal->j_revoke->hash_table) {
-		kmem_cache_free(jbd2_revoke_table_cache, journal->j_revoke_table[0]);
-		journal->j_revoke = NULL;
-		return -ENOMEM;
+	if (!table->hash_table) {
+		kmem_cache_free(jbd2_revoke_table_cache, table);
+		table = NULL;
+		goto out;
 	}
 
 	for (tmp = 0; tmp < hash_size; tmp++)
-		INIT_LIST_HEAD(&journal->j_revoke->hash_table[tmp]);
+		INIT_LIST_HEAD(&table->hash_table[tmp]);
 
-	journal->j_revoke_table[1] = kmem_cache_alloc(jbd2_revoke_table_cache, GFP_KERNEL);
-	if (!journal->j_revoke_table[1]) {
-		kfree(journal->j_revoke_table[0]->hash_table);
-		kmem_cache_free(jbd2_revoke_table_cache, journal->j_revoke_table[0]);
-		return -ENOMEM;
+out:
+	return table;
+}
+
+static void jbd2_journal_destroy_revoke_table(struct jbd2_revoke_table_s *table)
+{
+	int i;
+	struct list_head *hash_list;
+
+	for (i = 0; i < table->hash_size; i++) {
+		hash_list = &table->hash_table[i];
+		J_ASSERT(list_empty(hash_list));
 	}
 
-	journal->j_revoke = journal->j_revoke_table[1];
+	kfree(table->hash_table);
+	kmem_cache_free(jbd2_revoke_table_cache, table);
+}
 
-	/* Check that the hash_size is a power of two */
+/* Initialise the revoke table for a given journal to a given size. */
+int jbd2_journal_init_revoke(journal_t *journal, int hash_size)
+{
+	J_ASSERT(journal->j_revoke_table[0] == NULL);
 	J_ASSERT(is_power_of_2(hash_size));
 
-	journal->j_revoke->hash_size = hash_size;
+	journal->j_revoke_table[0] = jbd2_journal_init_revoke_table(hash_size);
+	if (!journal->j_revoke_table[0])
+		goto fail0;
 
-	journal->j_revoke->hash_shift = shift;
+	journal->j_revoke_table[1] = jbd2_journal_init_revoke_table(hash_size);
+	if (!journal->j_revoke_table[1])
+		goto fail1;
 
-	journal->j_revoke->hash_table =
-		kmalloc(hash_size * sizeof(struct list_head), GFP_KERNEL);
-	if (!journal->j_revoke->hash_table) {
-		kfree(journal->j_revoke_table[0]->hash_table);
-		kmem_cache_free(jbd2_revoke_table_cache, journal->j_revoke_table[0]);
-		kmem_cache_free(jbd2_revoke_table_cache, journal->j_revoke_table[1]);
-		journal->j_revoke = NULL;
-		return -ENOMEM;
-	}
-
-	for (tmp = 0; tmp < hash_size; tmp++)
-		INIT_LIST_HEAD(&journal->j_revoke->hash_table[tmp]);
+	journal->j_revoke = journal->j_revoke_table[1];
 
 	spin_lock_init(&journal->j_revoke_lock);
 
 	return 0;
-}
 
-/* Destoy a journal's revoke table.  The table must already be empty! */
+fail1:
+	jbd2_journal_destroy_revoke_table(journal->j_revoke_table[0]);
+fail0:
+	return -ENOMEM;
+}
 
+/* Destroy a journal's revoke table.  The table must already be empty! */
 void jbd2_journal_destroy_revoke(journal_t *journal)
 {
-	struct jbd2_revoke_table_s *table;
-	struct list_head *hash_list;
-	int i;
-
-	table = journal->j_revoke_table[0];
-	if (!table)
-		return;
-
-	for (i=0; i<table->hash_size; i++) {
-		hash_list = &table->hash_table[i];
-		J_ASSERT (list_empty(hash_list));
-	}
-
-	kfree(table->hash_table);
-	kmem_cache_free(jbd2_revoke_table_cache, table);
-	journal->j_revoke = NULL;
-
-	table = journal->j_revoke_table[1];
-	if (!table)
-		return;
-
-	for (i=0; i<table->hash_size; i++) {
-		hash_list = &table->hash_table[i];
-		J_ASSERT (list_empty(hash_list));
-	}
-
-	kfree(table->hash_table);
-	kmem_cache_free(jbd2_revoke_table_cache, table);
 	journal->j_revoke = NULL;
+	if (journal->j_revoke_table[0])
+		jbd2_journal_destroy_revoke_table(journal->j_revoke_table[0]);
+	if (journal->j_revoke_table[1])
+		jbd2_journal_destroy_revoke_table(journal->j_revoke_table[1]);
 }
 
 
-- 
1.5.3.7

[PATCH] jbd: tidy up revoke cache initialisation and destruction

[Duane Griffin]

Make revocation cache destruction safe to call if initialisation fails
partially or entirely. This allows it to be used to cleanup in the case of
initialisation failure, simplifying that code slightly.

Signed-off-by: Duane Griffin <duaneg@dghda.com>
---
 fs/jbd/revoke.c |   35 ++++++++++++++++++++---------------
 1 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/fs/jbd/revoke.c b/fs/jbd/revoke.c
index 1ef5c80..c1fcd62 100644
--- a/fs/jbd/revoke.c
+++ b/fs/jbd/revoke.c
@@ -166,33 +166,38 @@ static struct jbd_revoke_record_s *find_revoke_record(journal_t *journal,
 	return NULL;
 }
 
+void journal_destroy_revoke_caches(void)
+{
+	if (revoke_record_cache) {
+		kmem_cache_destroy(revoke_record_cache);
+		revoke_record_cache = NULL;
+	}
+	if (revoke_table_cache) {
+		kmem_cache_destroy(revoke_table_cache);
+		revoke_table_cache = NULL;
+	}
+}
+
 int __init journal_init_revoke_caches(void)
 {
+	J_ASSERT(!revoke_record_cache);
+	J_ASSERT(!revoke_table_cache);
+
 	revoke_record_cache = kmem_cache_create("revoke_record",
 					   sizeof(struct jbd_revoke_record_s),
 					   0,
 					   SLAB_HWCACHE_ALIGN|SLAB_TEMPORARY,
 					   NULL);
-	if (revoke_record_cache == 0)
-		return -ENOMEM;
-
 	revoke_table_cache = kmem_cache_create("revoke_table",
 					   sizeof(struct jbd_revoke_table_s),
 					   0, SLAB_TEMPORARY, NULL);
-	if (revoke_table_cache == 0) {
-		kmem_cache_destroy(revoke_record_cache);
-		revoke_record_cache = NULL;
+
+	if (revoke_record_cache && revoke_table_cache) {
+		return 0;
+	} else {
+		journal_destroy_revoke_caches();
 		return -ENOMEM;
 	}
-	return 0;
-}
-
-void journal_destroy_revoke_caches(void)
-{
-	kmem_cache_destroy(revoke_record_cache);
-	revoke_record_cache = NULL;
-	kmem_cache_destroy(revoke_table_cache);
-	revoke_table_cache = NULL;
 }
 
 static struct jbd_revoke_table_s *journal_init_revoke_table(int hash_size)
-- 
1.5.3.7

[PATCH] jbd2: tidy up revoke cache initialisation and destruction

[Duane Griffin]

Make revocation cache destruction safe to call if initialisation fails
partially or entirely. This allows it to be used to cleanup in the case of
initialisation failure, simplifying that code slightly.

Signed-off-by: Duane Griffin <duaneg@dghda.com>
---
 fs/jbd2/revoke.c |   35 ++++++++++++++++++++---------------
 1 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/fs/jbd2/revoke.c b/fs/jbd2/revoke.c
index 07e4703..5a020f1 100644
--- a/fs/jbd2/revoke.c
+++ b/fs/jbd2/revoke.c
@@ -167,33 +167,38 @@ static struct jbd2_revoke_record_s *find_revoke_record(journal_t *journal,
 	return NULL;
 }
 
+void jbd2_journal_destroy_revoke_caches(void)
+{
+	if (jbd2_revoke_record_cache) {
+		kmem_cache_destroy(jbd2_revoke_record_cache);
+		jbd2_revoke_record_cache = NULL;
+	}
+	if (jbd2_revoke_table_cache) {
+		kmem_cache_destroy(jbd2_revoke_table_cache);
+		jbd2_revoke_table_cache = NULL;
+	}
+}
+
 int __init jbd2_journal_init_revoke_caches(void)
 {
+	J_ASSERT(!jbd2_revoke_record_cache);
+	J_ASSERT(!jbd2_revoke_table_cache);
+
 	jbd2_revoke_record_cache = kmem_cache_create("jbd2_revoke_record",
 					   sizeof(struct jbd2_revoke_record_s),
 					   0,
 					   SLAB_HWCACHE_ALIGN|SLAB_TEMPORARY,
 					   NULL);
-	if (jbd2_revoke_record_cache == 0)
-		return -ENOMEM;
-
 	jbd2_revoke_table_cache = kmem_cache_create("jbd2_revoke_table",
 					   sizeof(struct jbd2_revoke_table_s),
 					   0, SLAB_TEMPORARY, NULL);
-	if (jbd2_revoke_table_cache == 0) {
-		kmem_cache_destroy(jbd2_revoke_record_cache);
-		jbd2_revoke_record_cache = NULL;
+
+	if (jbd2_revoke_table_cache && jbd2_revoke_table_cache) {
+		return 0;
+	} else {
+		jbd2_journal_destroy_revoke_caches();
 		return -ENOMEM;
 	}
-	return 0;
-}
-
-void jbd2_journal_destroy_revoke_caches(void)
-{
-	kmem_cache_destroy(jbd2_revoke_record_cache);
-	jbd2_revoke_record_cache = NULL;
-	kmem_cache_destroy(jbd2_revoke_table_cache);
-	jbd2_revoke_table_cache = NULL;
 }
 
 static struct jbd2_revoke_table_s *jbd2_journal_init_revoke_table(int hash_size)
-- 
1.5.3.7

[PATCH] jbd: handle cache initialisation failure gracefully when statically compiled in

[Duane Griffin]

When jbd is statically compiled into the kernel we need to check whether the
caches are properly initialised before using them. If they are not then have
another go before failing gracefully.

Signed-off-by: Duane Griffin <duaneg@dghda.com>
---
 fs/jbd/journal.c |  149 ++++++++++++++++++++++++++----------------------------
 fs/jbd/revoke.c  |    2 +-
 2 files changed, 73 insertions(+), 78 deletions(-)

diff --git a/fs/jbd/journal.c b/fs/jbd/journal.c
index 3943a89..884e4ee 100644
--- a/fs/jbd/journal.c
+++ b/fs/jbd/journal.c
@@ -642,6 +642,67 @@ struct journal_head *journal_get_descriptor_buffer(journal_t *journal)
 }
 
 /*
+ * Journal_head storage management
+ */
+static struct kmem_cache *journal_head_cache;
+#ifdef CONFIG_JBD_DEBUG
+static atomic_t nr_journal_heads = ATOMIC_INIT(0);
+#endif
+
+struct kmem_cache *jbd_handle_cache;
+
+static void journal_destroy_caches(void)
+{
+	journal_destroy_revoke_caches();
+
+	if (journal_head_cache) {
+		kmem_cache_destroy(journal_head_cache);
+		journal_head_cache = NULL;
+	}
+
+	if (jbd_handle_cache) {
+		kmem_cache_destroy(jbd_handle_cache);
+		jbd_handle_cache = NULL;
+	}
+}
+
+static int journal_init_caches(void)
+{
+	int ret;
+
+	J_ASSERT(!journal_head_cache);
+	J_ASSERT(!jbd_handle_cache);
+
+	ret = journal_init_revoke_caches();
+	if (ret)
+		goto fail;
+
+	ret = -ENOMEM;
+	journal_head_cache = kmem_cache_create("journal_head",
+				sizeof(struct journal_head),
+				0,		/* offset */
+				SLAB_TEMPORARY,	/* flags */
+				NULL);		/* ctor */
+	if (!journal_head_cache)
+		goto fail;
+
+	jbd_handle_cache = kmem_cache_create("journal_handle",
+				sizeof(handle_t),
+				0,		/* offset */
+				SLAB_TEMPORARY,	/* flags */
+				NULL);		/* ctor */
+	if (!jbd_handle_cache)
+		goto fail;
+
+	return 0;
+
+fail:
+	journal_destroy_caches();
+	printk(KERN_EMERG "JBD: no memory to initialise cache\n");
+	return ret;
+}
+
+/*
  * Management for journal control blocks: functions to create and
  * destroy journal_t structures, and to initialise and read existing
  * journal blocks from disk.  */
@@ -653,7 +714,14 @@ struct journal_head *journal_get_descriptor_buffer(journal_t *journal)
 static journal_t * journal_init_common (void)
 {
 	journal_t *journal;
-	int err;
+	int err = 0;
+
+	if (!journal_head_cache)
+		err = journal_init_caches();
+	if (err) {
+		journal_destroy_caches();
+		return NULL;
+	}
 
 	journal = kzalloc(sizeof(*journal), GFP_KERNEL);
 	if (!journal)
@@ -1608,39 +1676,6 @@ int journal_blocks_per_page(struct inode *inode)
 }
 
 /*
- * Journal_head storage management
- */
-static struct kmem_cache *journal_head_cache;
-#ifdef CONFIG_JBD_DEBUG
-static atomic_t nr_journal_heads = ATOMIC_INIT(0);
-#endif
-
-static int journal_init_journal_head_cache(void)
-{
-	int retval;
-
-	J_ASSERT(journal_head_cache == 0);
-	journal_head_cache = kmem_cache_create("journal_head",
-				sizeof(struct journal_head),
-				0,		/* offset */
-				SLAB_TEMPORARY,	/* flags */
-				NULL);		/* ctor */
-	retval = 0;
-	if (journal_head_cache == 0) {
-		retval = -ENOMEM;
-		printk(KERN_EMERG "JBD: no memory for journal_head cache\n");
-	}
-	return retval;
-}
-
-static void journal_destroy_journal_head_cache(void)
-{
-	J_ASSERT(journal_head_cache != NULL);
-	kmem_cache_destroy(journal_head_cache);
-	journal_head_cache = NULL;
-}
-
-/*
  * journal_head splicing and dicing
  */
 static struct journal_head *journal_alloc_journal_head(void)
@@ -1889,51 +1924,10 @@ static inline void jbd_remove_debugfs_entry(void)
 
 #endif
 
-struct kmem_cache *jbd_handle_cache;
-
-static int __init journal_init_handle_cache(void)
-{
-	jbd_handle_cache = kmem_cache_create("journal_handle",
-				sizeof(handle_t),
-				0,		/* offset */
-				SLAB_TEMPORARY,	/* flags */
-				NULL);		/* ctor */
-	if (jbd_handle_cache == NULL) {
-		printk(KERN_EMERG "JBD: failed to create handle cache\n");
-		return -ENOMEM;
-	}
-	return 0;
-}
-
-static void journal_destroy_handle_cache(void)
-{
-	if (jbd_handle_cache)
-		kmem_cache_destroy(jbd_handle_cache);
-}
-
 /*
  * Module startup and shutdown
  */
 
-static int __init journal_init_caches(void)
-{
-	int ret;
-
-	ret = journal_init_revoke_caches();
-	if (ret == 0)
-		ret = journal_init_journal_head_cache();
-	if (ret == 0)
-		ret = journal_init_handle_cache();
-	return ret;
-}
-
-static void journal_destroy_caches(void)
-{
-	journal_destroy_revoke_caches();
-	journal_destroy_journal_head_cache();
-	journal_destroy_handle_cache();
-}
-
 static int __init journal_init(void)
 {
 	int ret;
@@ -1941,9 +1935,10 @@ static int __init journal_init(void)
 	BUILD_BUG_ON(sizeof(struct journal_superblock_s) != 1024);
 
 	ret = journal_init_caches();
-	if (ret != 0)
+	if (ret == 0)
+		jbd_create_debugfs_entry();
+	else
 		journal_destroy_caches();
-	jbd_create_debugfs_entry();
 	return ret;
 }
 
diff --git a/fs/jbd/revoke.c b/fs/jbd/revoke.c
index c1fcd62..b586801 100644
--- a/fs/jbd/revoke.c
+++ b/fs/jbd/revoke.c
@@ -178,7 +178,7 @@ void journal_destroy_revoke_caches(void)
 	}
 }
 
-int __init journal_init_revoke_caches(void)
+int journal_init_revoke_caches(void)
 {
 	J_ASSERT(!revoke_record_cache);
 	J_ASSERT(!revoke_table_cache);
-- 
1.5.3.7

[PATCH] jbd2: handle cache initialisation failure gracefully when statically compiled in

[Duane Griffin]

When jbd2 is statically compiled into the kernel we need to check whether the
caches are properly initialised before using them. If they are not then have
another go before failing gracefully.

Signed-off-by: Duane Griffin <duaneg@dghda.com>
---
 fs/jbd2/journal.c |  154 ++++++++++++++++++++++++++---------------------------
 fs/jbd2/revoke.c  |    2 +-
 2 files changed, 76 insertions(+), 80 deletions(-)

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 96ba846..d4cc95a 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -641,6 +641,67 @@ struct journal_head *jbd2_journal_get_descriptor_buffer(journal_t *journal)
 	return jbd2_journal_add_journal_head(bh);
 }
 
+/*
+ * Journal_head storage management
+ */
+static struct kmem_cache *jbd2_journal_head_cache;
+#ifdef CONFIG_JBD2_DEBUG
+static atomic_t nr_journal_heads = ATOMIC_INIT(0);
+#endif
+
+struct kmem_cache *jbd2_handle_cache;
+
+static void jbd2_journal_destroy_caches(void)
+{
+	jbd2_journal_destroy_revoke_caches();
+
+	if (jbd2_journal_head_cache) {
+		kmem_cache_destroy(jbd2_journal_head_cache);
+		jbd2_journal_head_cache = NULL;
+	}
+
+	if (jbd2_handle_cache) {
+		kmem_cache_destroy(jbd2_handle_cache);
+		jbd2_handle_cache = NULL;
+	}
+}
+
+static int jbd2_journal_init_caches(void)
+{
+	int ret;
+
+	J_ASSERT(!jbd2_journal_head_cache);
+	J_ASSERT(!jbd2_handle_cache);
+
+	ret = jbd2_journal_init_revoke_caches();
+	if (ret)
+		goto fail;
+
+	ret = -ENOMEM;
+	jbd2_journal_head_cache = kmem_cache_create("jbd2_journal_head",
+				sizeof(struct journal_head),
+				0,		/* offset */
+				SLAB_TEMPORARY,	/* flags */
+				NULL);		/* ctor */
+	if (!jbd2_journal_head_cache)
+		goto fail;
+
+	jbd2_handle_cache = kmem_cache_create("jbd2_journal_handle",
+				sizeof(handle_t),
+				0,		/* offset */
+				SLAB_TEMPORARY,	/* flags */
+				NULL);		/* ctor */
+	if (!jbd2_handle_cache)
+		goto fail;
+
+	return 0;
+
+fail:
+	jbd2_journal_destroy_caches();
+	printk(KERN_EMERG "JBD2: no memory to initialise cache\n");
+	return ret;
+}
+
 struct jbd2_stats_proc_session {
 	journal_t *journal;
 	struct transaction_stats_s *stats;
@@ -959,7 +1020,14 @@ static void journal_init_stats(journal_t *journal)
 static journal_t * journal_init_common (void)
 {
 	journal_t *journal;
-	int err;
+	int err = 0;
+
+	if (!jbd2_journal_head_cache)
+		err = jbd2_journal_init_caches();
+	if (err) {
+		jbd2_journal_destroy_caches();
+		return NULL;
+	}
 
 	journal = kzalloc(sizeof(*journal), GFP_KERNEL|__GFP_NOFAIL);
 	if (!journal)
@@ -1958,39 +2026,6 @@ size_t journal_tag_bytes(journal_t *journal)
 }
 
 /*
- * Journal_head storage management
- */
-static struct kmem_cache *jbd2_journal_head_cache;
-#ifdef CONFIG_JBD2_DEBUG
-static atomic_t nr_journal_heads = ATOMIC_INIT(0);
-#endif
-
-static int journal_init_jbd2_journal_head_cache(void)
-{
-	int retval;
-
-	J_ASSERT(jbd2_journal_head_cache == 0);
-	jbd2_journal_head_cache = kmem_cache_create("jbd2_journal_head",
-				sizeof(struct journal_head),
-				0,		/* offset */
-				SLAB_TEMPORARY,	/* flags */
-				NULL);		/* ctor */
-	retval = 0;
-	if (jbd2_journal_head_cache == 0) {
-		retval = -ENOMEM;
-		printk(KERN_EMERG "JBD: no memory for journal_head cache\n");
-	}
-	return retval;
-}
-
-static void jbd2_journal_destroy_jbd2_journal_head_cache(void)
-{
-	J_ASSERT(jbd2_journal_head_cache != NULL);
-	kmem_cache_destroy(jbd2_journal_head_cache);
-	jbd2_journal_head_cache = NULL;
-}
-
-/*
  * journal_head splicing and dicing
  */
 static struct journal_head *journal_alloc_journal_head(void)
@@ -2262,62 +2297,23 @@ static void __exit jbd2_remove_jbd_stats_proc_entry(void)
 
 #endif
 
-struct kmem_cache *jbd2_handle_cache;
-
-static int __init journal_init_handle_cache(void)
-{
-	jbd2_handle_cache = kmem_cache_create("jbd2_journal_handle",
-				sizeof(handle_t),
-				0,		/* offset */
-				SLAB_TEMPORARY,	/* flags */
-				NULL);		/* ctor */
-	if (jbd2_handle_cache == NULL) {
-		printk(KERN_EMERG "JBD: failed to create handle cache\n");
-		return -ENOMEM;
-	}
-	return 0;
-}
-
-static void jbd2_journal_destroy_handle_cache(void)
-{
-	if (jbd2_handle_cache)
-		kmem_cache_destroy(jbd2_handle_cache);
-}
-
 /*
  * Module startup and shutdown
  */
 
-static int __init journal_init_caches(void)
-{
-	int ret;
-
-	ret = jbd2_journal_init_revoke_caches();
-	if (ret == 0)
-		ret = journal_init_jbd2_journal_head_cache();
-	if (ret == 0)
-		ret = journal_init_handle_cache();
-	return ret;
-}
-
-static void jbd2_journal_destroy_caches(void)
-{
-	jbd2_journal_destroy_revoke_caches();
-	jbd2_journal_destroy_jbd2_journal_head_cache();
-	jbd2_journal_destroy_handle_cache();
-}
-
 static int __init journal_init(void)
 {
 	int ret;
 
 	BUILD_BUG_ON(sizeof(struct journal_superblock_s) != 1024);
 
-	ret = journal_init_caches();
-	if (ret != 0)
+	ret = jbd2_journal_init_caches();
+	if (ret == 0) {
+		jbd2_create_debugfs_entry();
+		jbd2_create_jbd_stats_proc_entry();
+	} else {
 		jbd2_journal_destroy_caches();
-	jbd2_create_debugfs_entry();
-	jbd2_create_jbd_stats_proc_entry();
+	}
 	return ret;
 }
 
diff --git a/fs/jbd2/revoke.c b/fs/jbd2/revoke.c
index 5a020f1..da4551e 100644
--- a/fs/jbd2/revoke.c
+++ b/fs/jbd2/revoke.c
@@ -179,7 +179,7 @@ void jbd2_journal_destroy_revoke_caches(void)
 	}
 }
 
-int __init jbd2_journal_init_revoke_caches(void)
+int jbd2_journal_init_revoke_caches(void)
 {
 	J_ASSERT(!jbd2_revoke_record_cache);
 	J_ASSERT(!jbd2_revoke_table_cache);
-- 
1.5.3.7

Re: jbd/2: eliminate code duplication and gracefully handle cache initialisation failure

[Andrew Morton]

On Sat,  8 Mar 2008 18:37:33 +0000
"Duane Griffin" <duaneg@dghda.com> wrote:

>  fs/jbd/journal.c  |  149 +++++++++++++++++++++++-------------------------
>  fs/jbd/revoke.c   |  164 +++++++++++++++++++++++------------------------------
>  fs/jbd2/journal.c |  154 ++++++++++++++++++++++++-------------------------
>  fs/jbd2/revoke.c  |  164 +++++++++++++++++++++++------------------------------
>  4 files changed, 291 insertions(+), 340 deletions(-)
> 
> This is version 2 of the jdb/2 patches to eliminate code duplication and
> gracefully handle cace initialisation failure.
> 

I couldn't get these to apply and I didn't understand the relationship
between these new patches and your earlier
jbd[2]-replace-potentially-false-assertion-with-if-block.patch and
jbd[2]-only-create-debugfs-entries-if-cache-initialisation-is-successful.patch
so I just dropped everything.

Please redo and resend against next -mm or
http://userweb.kernel.org/~akpm/mmotm/, thanks.

Re: jbd/2: eliminate code duplication and gracefully handle cache initialisation failure

[Duane Griffin]

On Mon, Mar 10, 2008 at 12:46:28PM -0700, Andrew Morton wrote:
> > This is version 2 of the jdb/2 patches to eliminate code duplication and
> > gracefully handle cace initialisation failure.
> > 
> 
> I couldn't get these to apply and I didn't understand the relationship
> between these new patches and your earlier
> jbd[2]-replace-potentially-false-assertion-with-if-block.patch and
> jbd[2]-only-create-debugfs-entries-if-cache-initialisation-is-successful.patch
> so I just dropped everything.
> 
> Please redo and resend against next -mm or
> http://userweb.kernel.org/~akpm/mmotm/, thanks.

Will do, sorry for the inconvenience.

Cheers,
Duane.

-- 
"I never could learn to drink that blood and call it wine" - Bob Dylan