From: "Theodore Ts'o" <tytso@mit.edu>
To: Kemeng Shi <shikemeng@huaweicloud.com>
Cc: adilger.kernel@dilger.ca, linux-ext4@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 12/13] ext4: remove unnecessary check for avoiding multiple update_backups in ext4_flex_group_add
Date: Thu, 17 Aug 2023 10:11:12 -0400 [thread overview]
Message-ID: <20230817141112.GZ2247938@mit.edu> (raw)
In-Reply-To: <2a0c45d9-29f0-10a3-fc40-d48e101c8d91@huaweicloud.com>
On Thu, Aug 17, 2023 at 11:53:52AM +0800, Kemeng Shi wrote:
>
>
> on 8/16/2023 11:47 AM, Theodore Ts'o wrote:
> > On Thu, Jun 29, 2023 at 08:00:43PM +0800, Kemeng Shi wrote:
> >> Commit 0acdb8876fead ("ext4: don't call update_backups() multiple times
> >> for the same bg") add check in ext4_flex_group_add to avoid call
> >> update_backups multiple times for block group descriptors in the same
> >> group descriptor block. However, we have already call update_backups in
> >> block step, so the added check is unnecessary.
> >
> > I'm having trouble understaind this comit. What do you mean by the
> > "block step" in the last paragraph?
> >
> Sorry for the confusing stuff. I mean we foreach in group descriptor block
> step instead of foreach in group descriptor step to update backup.
> So there is no chance to call update_backups for different descriptors
> in the same bg.
I'm still confused. This commit is not so much removing an
unnecessary check as much as forcing update_backups() to be called for
every single block group. Right?
So either we're doing extra work, or (b) we're fixing a problem
because we actually *need* to call update_backups() for every single
block group.
More generally, this whole patch series is making it clear that the
online resize code is hard to understand, because it's super
complicated, leading to potential bugs, and very clearly code which is
very hard to maintain. So this may mean we need better comments to
make it clear *when* the backup block groups are going to be
accomplished, given the various different cases (e.g., no flex_bg or
meta_bg, with flex_bg, or with meat_bg).
- Ted
next prev parent reply other threads:[~2023-08-17 14:12 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-29 12:00 [PATCH 00/13] fixes and cleanups to ext4 resize Kemeng Shi
2023-06-29 12:00 ` [PATCH 01/13] ext4: correct offset of gdb backup in non meta_bg group to update_backups Kemeng Shi
2023-08-16 3:03 ` Theodore Ts'o
2023-06-29 12:00 ` [PATCH 02/13] ext4: add missed brelse in update_backups Kemeng Shi
2023-08-16 3:03 ` Theodore Ts'o
2023-06-29 12:00 ` [PATCH 03/13] ext4: correct return value of ext4_convert_meta_bg Kemeng Shi
2023-08-16 3:00 ` Theodore Ts'o
2023-08-17 2:48 ` Kemeng Shi
2023-08-17 13:58 ` Theodore Ts'o
2023-06-29 12:00 ` [PATCH 04/13] ext4: remove gdb backup copy for meta bg in setup_new_flex_group_blocks Kemeng Shi
2023-08-16 3:10 ` Theodore Ts'o
2023-06-29 12:00 ` [PATCH 05/13] ext4: fix typo " Kemeng Shi
2023-08-16 3:10 ` Theodore Ts'o
2023-06-29 12:00 ` [PATCH 06/13] ext4: remove redundant check of count Kemeng Shi
2023-08-16 3:14 ` Theodore Ts'o
2023-06-29 12:00 ` [PATCH 07/13] ext4: remove commented code in reserve_backup_gdb Kemeng Shi
2023-08-16 3:14 ` Theodore Ts'o
2023-06-29 12:00 ` [PATCH 08/13] ext4: calculate free_clusters_count in cluster unit in verify_group_input Kemeng Shi
2023-08-16 3:22 ` Theodore Ts'o
2023-08-17 2:57 ` Kemeng Shi
2023-06-29 12:00 ` [PATCH 09/13] ext4: remove EXT4FS_DEBUG defination in resize.c Kemeng Shi
2023-08-16 3:23 ` Theodore Ts'o
2023-06-29 12:00 ` [PATCH 10/13] ext4: use saved local variable sbi instead of EXT4_SB(sb) Kemeng Shi
2023-08-16 3:24 ` Theodore Ts'o
2023-06-29 12:00 ` [PATCH 11/13] ext4: correct gdblock calculation in add_new_gdb_meta_bg to support non first group Kemeng Shi
2023-08-16 3:45 ` Theodore Ts'o
2023-08-17 3:38 ` Kemeng Shi
2023-08-17 14:03 ` Theodore Ts'o
2023-08-18 2:29 ` Kemeng Shi
2023-08-18 4:07 ` Theodore Ts'o
2023-08-18 7:09 ` Kemeng Shi
2023-08-18 16:54 ` Theodore Ts'o
2023-08-22 2:48 ` Kemeng Shi
2023-06-29 12:00 ` [PATCH 12/13] ext4: remove unnecessary check for avoiding multiple update_backups in ext4_flex_group_add Kemeng Shi
2023-08-16 3:47 ` Theodore Ts'o
2023-08-17 3:53 ` Kemeng Shi
2023-08-17 14:11 ` Theodore Ts'o [this message]
2023-08-18 3:16 ` Kemeng Shi
2023-08-18 5:00 ` Theodore Ts'o
2023-08-18 8:42 ` Kemeng Shi
2023-08-18 16:00 ` Theodore Ts'o
2023-08-22 2:21 ` Kemeng Shi
2023-06-29 12:00 ` [PATCH 13/13] ext4: remove unnecessary initialization of count2 in set_flexbg_block_bitmap Kemeng Shi
2023-08-16 3:48 ` Theodore 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=20230817141112.GZ2247938@mit.edu \
--to=tytso@mit.edu \
--cc=adilger.kernel@dilger.ca \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=shikemeng@huaweicloud.com \
/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).