From: "Theodore Y. Ts'o" <tytso@mit.edu>
To: brookxu <brookxu.cn@gmail.com>
Cc: adilger.kernel@dilger.ca, linux-ext4@vger.kernel.org
Subject: Re: [PATCH RESEND 4/8] ext4: add the gdt block of meta_bg to system_zone
Date: Wed, 9 Dec 2020 14:39:35 -0500 [thread overview]
Message-ID: <20201209193935.GO52960@mit.edu> (raw)
In-Reply-To: <dd6c2921-1397-4b1a-5a20-23956f9cf956@gmail.com>
On Wed, Dec 09, 2020 at 07:48:09PM +0800, brookxu wrote:
>
> Maybe I missed something. If i% meta_bg_size is used instead, if
> flex_size <64, then we will miss some flex_bg. There seems to be
> a contradiction here. In the scenario where only flex_bg is
> enabled, it may not be appropriate to use meta_bg_size. In the
> scenario where only meta_bg is enabled, it may not be appropriate
> to use flex_size.
>
> As you said before, it maybe better to remove
>
> if ((i <5) || ((i% flex_size) == 0))
>
> and do it for all groups.
I don't think the original (i % flex_size) made any sense in the first
place.
What flex_bg does is that it collects the allocation bitmaps and inode
tables for each block group and locates them within the first block
group in a flex_bg. It doesn't have anything to do with whether or
not a particular block group has a backup copy of the superblock and
block group descriptor table --- in non-meta_bg file systems and the
meta_bg file systems where the block group is less than
s_first_meta_bg * EXT4_DESC_PER_BLOCK(sb). And the condition in
question is only about whether or not to add the backup superblock and
backup block group descriptors. So checking for i % flex_size made no
sense, and I'm not sure that check was there in the first place.
> In this way weh won't miss some flex_bg, meta_bg, and sparse_bg.
> I tested it on an 80T disk and found that the performance loss
> was small:
>
> unpatched kernel:
> ext4_setup_system_zone() takes 524ms,
>
> patched kernel:
> ext4_setup_system_zone() takes 552ms,
I don't really care that much about the time it takes to execute
ext4_setup_system_zone().
The really interesting question is how large is the rb_tree
constructed by that function, and what is the percentage increase of
time that the ext4_inode_block_valid() function takes. (e.g., how
much additional memory is the system_blks tree taking, and how deep is
that tree, since ext4_inode_block_valid() gets called every time we
allocate or free a block, and every time we need to validate an extent
tree node.
Cheers,
- Ted
next prev parent reply other threads:[~2020-12-09 19:40 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-07 15:58 [PATCH RESEND 1/8] ext4: use ext4_assert() to replace J_ASSERT() Chunguang Xu
2020-11-07 15:58 ` [PATCH RESEND 2/8] ext4: remove redundant mb_regenerate_buddy() Chunguang Xu
2020-12-03 14:42 ` Theodore Y. Ts'o
2020-11-07 15:58 ` [PATCH RESEND 3/8] ext4: simplify the code of mb_find_order_for_block Chunguang Xu
2020-12-03 14:43 ` Theodore Y. Ts'o
2020-11-07 15:58 ` [PATCH RESEND 4/8] ext4: add the gdt block of meta_bg to system_zone Chunguang Xu
2020-12-03 15:08 ` Theodore Y. Ts'o
2020-12-04 1:26 ` brookxu
2020-12-09 4:34 ` Theodore Y. Ts'o
2020-12-09 11:48 ` brookxu
2020-12-09 19:39 ` Theodore Y. Ts'o [this message]
2020-12-10 11:00 ` brookxu
2020-12-15 1:14 ` brookxu
2020-12-15 20:13 ` Theodore Y. Ts'o
2020-12-17 16:01 ` Andreas Dilger
2020-12-04 1:29 ` brookxu
2020-11-07 15:58 ` [PATCH RESEND 5/8] ext4: update ext4_data_block_valid related comments Chunguang Xu
2020-12-09 19:11 ` Theodore Y. Ts'o
2020-11-07 15:58 ` [PATCH 6/8] ext4: add a helper function to validate metadata block Chunguang Xu
2020-12-09 4:55 ` Theodore Y. Ts'o
2020-12-09 12:12 ` brookxu
2020-11-07 15:58 ` [PATCH RESEND 7/8] ext4: delete invalid code inside ext4_xattr_block_set() Chunguang Xu
2020-12-09 19:24 ` Theodore Y. Ts'o
2020-11-07 15:58 ` [PATCH RESEND 8/8] ext4: fix a memory leak of ext4_free_data Chunguang Xu
2020-12-09 19:29 ` Theodore Y. Ts'o
2020-12-03 14:38 ` [PATCH RESEND 1/8] ext4: use ext4_assert() to replace J_ASSERT() Theodore Y. Ts'o
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201209193935.GO52960@mit.edu \
--to=tytso@mit.edu \
--cc=adilger.kernel@dilger.ca \
--cc=brookxu.cn@gmail.com \
--cc=linux-ext4@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.