Re: [PATCH 0/6 v4] Lazy itable initialization for Ext4

Previous thread: [PATCH 5/6] Use sb_issue_zeroout in ext4_ext_zeroout by Lukas Czerner on Thursday, September 16, 2010 - 5:47 am. (1 message)

Next thread: [RESEND][PATCH] ext4: create own llseek function to handle 2 maxbytes by Toshiyuki Okajima on Thursday, September 16, 2010 - 10:32 pm. (2 messages)
From: Lukas Czerner
Date: Thursday, September 16, 2010 - 5:47 am

Hi,

as Mike suggested I have rebased the patch #1 against Jens'
linux-2.6-block.git 'for-next' branch and changed sb_issue_zeroout()
to cope with the new blkdev_issue_zeroout(), and changed
sb_issue_zeroout() to the new syntax everywhere I am using it.
Also some typos gets fixed.

-Lukas


--

From: Lukas Czerner
Date: Thursday, September 16, 2010 - 5:47 am

This is done the same way as helper sb_issue_discard for
blkdev_issue_discard.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 include/linux/blkdev.h |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index e7ddd6b..e37e82d 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -895,6 +895,13 @@ static inline int sb_issue_discard(struct super_block *sb, sector_t block,
 				    nr_blocks << (sb->s_blocksize_bits - 9),
 				    gfp_mask, flags);
 }
+static inline int sb_issue_zeroout(struct super_block *sb, sector_t block,
+		sector_t nr_blocks, gfp_t gfp_mask, unsigned long flags)
+{
+	return blkdev_issue_zeroout(sb->s_bdev, block << (sb->s_blocksize_bits - 9),
+				    nr_blocks << (sb->s_blocksize_bits - 9),
+				    gfp_mask, flags);
+}
 
 extern int blk_verify_command(unsigned char *cmd, fmode_t has_write_perm);
 
-- 
1.7.2.2

--

From: Lukas Czerner
Date: Thursday, September 16, 2010 - 5:47 am

Add new mount flag EXT4_MOUNT_INIT_INODE_TABLE and add new pair of mount
options (inititable/noinititable). When mounted with inititable file
system should try to initialize uninitialized inode tables, otherwise it
should prevent initializing inode tables. For now, default is noinittable.

