From: Tao Ma <[email protected]>
Now in online resize, we will add a bunch of groups at one time in
ext4_flex_group_add, so in most cases a lot of group descriptors
will be in the same group block. But in the end of this function,
update_backups will be called for every group descriptor and the
same block will be copied and journalled again and again.
It is really a waste.
So add a old_gdb to store the group block we have already done
the backup and skip the backup until we meet with a new group block.
Signed-off-by: Tao Ma <[email protected]>
---
fs/ext4/resize.c | 13 +++++++++++--
1 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 019d528..ae2f5fc 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -1348,6 +1348,7 @@ exit_journal:
if (!err) {
int i;
+ sector_t old_gdb = 0;
update_backups(sb, sbi->s_sbh->b_blocknr, (char *)es,
sizeof(struct ext4_super_block));
for (i = 0; i < flex_gd->count; i++, group++) {
@@ -1355,8 +1356,16 @@ exit_journal:
int gdb_num;
gdb_num = group / EXT4_BLOCKS_PER_GROUP(sb);
gdb_bh = sbi->s_group_desc[gdb_num];
- update_backups(sb, gdb_bh->b_blocknr, gdb_bh->b_data,
- gdb_bh->b_size);
+ /*
+ * only backup the group descriptor block
+ * which hasn't been updated before.
+ */
+ if (old_gdb != gdb_bh->b_blocknr) {
+ update_backups(sb, gdb_bh->b_blocknr,
+ gdb_bh->b_data,
+ gdb_bh->b_size);
+ old_gdb = gdb_bh->b_blocknr;
+ }
}
}
exit:
--
1.7.1
On Mon, Sep 24, 2012 at 11:00:17PM +0800, Tao Ma wrote:
> @@ -1355,8 +1356,16 @@ exit_journal:
> int gdb_num;
> gdb_num = group / EXT4_BLOCKS_PER_GROUP(sb);
> gdb_bh = sbi->s_group_desc[gdb_num];
> - update_backups(sb, gdb_bh->b_blocknr, gdb_bh->b_data,
> - gdb_bh->b_size);
> + /*
> + * only backup the group descriptor block
> + * which hasn't been updated before.
> + */
> + if (old_gdb != gdb_bh->b_blocknr) {
> + update_backups(sb, gdb_bh->b_blocknr,
> + gdb_bh->b_data,
> + gdb_bh->b_size);
> + old_gdb = gdb_bh->b_blocknr;
> + }
> }
> }
Don't we also need to add a call to update_backups() at the end of the
loop so we update the backup block descriptors for the very last set
of block groups?
Thanks,
- Ted
On 09/26/2012 11:39 AM, Theodore Ts'o wrote:
> On Mon, Sep 24, 2012 at 11:00:17PM +0800, Tao Ma wrote:
>> @@ -1355,8 +1356,16 @@ exit_journal:
>> int gdb_num;
>> gdb_num = group / EXT4_BLOCKS_PER_GROUP(sb);
>> gdb_bh = sbi->s_group_desc[gdb_num];
>> - update_backups(sb, gdb_bh->b_blocknr, gdb_bh->b_data,
>> - gdb_bh->b_size);
>> + /*
>> + * only backup the group descriptor block
>> + * which hasn't been updated before.
>> + */
>> + if (old_gdb != gdb_bh->b_blocknr) {
>> + update_backups(sb, gdb_bh->b_blocknr,
>> + gdb_bh->b_data,
>> + gdb_bh->b_size);
>> + old_gdb = gdb_bh->b_blocknr;
>> + }
>> }
>> }
>
> Don't we also need to add a call to update_backups() at the end of the
> loop so we update the backup block descriptors for the very last set
> of block groups?
Why? If it isn't the same as the previous one, it will be updated. If
it is the same, it don't need to be updated. I don't see what your mean
here.
Thanks
Tao
On Wed, Sep 26, 2012 at 11:51:01AM +0800, Tao Ma wrote:
> Why? If it isn't the same as the previous one, it will be updated. If
> it is the same, it don't need to be updated. I don't see what your mean
> here.
Oh, I see; sorry, I was just looking at the patch and not the file
with the changes in context. I thought you were updating the backup
descriptors after modifying the last block group in the meta_bg group,
but that was my confusion.
Yes, your patch looks good and I will apply it, thanks!!
- Ted
2012/9/26 Theodore Ts'o <[email protected]>
>
> On Wed, Sep 26, 2012 at 11:51:01AM +0800, Tao Ma wrote:
> > Why? If it isn't the same as the previous one, it will be updated. If
> > it is the same, it don't need to be updated. I don't see what your mean
> > here.
>
> Oh, I see; sorry, I was just looking at the patch and not the file
> with the changes in context. I thought you were updating the backup
> descriptors after modifying the last block group in the meta_bg group,
> but that was my confusion.
>
> Yes, your patch looks good and I will apply it, thanks!!
>
> - Ted
Hi Ted,
Is this patch should be merged with your previous patches about meta_bg
online resize on 9/14?
[PATCH 2/9] avoid duplicate writes of the backup bg descriptor blocks
[PATCH 5/9] add online resizing support for meta_bg and 64-bit file systems
Regards,
Kevin Liao
On Wed, Sep 26, 2012 at 01:26:14PM +0800, Kevin Liao wrote:
>
> Is this patch should be merged with your previous patches about meta_bg
> online resize on 9/14?
>
> [PATCH 2/9] avoid duplicate writes of the backup bg descriptor blocks
> [PATCH 5/9] add online resizing support for meta_bg and 64-bit file systems
It would be, except that those patches are now in the "master" branch
of the ext4 git tree, which is an implicit promise on my part not to
rebase or change the commit. This allows people to build git trees
based on the "master" branch. Commits between "master" and "dev" are
still subject to change, although I am pretty happen with them at that
point, so it will general be only to fix bugs in the patches, or to
adjust the commit descrpition, etc.
- Ted