Patchwork [RFC] mkjournal: zero journal blocks only when necessary

login
register
mail settings
Submitter Alexander Shishkin
Date July 29, 2009, 3:58 p.m.
Message ID <1248883096-2294-1-git-send-email-alexander.shishckin@gmail.com>
Download mbox | patch
Permalink /patch/30348/
State Rejected
Headers show

Comments

Alexander Shishkin - July 29, 2009, 3:58 p.m.
Will something like this do?

The only blocks that might theoretically (although very unlikely) be
dangerous for newly-created filesystem's integrity are those that
still contain valid signatures, others can be safely skipped.

Since reads are generally faster (or at least, not slower), this
gives some performance increase during mkfs run.

Signed-off-by: Alexander Shishkin <alexander.shishckin@gmail.com>
---
 lib/ext2fs/mkjournal.c |   49 ++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 43 insertions(+), 6 deletions(-)
Theodore Ts'o - July 29, 2009, 5:16 p.m.
On Wed, Jul 29, 2009 at 06:58:16PM +0300, Alexander Shishkin wrote:
> Will something like this do?
> 
> The only blocks that might theoretically (although very unlikely) be
> dangerous for newly-created filesystem's integrity are those that
> still contain valid signatures, others can be safely skipped.
> 
> Since reads are generally faster (or at least, not slower), this
> gives some performance increase during mkfs run.

Did you bother benchmarking what this would do on normal disk drives?
Previously we were writing out the blocks to be zeroed in large chunks
at a time for speed reasons.  This patch reduces it to reading the
journal one block at a time, and if it contains a valid signature it
writes a zero block.

The patch also doesn't check for commit blocks, which are just as much
a problem (if not more so) than revoke blocks.

						- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/lib/ext2fs/mkjournal.c b/lib/ext2fs/mkjournal.c
index f5a9dba..bb6e748 100644
--- a/lib/ext2fs/mkjournal.c
+++ b/lib/ext2fs/mkjournal.c
@@ -144,6 +144,44 @@  errout:
  * attempt to free the static zeroizing buffer.  (This is to keep
  * programs that check for memory leaks happy.)
  */
+static errcode_t jzero_blocks(io_channel channel, unsigned long long block,
+		int count)
+{
+	errcode_t retval;
+	unsigned long long n;
+	static void *__buf = NULL;
+	struct unix_private_data *data;
+
+	data = channel->private_data;
+	if (!__buf)
+		__buf = malloc(channel->block_size);
+	if (!__buf)
+		return -1;
+
+	for (n = block; n < block + count; n++) {
+		journal_header_t *jr;
+
+		retval = io_channel_read_blk(channel, n, 1, __buf);
+		if (retval)
+			goto out_err;
+		continue;
+
+		jr = __buf;
+		if (jr->h_magic == htonl(JFS_MAGIC_NUMBER) &&
+		    jr->h_blocktype == htonl(JFS_REVOKE_BLOCK)) {
+			memset(__buf, 0, channel->block_size);
+			retval = io_channel_write_blk(channel, n, 1, __buf);
+		}
+
+		if (retval)
+			goto out_err;
+	}
+
+	return 0;
+out_err:
+	return -1;
+}
+
 #define STRIDE_LENGTH 8
 errcode_t ext2fs_zero_blocks(ext2_filsys fs, blk_t blk, int num,
 			     blk_t *ret_blk, int *ret_count)
@@ -238,10 +276,9 @@  static int mkjournal_proc(ext2_filsys	fs,
 			    (es->zero_count < 1024))
 				es->zero_count++;
 			else {
-				retval = ext2fs_zero_blocks(fs,
-							    es->blk_to_zero,
-							    es->zero_count,
-							    0, 0);
+				retval = jzero_blocks(fs->io,
+						es->blk_to_zero,
+						es->zero_count);
 				es->zero_count = 0;
 			}
 		}
@@ -339,8 +376,8 @@  static errcode_t write_journal_inode(ext2_filsys fs, ext2_ino_t journal_ino,
 		goto errout;
 	}
 	if (es.zero_count) {
-		retval = ext2fs_zero_blocks(fs, es.blk_to_zero,
-					    es.zero_count, 0, 0);
+		retval = jzero_blocks(fs->io, es.blk_to_zero,
+					    es.zero_count);
 		if (retval)
 			goto errout;
 	}