Re: [PATCH 2/3] jbd2: replace potentially false assertion with ifblock

[Duane Griffin]

On Fri, Mar 07, 2008 at 02:23:36PM -0700, Andreas Dilger wrote:
> On Mar 07, 2008  01:31 +0000, Duane Griffin wrote:
> > If an error occurs during jbd2 cache initialisation it is possible for the
> > journal_head_cache to be NULL when jbd2_journal_destroy_journal_head_cache is
> > called. Replace the J_ASSERT with an if block to handle the situation
> > correctly.
> > 
> > Note that even with this fix things will break badly if jbd2 is statically
> > compiled in and cache initialisation fails.
> 
> It would probably be prudent to verify that these caches are initialized
> at journal_load() time and either re-try to create the cache, and/or report
> an error in that case and refuse to continue mounting.

Sounds like a plan. I'll see what I can whip up.

> Also, I note that journal_init_journal_head_cache() is comparing pointers
> to "0", a style no-no...

Yes, checkpatch noticed that, too. It was in the original, so I left it
alone. There are also a couple of places in revoke.c where it is done.
See the patch below. If you like it I'll send through one for jbd too.

Cheers,
Duane.

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

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 9d48419..75349b1 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1969,14 +1969,14 @@ static int journal_init_jbd2_journal_head_cache(void)
 {
 	int retval;
 
-	J_ASSERT(jbd2_journal_head_cache == 0);
+	J_ASSERT(!jbd2_journal_head_cache);
 	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) {
+	if (!jbd2_journal_head_cache) {
 		retval = -ENOMEM;
 		printk(KERN_EMERG "JBD: no memory for journal_head cache\n");
 	}
diff --git a/fs/jbd2/revoke.c b/fs/jbd2/revoke.c
index 1bf4c1f..cae1172 100644
--- a/fs/jbd2/revoke.c
+++ b/fs/jbd2/revoke.c
@@ -174,13 +174,13 @@ int __init jbd2_journal_init_revoke_caches(void)
 					   0,
 					   SLAB_HWCACHE_ALIGN|SLAB_TEMPORARY,
 					   NULL);
-	if (jbd2_revoke_record_cache == 0)
+	if (!jbd2_revoke_record_cache)
 		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) {
+	if (!jbd2_revoke_table_cache) {
 		kmem_cache_destroy(jbd2_revoke_record_cache);
 		jbd2_revoke_record_cache = NULL;
 		return -ENOMEM;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/