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 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.