From: "Theodore Y. Ts'o" <tytso@mit.edu>
To: Chunguang Xu <brookxu.cn@gmail.com>
Cc: adilger.kernel@dilger.ca, linux-ext4@vger.kernel.org
Subject: Re: [PATCH 6/8] ext4: add a helper function to validate metadata block
Date: Tue, 8 Dec 2020 23:55:15 -0500 [thread overview]
Message-ID: <20201209045515.GH52960@mit.edu> (raw)
In-Reply-To: <1604764698-4269-6-git-send-email-brookxu@tencent.com>
On Sat, Nov 07, 2020 at 11:58:16PM +0800, Chunguang Xu wrote:
> From: Chunguang Xu <brookxu@tencent.com>
>
> There is a need to check whether a block or a segment overlaps
> with metadata, since information of system_zone is incomplete,
> we need a more accurate function. Now we check whether it
> overlaps with block bitmap, inode bitmap, and inode table.
> Perhaps it is better to add a check of super_block and block
> group descriptor and provide a helper function.
The original code was valid only for file systems that are not using
flex_bg. I suspect the Lustre folks who implemented mballoc.c did so
before flex_bg, and fortunately, on flex_bg we the check is simply
going to have more false negaties, but not any false positives, so no
one noticed.
> +/*
> + * Returns 1 if the passed-in block region (block, block+count)
> + * overlaps with some other filesystem metadata blocks. Others,
> + * return 0.
> + */
> +int ext4_metadata_block_overlaps(struct super_block *sb,
> + ext4_group_t block_group,
> + ext4_fsblk_t block,
> + unsigned long count)
> +{
> + struct ext4_sb_info *sbi = EXT4_SB(sb);
> + struct ext4_group_desc *gdp;
> + int gd_first = ext4_group_first_block_no(sb, block_group);
> + int itable, gd_blk;
> + int ret = 0;
> +
> + gdp = ext4_get_group_desc(sb, block_group, NULL);
> + // check block bitmap and inode bitmap
> + if (in_range(ext4_block_bitmap(sb, gdp), block, count) ||
> + in_range(ext4_inode_bitmap(sb, gdp), block, count))
We are only checking a single block group descriptor; this is fine if
the allocation bitmaps and inode table are guaranteed to be located in
their own block group. But this is no longer true when flex_bg is
enabled.
I think what we should do is to rely on the rb tree maintained by
block_validity.c (if the inode number is zero, then the entry refers
to blocks in the "system zone"); that's going to be a much more
complete check.
What do you think?
- Ted
next prev parent reply other threads:[~2020-12-09 4:56 UTC|newest]
Thread overview: 27+ 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
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 [this message]
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
-- strict thread matches above, loose matches on Subject: below --
2020-10-21 9:15 [PATCH " Chunguang Xu
2020-10-21 9:15 ` [PATCH 6/8] ext4: add a helper function to validate metadata block Chunguang Xu
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=20201209045515.GH52960@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 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).