One can also specify inititable=n where n is a number that will be used
as the wait multiplier (see "Add inode table initialization code into
Ext4" patch for more info). Bigger number means slower inode table
initialization thus less impact on performance, but longer
inititalization (default is 10).

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 fs/ext4/ext4.h  |    1 +
 fs/ext4/super.c |   22 ++++++++++++++++++++++
 2 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 889ec9d..9600897 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -889,6 +889,7 @@ struct ext4_inode_info {
 #define EXT4_MOUNT_DATA_ERR_ABORT	0x10000000 /* Abort on file data write */
 #define EXT4_MOUNT_BLOCK_VALIDITY	0x20000000 /* Block validity checking */
 #define EXT4_MOUNT_DISCARD		0x40000000 /* Issue DISCARD requests */
+#define EXT4_MOUNT_INIT_INODE_TABLE	0x80000000 /* Initialize uninitialized itables */
 
 #define clear_opt(o, opt)		o &= ~EXT4_MOUNT_##opt
 #define set_opt(o, opt)			o |= EXT4_MOUNT_##opt
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 2614774..c15e84d 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1045,6 +1045,10 @@ static int ext4_show_options(struct seq_file *seq, struct vfsmount *vfs)
 	    !(def_mount_opts & EXT4_DEFM_BLOCK_VALIDITY))
 		seq_puts(seq, ",block_validity");
 
+	if (test_opt(sb, INIT_INODE_TABLE))
+		seq_printf(seq, ",init_inode_table=%u",
+			   (unsigned) sbi->s_li_wait_mult);
+
 	ext4_show_quota_options(seq, sb);
 
 	return 0;
@@ -1219,6 +1223,7 @@ enum {
 	Opt_inode_readahead_blks, Opt_journal_ioprio,
 	Opt_dioread_nolock, Opt_dioread_lock,
 	Opt_discard, ...
From: Ted Ts'o
Date: Monday, September 27, 2010 - 11:35 am

Note: this patch doesn't compile on its own, since it uses s_li_wait
before it is defined (in the next patch).

This is a problem if someone tries to do a "git bisect" and lands
between these two patches.

I'll probably fix this just by merging these two patches together...

     	      	       	       	       - Ted
--

From: Lukas Czerner
Date: Thursday, September 16, 2010 - 5:47 am

Use sb_issue_zeroout to zero out inode table and descriptor table
blocks instead of old approach which involves journaling.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 fs/ext4/resize.c |   46 +++++++++++++---------------------------------
 1 files changed, 13 insertions(+), 33 deletions(-)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index ca5c8aa..afba286 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -226,23 +226,13 @@ static int setup_new_group_blocks(struct super_block *sb,
 	}
 
 	/* Zero out all of the reserved backup group descriptor table blocks */
-	for (i = 0, bit = gdblocks + 1, block = start + bit;
-	     i < reserved_gdb; i++, block++, bit++) {
-		struct buffer_head *gdb;
-
-		ext4_debug("clear reserved block %#04llx (+%d)\n", block, bit);
-
-		if ((err = extend_or_restart_transaction(handle, 1, bh)))
-			goto exit_bh;
+	ext4_debug("clear inode table blocks %#04llx -> %#04llx\n",
+			block, sbi->s_itb_per_group);
+	err = sb_issue_zeroout(sb, gdblocks + start + 1, reserved_gdb,
+			       GFP_NOFS, BLKDEV_IFL_WAIT);
+	if (err)
+		goto exit_journal;
 
-		if (IS_ERR(gdb = bclean(handle, sb, block))) {
-			err = PTR_ERR(gdb);
-			goto exit_bh;
-		}
-		ext4_handle_dirty_metadata(handle, NULL, gdb);
-		ext4_set_bit(bit, bh->b_data);
-		brelse(gdb);
-	}
 	ext4_debug("mark block bitmap %#04llx (+%llu)\n", input->block_bitmap,
 		   input->block_bitmap - start);
 	ext4_set_bit(input->block_bitmap - start, bh->b_data);
@@ -251,23 +241,13 @@ static int setup_new_group_blocks(struct super_block *sb,
 	ext4_set_bit(input->inode_bitmap - start, bh->b_data);
 
 	/* Zero out all of the inode table blocks */
-	for (i = 0, block = input->inode_table, bit = block - start;
-	     i < sbi->s_itb_per_group; i++, bit++, block++) {
-		struct buffer_head *it;
-
-		ext4_debug("clear inode block %#04llx (+%d)\n", block, bit);
-
-		if ((err = extend_or_restart_transaction(handle, 1, bh)))
-			goto exit_bh;
-
-		if (IS_ERR(it = bclean(handle, ...
From: Lukas Czerner
Date: Wednesday, September 29, 2010 - 7:12 am

When I look at this now, it seems it is bad, because when
sb_issue_discard() returns error for some reason we end up with not
released buffer_head. Since I am still not able to reproduce Ted's


I'll resend the patch shortly.

Thanks!
-Lukas
--

From: Lukas Czerner
Date: Wednesday, September 29, 2010 - 7:14 am

Use sb_issue_zeroout to zero out inode table and descriptor table
blocks instead of old approach which involves journaling.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 fs/ext4/resize.c |   46 +++++++++++++---------------------------------
 1 files changed, 13 insertions(+), 33 deletions(-)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index ca5c8aa..2f5e347 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -226,23 +226,13 @@ static int setup_new_group_blocks(struct super_block *sb,
 	}
 
 	/* Zero out all of the reserved backup group descriptor table blocks */
-	for (i = 0, bit = gdblocks + 1, block = start + bit;
-	     i < reserved_gdb; i++, block++, bit++) {
-		struct buffer_head *gdb;
-
-		ext4_debug("clear reserved block %#04llx (+%d)\n", block, bit);
-
-		if ((err = extend_or_restart_transaction(handle, 1, bh)))
-			goto exit_bh;
+	ext4_debug("clear inode table blocks %#04llx -> %#04llx\n",
+			block, sbi->s_itb_per_group);
+	err = sb_issue_zeroout(sb, gdblocks + start + 1, reserved_gdb,
+			       GFP_NOFS, BLKDEV_IFL_WAIT);
+	if (err)
+		goto exit_bh;
 
-		if (IS_ERR(gdb = bclean(handle, sb, block))) {
-			err = PTR_ERR(gdb);
-			goto exit_bh;
-		}
-		ext4_handle_dirty_metadata(handle, NULL, gdb);
-		ext4_set_bit(bit, bh->b_data);
-		brelse(gdb);
-	}
 	ext4_debug("mark block bitmap %#04llx (+%llu)\n", input->block_bitmap,
 		   input->block_bitmap - start);
 	ext4_set_bit(input->block_bitmap - start, bh->b_data);
@@ -251,23 +241,13 @@ static int setup_new_group_blocks(struct super_block *sb,
 	ext4_set_bit(input->inode_bitmap - start, bh->b_data);
 
 	/* Zero out all of the inode table blocks */
-	for (i = 0, block = input->inode_table, bit = block - start;
-	     i < sbi->s_itb_per_group; i++, bit++, block++) {
-		struct buffer_head *it;
-
-		ext4_debug("clear inode block %#04llx (+%d)\n", block, bit);
-
-		if ((err = extend_or_restart_transaction(handle, 1, bh)))
-			goto exit_bh;
-
-		if (IS_ERR(it = bclean(handle, sb, ...
From: Lukas Czerner
Date: Friday, October 1, 2010 - 9:00 am

Use sb_issue_zeroout to zero out inode table and descriptor table
blocks instead of old approach which involves journaling.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 fs/ext4/resize.c |   47 +++++++++++++++--------------------------------
 1 files changed, 15 insertions(+), 32 deletions(-)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index ca5c8aa..49c8aff 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -226,23 +226,16 @@ static int setup_new_group_blocks(struct super_block *sb,
 	}
 
 	/* Zero out all of the reserved backup group descriptor table blocks */
-	for (i = 0, bit = gdblocks + 1, block = start + bit;
-	     i < reserved_gdb; i++, block++, bit++) {
-		struct buffer_head *gdb;
-
-		ext4_debug("clear reserved block %#04llx (+%d)\n", block, bit);
+	ext4_debug("clear inode table blocks %#04llx -> %#04llx\n",
+			block, sbi->s_itb_per_group);
+	err = sb_issue_zeroout(sb, gdblocks + start + 1, reserved_gdb,
+			       GFP_NOFS, BLKDEV_IFL_WAIT);
+	if (err)
+		goto exit_bh;
 
-		if ((err = extend_or_restart_transaction(handle, 1, bh)))
-			goto exit_bh;
+	if ((err = extend_or_restart_transaction(handle, 1, bh)))
+		goto exit_bh;
 
-		if (IS_ERR(gdb = bclean(handle, sb, block))) {
-			err = PTR_ERR(gdb);
-			goto exit_bh;
-		}
-		ext4_handle_dirty_metadata(handle, NULL, gdb);
-		ext4_set_bit(bit, bh->b_data);
-		brelse(gdb);
-	}
 	ext4_debug("mark block bitmap %#04llx (+%llu)\n", input->block_bitmap,
 		   input->block_bitmap - start);
 	ext4_set_bit(input->block_bitmap - start, bh->b_data);
@@ -251,23 +244,13 @@ static int setup_new_group_blocks(struct super_block *sb,
 	ext4_set_bit(input->inode_bitmap - start, bh->b_data);
 
 	/* Zero out all of the inode table blocks */
-	for (i = 0, block = input->inode_table, bit = block - start;
-	     i < sbi->s_itb_per_group; i++, bit++, block++) {
-		struct buffer_head *it;
-
-		ext4_debug("clear inode block %#04llx (+%d)\n", block, bit);
-
-		if ((err = ...
From: Lukas Czerner
Date: Thursday, September 16, 2010 - 5:47 am

User-space should have the opportunity to check what features doest ext4
support in each particular copy. This adds easy interface by creating new
"features" directory in sys/fs/ext4/. In that directory files
advertising feature names can be created.

Add lazy_itable_init to the feature list.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 fs/ext4/ext4.h  |    5 +++++
 fs/ext4/super.c |   50 +++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 54 insertions(+), 1 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 96884c5..74ec1fc 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1572,6 +1572,11 @@ struct ext4_li_request {
 	unsigned long		lr_next_sched;
 };
 
+struct ext4_features {
+	struct kobject f_kobj;
+	struct completion f_kobj_unregister;
+};
+
 /*
  * Function prototypes
  */
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 2b53a48..bb84c27 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -57,6 +57,7 @@ struct proc_dir_entry *ext4_proc_root;
 static struct kset *ext4_kset;
 struct ext4_lazy_init *ext4_li_info;
 struct mutex ext4_li_mtx;
+struct ext4_features *ext4_feat;
 
 static int ext4_load_journal(struct super_block *, struct ext4_super_block *,
 			     unsigned long journal_devnum);
@@ -2413,6 +2414,7 @@ static struct ext4_attr ext4_attr_##_name = {			\
 #define EXT4_ATTR(name, mode, show, store) \
 static struct ext4_attr ext4_attr_##name = __ATTR(name, mode, show, store)
 
+#define EXT4_INFO_ATTR(name) EXT4_ATTR(name, 0444, NULL, NULL)
 #define EXT4_RO_ATTR(name) EXT4_ATTR(name, 0444, name##_show, NULL)
 #define EXT4_RW_ATTR(name) EXT4_ATTR(name, 0644, name##_show, name##_store)
 #define EXT4_RW_ATTR_SBI_UI(name, elname)	\
@@ -2449,6 +2451,14 @@ static struct attribute *ext4_attrs[] = {
 	NULL,
 };
 
+/* Features this copy of ext4 supports */
+EXT4_INFO_ATTR(lazy_itable_init);
+
+static struct attribute *ext4_feat_attrs[] = {
+	ATTR_LIST(lazy_itable_init),
+	NULL,
+};
+
 static ...
From: Lukas Czerner
Date: Thursday, September 16, 2010 - 5:47 am

When lazy_itable_init extended option is passed to mke2fs, it
considerably speed up filesystem creation because inode tables are
not zeroed out, thus contains some old data. When this fs is mounted
filesystem code should initialize (zero out) inode tables.
So far this code was missing for ext4 and this patch adds this feature.

For purpose of zeroing inode tables it introduces new kernel thread
called ext4lazyinit, which is created on demand and destroyed, when it
is no longer needed. There is only one thread for all ext4
filesystems in the system. When the first filesystem with inititable
mount option is mounted, ext4lazyinit thread is created, then the
filesystem can register its request in the request list.

This thread then walks through the list of requests picking up scheduled
requests and invoking ext4_init_inode_table(). Next schedule time for
the request is computed by multiplying the time it took to zero out last
inode table with wait multiplier, which can be set with the
(inititable=n) mount option (default is 10). We are doing this so we do
not take the whole I/O bandwidth. When the thread is no longer
necessary (request list is empty) it frees the appropriate structures and
exits (and can be created later later by another filesystem).

We do not disturb regular inode allocations in any way, it just do not
care whether the inode table is, or is not zeroed. But when zeroing, we
have to skip used inodes, obviously. Also we should prevent new inode
allocations from the group, while zeroing is on the way. For that we
take write alloc_sem lock in ext4_init_inode_table() and read alloc_sem
in the ext4_claim_inode, so when we are unlucky and allocator hits the
group which is currently being zeroed, it just has to wait.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 fs/ext4/ext4.h   |   38 +++++
 fs/ext4/ialloc.c |  116 +++++++++++++++
 fs/ext4/super.c  |  415 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 566 insertions(+), 3 ...
From: Ted Ts'o
Date: Monday, September 27, 2010 - 9:01 pm

We may have a problem with the lazy_itable patches.  I've tried
running the XFSTESTS three times now.  This was with a system where
mke2fs was setup (via /etc/mke2fs.conf) to always format the file
system using lazy_itable_init.  This meant that any of the xfstests
which reformated the scratch partition and then started a stress test
would stress the newly added itable initialization code.
Unfortunately the results weren't good.

The first time, I got the following soft lockup warning:

[ 2520.528745] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[ 2520.531445]  ef2b8e44 00000046 00000007 e29c1500 e29c1500 e29c1760 e29c175c c0b55500
[ 2520.534983]  c0b55500 e29c175c c0b55500 c0b55500 c0b55500 32423426 00000224 00000000
[ 2520.538270]  00000224 e29c1500 00000001 ef205000 00000005 ef2b8e74 ef2b8e80 c026eb2c
[ 2520.541743] Call Trace:
[ 2520.542742]  [<c026eb2c>] jbd2_log_wait_commit+0x103/0x14f
[ 2520.544291]  [<c01711dc>] ? autoremove_wake_function+0x0/0x34
[ 2520.545816]  [<c026bf95>] jbd2_log_do_checkpoint+0x1a8/0x458
[ 2520.547431]  [<c026f4ed>] jbd2_journal_destroy+0x107/0x1d3
[ 2520.549602]  [<c01711dc>] ? autoremove_wake_function+0x0/0x34
[ 2520.551100]  [<c0252bef>] ext4_put_super+0x78/0x2f7
[ 2520.552798]  [<c01f3c3c>] generic_shutdown_super+0x47/0xb8
[ 2520.554692]  [<c01f3ccf>] kill_block_super+0x22/0x36
[ 2520.556470]  [<c01f3816>] deactivate_locked_super+0x22/0x3e
[ 2520.558372]  [<c01f3bf1>] deactivate_super+0x3d/0x41
[ 2520.560138]  [<c02057a9>] mntput_no_expire+0xb5/0xd8
[ 2520.561880]  [<c0206609>] sys_umount+0x273/0x298
[ 2520.563358]  [<c0206640>] sys_oldumount+0x12/0x14
[ 2520.564952]  [<c0646715>] syscall_call+0x7/0xb
[ 2520.566596] 3 locks held by umount/15126:
[ 2520.568121]  #0:  (&type->s_umount_key#20){++++..}, at: [<c01f3bea>] deactivate_super+0x36/0x41
[ 2520.571819]  #1:  (&type->s_lock_key#2){+.+...}, at: [<c01f3096>] lock_super+0x20/0x22
[ 2520.574788]  #2:  (&journal->j_checkpoint_mutex){+.+...}, at: [<c026f4e6>] ...
From: Ted Ts'o
Date: Tuesday, September 28, 2010 - 8:05 am

I've just tried bisecting the patches, and tried applying the first
three (well, two since I combined patches #2 and #3).  Simply enabling
the init_itables code wasn't enough to trigger the problem.  It looks
like the problem is in the last three patches (probably in one of the
patches where we convert ext4 to use sb_issue_zeroout, either the
extent or the resize code).

What I'll probably do (unless we find the problem very quickly) is to
reorder things so that we take the init_itable patch and the sysfs
feature patch, and put the rest into the unstable portion of the patch
queue.  That way I can work on the rest of the ext4 patches for the
merge window, without getting blocked on this patch series.  And if we
don't manage to figure out what went wrong, while it would be nice to
simplify the code for 2.6.36, it won't be the end of the world if they
need to wait until the next cycle.

				- Ted
--

From: Lukas Czerner
Date: Wednesday, September 29, 2010 - 6:37 am

Hi Ted,

this is really strange. I have never seen anything like this and I have
tried running the xfstests several times on the patchset while I was
creating it. Unfortunately I am not able to reproduce those errors even
now. I am running 2.6.26-rc6 with real SSD device.

Maybe the one difference is that I am using 2.6.36-rc6, so there is old
sb_issue_discard() interface (no flags and gfp_mask in function definition).
 And it is before Christoph's "remove BLKDEV_IFL_WAIT" patch
(dd3932eddf428571762596e17b65f5dc92ca361b in Jens for-next branch).


Are you sure about the test numbers ? 083 does not even run on ext4 it
--

From: Lukas Czerner
Date: Friday, October 1, 2010 - 8:58 am

After extensive xfstest-ing I have not been able to reproduce it.
However, after a while hammering it with other stress test (the one
I have proposed to test batched discard implementation with) I have
got a panic due to not up-to-date buffer_head in submit_bh() :
kernel BUG at fs/buffer.c:2910! - I have been able to reproduce it
every time (on different BUG_ON sometimes)

The one responsible for this bug is [PATCH 4/6] Use sb_issue_zeroout
in setup_new_group_blocks. Without this patch I was not able to hit
that panic again. Also I have manage to find and fix the problem in
this patch. I'll send fixed version of [PATCH 4/6] shortly.

Importantly, with that fix I was not able to hit that panic again,
but since I was not able to reproduce what you have seen with
xfstests it may be a different issue. It would be great if you have
time to try it with that fixed patch, to see if ti makes a
difference.

Thanks!
-Lukas
--

From: Ted Ts'o
Date: Saturday, October 2, 2010 - 7:43 pm

OK, so this looks like it's working for me now.  This combines patches
the old 2/6 and 3/6.  I've added documentation for the mount options,
fixed the lazyinit daemon shutdown-on-mount bug, as well as cleaning
up the sanity check in ext4_init_inode_table().

I've run XFStests with a 2CPU KVM test setup (this may have been why
you couldn't reproduce it --- perhaps you weren't using an SMP
system?), with both 1k and 4k block sizes, and it seem to pass without
problems.

     	       	     	    	   	- Ted

From 7d729b3c1baaed56099378927e433fd90a46a91f Mon Sep 17 00:00:00 2001
From: Lukas Czerner <lczerner@redhat.com>
Date: Sat, 2 Oct 2010 16:43:37 -0400
Subject: [PATCH] ext4: Add support for lazy inode table initialization

When the lazy_itable_init extended option is passed to mke2fs, it
considerably speeds up filesystem creation because inode tables are
not zeroed out.  The fact that parts of the inode table are
uninitialized is not a problem so long as the block group descriptors,
which contain information regarding how much of the inode table has
been initialized, has not been corrupted However, if the block group
checksums are not valid, e2fsck must scan the entire inode table, and
the the old, uninitialized data could potentially cause e2fsck to
report false problems.

Hence, it is important for the inode tables to be initialized as soon
as possble.  This commit adds this feature so that mke2fs can safely
use the lazy inode table initialization feature to speed up formatting
file systems.

This is done via a new new kernel thread called ext4lazyinit, which is
created on demand and destroyed, when it is no longer needed.  There
is only one thread for all ext4 filesystems in the system. When the
first filesystem with inititable mount option is mounted, ext4lazyinit
thread is created, then the filesystem can register its request in the
request list.

This thread then walks through the list of requests picking up
scheduled requests and invoking ext4_init_inode_table(). ...
From: Ted Ts'o
Date: Sunday, October 3, 2010 - 7:36 pm

I've made some more changes.  This version updates the timing control.
The major changes are:

1) Time the it takes to clear the inode table with a barrier (once),
and then use it for the rest of the block groups for that file system.

2) s_li_wait_nult wasn't getting defaulted, so we weren't waiting any
time at all between sb_issue_zeroout calls.

3) Fix the timer arithmetic so it works across jiffies rollover.
(This means using time_before() instead of <)

						- Ted

From 87fe012bfa04e1ac95a4a96f90b70c2a0983e228 Mon Sep 17 00:00:00 2001
From: Lukas Czerner <lczerner@redhat.com>
Date: Sun, 3 Oct 2010 22:31:15 -0400
Subject: [PATCH] ext4: Add support for lazy inode table initialization

When the lazy_itable_init extended option is passed to mke2fs, it
considerably speeds up filesystem creation because inode tables are
not zeroed out.  The fact that parts of the inode table are
uninitialized is not a problem so long as the block group descriptors,
which contain information regarding how much of the inode table has
been initialized, has not been corrupted However, if the block group
checksums are not valid, e2fsck must scan the entire inode table, and
the the old, uninitialized data could potentially cause e2fsck to
report false problems.

Hence, it is important for the inode tables to be initialized as soon
as possble.  This commit adds this feature so that mke2fs can safely
use the lazy inode table initialization feature to speed up formatting
file systems.

This is done via a new new kernel thread called ext4lazyinit, which is
created on demand and destroyed, when it is no longer needed.  There
is only one thread for all ext4 filesystems in the system. When the
first filesystem with inititable mount option is mounted, ext4lazyinit
thread is created, then the filesystem can register its request in the
request list.

This thread then walks through the list of requests picking up
scheduled requests and invoking ext4_init_inode_table(). Next schedule
time for the ...
From: Ted Ts'o
Date: Monday, October 4, 2010 - 12:31 am

This should be a time_after_eq(jiffies, next_wakeup), of course...

					- Ted

--

From: Lukas Czerner
Date: Monday, October 4, 2010 - 6:14 am

Hi Ted,

first of all, thank you very much for tracking down those issues and for
all the improvement you have done on this. Now, I have some questions
about changes you have introduced with this patch.


So if I understand this right it means that we measure the time it takes
to zeroout inode table just once (set the lr_timeout) and then we use
this value for all the following zeroouts.

Initially I have done this "measuring time" thing to adaptively balance
the load it generates and thus do not disturb other ongoing I/O very
much. So this change does not really make sense to me, because when we
measure the time right after the mount (just once) and the system is
relatively still we end up with rather small lr_timeout and then, when
system is under heavy load it will keep the same zeroout rate as when
system was still - resulting in much more impact on performance than my
previous solution.

Conversely, when the system is under heavy load when the filesystem
with init_itable option is mounted the zeroing will proceed very slowly

Actually it is getting defaulted:

+		case Opt_init_inode_table:
+			set_opt(sbi->s_mount_opt, INIT_INODE_TABLE);
+			if (args[0].from) {
+				if (match_int(&args[0], &option))
+					return 0;
+			} else
+				option = EXT4_DEF_LI_WAIT_MULT;
+			if (option < 0)
+				return 0;
+			sbi->s_li_wait_mult = option;
+			break;

EXT4_DEF_LI_WAIT_MULT is the default value for s_li_wait_mult.


When we are using this time functions (with really confusing names) we can
use one here as well I think.

		if (time_after_eq(jiffies, next_wakeup) {
			cond_resched();
			continue;
I do not know why I did this, but apparently we do not need to test

-Lukas
--

From: Lukas Czerner
Date: Monday, October 4, 2010 - 6:19 am

Ok, this is not right:) and it is probably why do you thought that it was
not getting defaulted at all (because it actually did not). It should be:

		case Opt_init_inode_table:
			set_opt(sbi->s_mount_opt, INIT_INODE_TABLE);
			option = EXT4_DEF_LI_WAIT_MULT;
			if (args[0].from) {
				if (match_int(&args[0], &option))
					return 0;
			if (option < 0)
				return 0;
			sbi->s_li_wait_mult = option;

-Lukas
--

Previous thread: [PATCH 5/6] Use sb_issue_zeroout in ext4_ext_zeroout by Lukas Czerner on Thursday, September 16, 2010 - 5:47 am. (1 message)

Next thread: [RESEND][PATCH] ext4: create own llseek function to handle 2 maxbytes by Toshiyuki Okajima on Thursday, September 16, 2010 - 10:32 pm. (2 messages)