* [PATCH] ext4: Only call update_backups if necessary in online resize.
@ 2012-09-24 15:00 Tao Ma
2012-09-26 3:39 ` Theodore Ts'o
0 siblings, 1 reply; 6+ messages in thread
From: Tao Ma @ 2012-09-24 15:00 UTC (permalink / raw)
To: linux-ext4
From: Tao Ma <boyu.mt@taobao.com>
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 <boyu.mt@taobao.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] ext4: Only call update_backups if necessary in online resize.
2012-09-24 15:00 [PATCH] ext4: Only call update_backups if necessary in online resize Tao Ma
@ 2012-09-26 3:39 ` Theodore Ts'o
2012-09-26 3:51 ` Tao Ma
0 siblings, 1 reply; 6+ messages in thread
From: Theodore Ts'o @ 2012-09-26 3:39 UTC (permalink / raw)
To: Tao Ma; +Cc: linux-ext4
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ext4: Only call update_backups if necessary in online resize.
2012-09-26 3:39 ` Theodore Ts'o
@ 2012-09-26 3:51 ` Tao Ma
2012-09-26 4:03 ` Theodore Ts'o
0 siblings, 1 reply; 6+ messages in thread
From: Tao Ma @ 2012-09-26 3:51 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: linux-ext4
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ext4: Only call update_backups if necessary in online resize.
2012-09-26 3:51 ` Tao Ma
@ 2012-09-26 4:03 ` Theodore Ts'o
2012-09-26 5:26 ` Kevin Liao
0 siblings, 1 reply; 6+ messages in thread
From: Theodore Ts'o @ 2012-09-26 4:03 UTC (permalink / raw)
To: Tao Ma; +Cc: linux-ext4
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ext4: Only call update_backups if necessary in online resize.
2012-09-26 4:03 ` Theodore Ts'o
@ 2012-09-26 5:26 ` Kevin Liao
2012-09-26 13:57 ` Theodore Ts'o
0 siblings, 1 reply; 6+ messages in thread
From: Kevin Liao @ 2012-09-26 5:26 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: Tao Ma, linux-ext4
2012/9/26 Theodore Ts'o <tytso@mit.edu>
>
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ext4: Only call update_backups if necessary in online resize.
2012-09-26 5:26 ` Kevin Liao
@ 2012-09-26 13:57 ` Theodore Ts'o
0 siblings, 0 replies; 6+ messages in thread
From: Theodore Ts'o @ 2012-09-26 13:57 UTC (permalink / raw)
To: Kevin Liao; +Cc: Tao Ma, linux-ext4
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-09-26 13:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-24 15:00 [PATCH] ext4: Only call update_backups if necessary in online resize Tao Ma
2012-09-26 3:39 ` Theodore Ts'o
2012-09-26 3:51 ` Tao Ma
2012-09-26 4:03 ` Theodore Ts'o
2012-09-26 5:26 ` Kevin Liao
2012-09-26 13:57 ` Theodore Ts'o
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).