[RFC, PATCH] Make EXT2_DEBUG work again

Valerie Henson <[email protected]>
Newsgroups gmane.comp.file-systems.ext2.devel
Message-ID <20060601040406.GN10420@goober>
The unfortunate result of an earlier clean-up patch combined with
defining EXT2_DEBUG:

fs/built-in.o: In function `ext2_count_free_blocks':
fs/ext2/balloc.c:548: undefined reference to `ext2_count_free'
fs/built-in.o: In function `ext2_count_free_inodes':
fs/ext2/ialloc.c:666: undefined reference to `ext2_count_free'
collect2: ld returned 1 exit status

The problem is that in bitmap.c, we ask if EXT2_DEBUG is defined -
without including the header file in which EXT2_DEBUG is defined.
Since ext2_count_free() is the only thing bitmap.c contains, I moved
it into balloc.c and deleted bitmap.c entirely.

Excitingly, it looks like ext3 has the exact same problem.  I'll fix
it once I get consensus on what the ext2 patch should look like.

Second, when debugging is enabled, ext2_count_free_blocks/inodes
attempts to reacquire the superblock lock (helpfully caught by
spinlock debugging).  I moved the lock into the caller, since most
callers already held it or did not need it (e.g., ext2_fill_super).
The only nasty bit is in ext2_statfs, in which I decided to grab the
superblock lock for the whole function.  It can be narrowed down just
to cover the two instances of ext2_count_free_*, but it really seems
to me like it should be held the entire time.  Unless, that is, we are
more worried about the costs of grabbing the lock than getting
consistent data back from statvfs().

Comments?

-VAL

This patch makes EXT2_DEBUG work again.  Due to lack of proper include
file, EXT2_DEBUG was undefined in bitmap.c and ext2_count_free() is
left out.  Moved to balloc.c and removed bitmap.c entirely.

Second, debug versions of ext2_count_free_{inodes/blocks} reacquires
superblock lock.  Moved lock into callers.

Signed-off-by: Val Henson <[email protected]>

 Makefile |    2 +-
 balloc.c |   22 ++++++++++++++++++++--
 bitmap.c |   32 --------------------------------
 ialloc.c |    3 +--
 super.c  |    2 ++
 5 files changed, 24 insertions(+), 37 deletions(-)
diff -x '*~' -puNr linux-2.6.16.19/fs/ext2/balloc.c tmp_linux/fs/ext2/balloc.c
--- linux-2.6.16.19/fs/ext2/balloc.c	2006-05-30 17:31:44.000000000 -0700
+++ tmp_linux/fs/ext2/balloc.c	2006-05-31 19:45:16.000000000 -0700
@@ -521,6 +521,26 @@ io_error:
 	goto out_release;
 }
 
+#ifdef EXT2FS_DEBUG
+
+static int nibblemap[] = {4, 3, 3, 2, 3, 2, 2, 1, 3, 2, 2, 1, 2, 1, 1, 0};
+
+unsigned long ext2_count_free (struct buffer_head * map, unsigned int numchars)
+{
+	unsigned int i;
+	unsigned long sum = 0;
+	
+	if (!map) 
+		return (0);
+	for (i = 0; i < numchars; i++)
+		sum += nibblemap[map->b_data[i] & 0xf] +
+			nibblemap[(map->b_data[i] >> 4) & 0xf];
+	return (sum);
+}
+
+#endif  /*  EXT2FS_DEBUG  */
+
+/* Superblock must be locked */
 unsigned long ext2_count_free_blocks (struct super_block * sb)
 {
 	struct ext2_group_desc * desc;
@@ -530,7 +550,6 @@ unsigned long ext2_count_free_blocks (st
 	unsigned long bitmap_count, x;
 	struct ext2_super_block *es;
 
-	lock_super (sb);
 	es = EXT2_SB(sb)->s_es;
 	desc_count = 0;
 	bitmap_count = 0;
@@ -554,7 +573,6 @@ unsigned long ext2_count_free_blocks (st
 	printk("ext2_count_free_blocks: stored = %lu, computed = %lu, %lu\n",
 		(long)le32_to_cpu(es->s_free_blocks_count),
 		desc_count, bitmap_count);
-	unlock_super (sb);
 	return bitmap_count;
 #else
         for (i = 0; i < EXT2_SB(sb)->s_groups_count; i++) {
diff -x '*~' -puNr linux-2.6.16.19/fs/ext2/bitmap.c tmp_linux/fs/ext2/bitmap.c
--- linux-2.6.16.19/fs/ext2/bitmap.c	2006-05-30 17:31:44.000000000 -0700
+++ tmp_linux/fs/ext2/bitmap.c	1969-12-31 16:00:00.000000000 -0800
@@ -1,32 +0,0 @@
-/*
- *  linux/fs/ext2/bitmap.c
- *
- * Copyright (C) 1992, 1993, 1994, 1995
- * Remy Card ([email protected])
- * Laboratoire MASI - Institut Blaise Pascal
- * Universite Pierre et Marie Curie (Paris VI)
- */
-
-#ifdef EXT2FS_DEBUG
-
-#include <linux/buffer_head.h>
-
-#include "ext2.h"
-
-static int nibblemap[] = {4, 3, 3, 2, 3, 2, 2, 1, 3, 2, 2, 1, 2, 1, 1, 0};
-
-unsigned long ext2_count_free (struct buffer_head * map, unsigned int numchars)
-{
-	unsigned int i;
-	unsigned long sum = 0;
-	
-	if (!map) 
-		return (0);
-	for (i = 0; i < numchars; i++)
-		sum += nibblemap[map->b_data[i] & 0xf] +
-			nibblemap[(map->b_data[i] >> 4) & 0xf];
-	return (sum);
-}
-
-#endif  /*  EXT2FS_DEBUG  */
-
diff -x '*~' -puNr linux-2.6.16.19/fs/ext2/ialloc.c tmp_linux/fs/ext2/ialloc.c
--- linux-2.6.16.19/fs/ext2/ialloc.c	2006-05-30 17:31:44.000000000 -0700
+++ tmp_linux/fs/ext2/ialloc.c	2006-05-31 15:27:24.000000000 -0700
@@ -638,6 +638,7 @@ fail:
 	return ERR_PTR(err);
 }
 
+/* Superblock must be locked */
 unsigned long ext2_count_free_inodes (struct super_block * sb)
 {
 	struct ext2_group_desc *desc;
@@ -649,7 +650,6 @@ unsigned long ext2_count_free_inodes (st
 	unsigned long bitmap_count = 0;
 	struct buffer_head *bitmap_bh = NULL;
 
-	lock_super (sb);
 	es = EXT2_SB(sb)->s_es;
 	for (i = 0; i < EXT2_SB(sb)->s_groups_count; i++) {
 		unsigned x;
@@ -672,7 +672,6 @@ unsigned long ext2_count_free_inodes (st
 	printk("ext2_count_free_inodes: stored = %lu, computed = %lu, %lu\n",
 		percpu_counter_read(&EXT2_SB(sb)->s_freeinodes_counter),
 		desc_count, bitmap_count);
-	unlock_super(sb);
 	return desc_count;
 #else
 	for (i = 0; i < EXT2_SB(sb)->s_groups_count; i++) {
diff -x '*~' -puNr linux-2.6.16.19/fs/ext2/Makefile tmp_linux/fs/ext2/Makefile
--- linux-2.6.16.19/fs/ext2/Makefile	2006-05-31 19:46:06.000000000 -0700
+++ tmp_linux/fs/ext2/Makefile	2006-05-31 19:46:18.000000000 -0700
@@ -4,7 +4,7 @@
 
 obj-$(CONFIG_EXT2_FS) += ext2.o
 
-ext2-y := balloc.o bitmap.o dir.o file.o fsync.o ialloc.o inode.o \
+ext2-y := balloc.o dir.o file.o fsync.o ialloc.o inode.o \
 	  ioctl.o namei.o super.o symlink.o
 
 ext2-$(CONFIG_EXT2_FS_XATTR)	 += xattr.o xattr_user.o xattr_trusted.o
diff -x '*~' -puNr linux-2.6.16.19/fs/ext2/super.c tmp_linux/fs/ext2/super.c
--- linux-2.6.16.19/fs/ext2/super.c	2006-05-30 17:31:44.000000000 -0700
+++ tmp_linux/fs/ext2/super.c	2006-05-31 16:07:49.000000000 -0700
@@ -1046,6 +1046,7 @@ static int ext2_statfs (struct super_blo
 	unsigned long overhead;
 	int i;
 
+	lock_super(sb);
 	if (test_opt (sb, MINIX_DF))
 		overhead = 0;
 	else {
@@ -1086,6 +1087,7 @@ static int ext2_statfs (struct super_blo
 	buf->f_files = le32_to_cpu(sbi->s_es->s_inodes_count);
 	buf->f_ffree = ext2_count_free_inodes (sb);
 	buf->f_namelen = EXT2_NAME_LEN;
+	unlock_super(sb);
 	return 0;
 }
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.