linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/13] fixes and cleanups to ext4 resize
@ 2023-08-26 17:46 Kemeng Shi
  2023-08-26 17:47 ` [PATCH v3 01/13] ext4: correct offset of gdb backup in non meta_bg group to update_backups Kemeng Shi
                   ` (13 more replies)
  0 siblings, 14 replies; 15+ messages in thread
From: Kemeng Shi @ 2023-08-26 17:46 UTC (permalink / raw)
  To: tytso, adilger.kernel, linux-ext4, linux-kernel

This series contains some random cleanups and a few fixes to correct gdb
backup copy, fix buffer_head leak and so on. More details can be found
in respective log messages.
I run kvm-xfstest with config "ext4/all" and "-g auto" together with
mballoc patchset I sent before.

v2->v3:
-Remove unused ext4_meta_bg_first_group defination in patch 11

v1->v2:
-Prioritize returning error in "err" in patch 3
-Rewrite patch 11
-Improve description and add comment to sbi->s_group_desc in patch 12
-Collect RVB from Ted for rest patches

-------------------- Summary report
KERNEL:    kernel 6.4.0-rc5-xfstests-g90158dd3c06b #23 SMP PREEMPT_DYNAMIC
Wed Jun 28 17:19:32 CST 2023 x86_64
CPUS:      2
MEM:       1975.36

ext4/4k: 531 tests, 31 skipped, 3421 seconds
ext4/1k: 527 tests, 33 skipped, 4638 seconds
ext4/ext3: 523 tests, 119 skipped, 3289 seconds
ext4/encrypt: 509 tests, 3 failures, 138 skipped, 2163 seconds
  Failures: generic/681 generic/682 generic/691
ext4/nojournal: 526 tests, 5 failures, 98 skipped, 3469 seconds
  Failures: ext4/301 ext4/304 generic/455 generic/459 generic/581
ext4/ext3conv: 528 tests, 31 skipped, 3877 seconds
ext4/adv: 528 tests, 4 failures, 38 skipped, 4170 seconds
  Failures: generic/475 generic/477
  Flaky: generic/455: 80% (4/5)   generic/482: 80% (4/5)
ext4/dioread_nolock: 529 tests, 31 skipped, 3825 seconds
ext4/data_journal: 527 tests, 3 failures, 99 skipped, 3307 seconds
  Failures: generic/455 generic/484
  Flaky: generic/068: 40% (2/5)
ext4/bigalloc_4k: 503 tests, 35 skipped, 3286 seconds
ext4/bigalloc_1k: 503 tests, 1 failures, 43 skipped, 4421 seconds
  Failures: shared/298
Totals: 5798 tests, 696 skipped, 75 failures, 0 errors, 39645s

FSTESTVER: blktests 676d42c (Thu, 2 Mar 2023 15:25:44 +0900)
FSTESTVER: e2fsprogs archive/debian/1.47.0-1 (Mon, 6 Feb 2023 22:36:16 -0500)
FSTESTVER: fio  fio-3.31 (Tue, 9 Aug 2022 14:41:25 -0600)
FSTESTVER: fsverity v1.5-6-g5d6f7c4 (Mon, 30 Jan 2023 23:22:45 -0800)
FSTESTVER: ima-evm-utils v1.3.2 (Wed, 28 Oct 2020 13:18:08 -0400)
FSTESTVER: nvme-cli v1.16 (Thu, 11 Nov 2021 13:09:06 -0800)
FSTESTVER: quota  v4.05-53-gd90b7d5 (Tue, 6 Dec 2022 12:59:03 +0100)
FSTESTVER: util-linux v2.38.1 (Thu, 4 Aug 2022 11:06:21 +0200)
FSTESTVER: xfsprogs v6.1.1 (Fri, 13 Jan 2023 19:06:37 +0100)
FSTESTVER: xfstests v2023.02.26-8-g821ef488 (Thu, 2 Mar 2023 10:23:51 -0500)
FSTESTVER: xfstests-bld 35650073 (Mon, 6 Mar 2023 20:48:08 -0500)
FSTESTVER: zz_build-distro bullseye
FSTESTCFG: ext4/all
FSTESTSET: -g auto
FSTESTOPT: aex

There more failures compared with "good" report in reply from Ted [1] as
following:
ext4/nojournal: generic/459 generic/581
ext4/adv: Flaky: generic/455: 80% (4/5)   generic/482: 80% (4/5)
ext4/data_journal: Flaky: generic/068: 40% (2/5)
It seems still a "good" test run according to rules listed in [1].
Please let me know if more tests to run. Thanks!

[1] https://lore.kernel.org/linux-ext4/db478a24-39f5-3cef-8814-89406ce4d2ca@huawei.com/T/#ma9de3f355f0300291d32fe1f0b32c5660c9bd191



Kemeng Shi (13):
  ext4: correct offset of gdb backup in non meta_bg group to
    update_backups
  ext4: add missed brelse in update_backups
  ext4: correct return value of ext4_convert_meta_bg
  ext4: remove gdb backup copy for meta bg in
    setup_new_flex_group_blocks
  ext4: fix typo in setup_new_flex_group_blocks
  ext4: remove redundant check of count
  ext4: remove commented code in reserve_backup_gdb
  ext4: calculate free_clusters_count in cluster unit in
    verify_group_input
  ext4: remove EXT4FS_DEBUG defination in resize.c
  ext4: use saved local variable sbi instead of EXT4_SB(sb)
  ext4: simplify the gdbblock calculation in add_new_gdb_meta_bg
  ext4: remove unnecessary check to avoid repeat update_backups for the
    same gdb
  ext4: remove unnecessary initialization of count2 in
    set_flexbg_block_bitmap

 fs/ext4/ext4.h   |  1 +
 fs/ext4/resize.c | 94 +++++++++++++++++++-----------------------------
 2 files changed, 38 insertions(+), 57 deletions(-)

-- 
2.30.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v3 01/13] ext4: correct offset of gdb backup in non meta_bg group to update_backups
  2023-08-26 17:46 [PATCH v3 00/13] fixes and cleanups to ext4 resize Kemeng Shi
@ 2023-08-26 17:47 ` Kemeng Shi
  2023-08-26 17:47 ` [PATCH v3 02/13] ext4: add missed brelse in update_backups Kemeng Shi
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Kemeng Shi @ 2023-08-26 17:47 UTC (permalink / raw)
  To: tytso, adilger.kernel, linux-ext4, linux-kernel

Commit 0aeaa2559d6d5 ("ext4: fix corruption when online resizing a 1K
bigalloc fs") found that primary superblock's offset in its group is not
equal to offset of backup superblock in its group when block size is 1K
and bigalloc is enabled. As group descriptor blocks are right after
superblock, we can't pass block number of gdb to update_backups for
the same reason.
The root casue of the issue above is that leading 1K padding block is
count as data block offset for primary block while backup block has
no padding block offset in its group.
Remove padding data block count to fix the issue for gdb backups.

For meta_bg case, update_backups treat blk_off as block number, do no
conversion in this case.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Reviewed-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/resize.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 0361c20910de..87cd5b07a970 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -1601,6 +1601,8 @@ static int ext4_flex_group_add(struct super_block *sb,
 		int gdb_num_end = ((group + flex_gd->count - 1) /
 				   EXT4_DESC_PER_BLOCK(sb));
 		int meta_bg = ext4_has_feature_meta_bg(sb);
+		sector_t padding_blocks = meta_bg ? 0 : sbi->s_sbh->b_blocknr -
+					 ext4_group_first_block_no(sb, 0);
 		sector_t old_gdb = 0;
 
 		update_backups(sb, ext4_group_first_block_no(sb, 0),
@@ -1612,8 +1614,8 @@ static int ext4_flex_group_add(struct super_block *sb,
 						     gdb_num);
 			if (old_gdb == gdb_bh->b_blocknr)
 				continue;
-			update_backups(sb, gdb_bh->b_blocknr, gdb_bh->b_data,
-				       gdb_bh->b_size, meta_bg);
+			update_backups(sb, gdb_bh->b_blocknr - padding_blocks,
+				       gdb_bh->b_data, gdb_bh->b_size, meta_bg);
 			old_gdb = gdb_bh->b_blocknr;
 		}
 	}
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 02/13] ext4: add missed brelse in update_backups
  2023-08-26 17:46 [PATCH v3 00/13] fixes and cleanups to ext4 resize Kemeng Shi
  2023-08-26 17:47 ` [PATCH v3 01/13] ext4: correct offset of gdb backup in non meta_bg group to update_backups Kemeng Shi
@ 2023-08-26 17:47 ` Kemeng Shi
  2023-08-26 17:47 ` [PATCH v3 03/13] ext4: correct return value of ext4_convert_meta_bg Kemeng Shi
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Kemeng Shi @ 2023-08-26 17:47 UTC (permalink / raw)
  To: tytso, adilger.kernel, linux-ext4, linux-kernel

add missed brelse in update_backups

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Reviewed-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/resize.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 87cd5b07a970..7cbc695b7005 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -1191,8 +1191,10 @@ static void update_backups(struct super_block *sb, sector_t blk_off, char *data,
 			   ext4_group_first_block_no(sb, group));
 		BUFFER_TRACE(bh, "get_write_access");
 		if ((err = ext4_journal_get_write_access(handle, sb, bh,
-							 EXT4_JTR_NONE)))
+							 EXT4_JTR_NONE))) {
+			brelse(bh);
 			break;
+		}
 		lock_buffer(bh);
 		memcpy(bh->b_data, data, size);
 		if (rest)
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 03/13] ext4: correct return value of ext4_convert_meta_bg
  2023-08-26 17:46 [PATCH v3 00/13] fixes and cleanups to ext4 resize Kemeng Shi
  2023-08-26 17:47 ` [PATCH v3 01/13] ext4: correct offset of gdb backup in non meta_bg group to update_backups Kemeng Shi
  2023-08-26 17:47 ` [PATCH v3 02/13] ext4: add missed brelse in update_backups Kemeng Shi
@ 2023-08-26 17:47 ` Kemeng Shi
  2023-08-26 17:47 ` [PATCH v3 04/13] ext4: remove gdb backup copy for meta bg in setup_new_flex_group_blocks Kemeng Shi
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Kemeng Shi @ 2023-08-26 17:47 UTC (permalink / raw)
  To: tytso, adilger.kernel, linux-ext4, linux-kernel

Avoid to ignore error in "err".

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 fs/ext4/resize.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 7cbc695b7005..9cb9ed968599 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -1984,9 +1984,7 @@ static int ext4_convert_meta_bg(struct super_block *sb, struct inode *inode)
 
 errout:
 	ret = ext4_journal_stop(handle);
-	if (!err)
-		err = ret;
-	return ret;
+	return err ? err : ret;
 
 invalid_resize_inode:
 	ext4_error(sb, "corrupted/inconsistent resize inode");
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 04/13] ext4: remove gdb backup copy for meta bg in setup_new_flex_group_blocks
  2023-08-26 17:46 [PATCH v3 00/13] fixes and cleanups to ext4 resize Kemeng Shi
                   ` (2 preceding siblings ...)
  2023-08-26 17:47 ` [PATCH v3 03/13] ext4: correct return value of ext4_convert_meta_bg Kemeng Shi
@ 2023-08-26 17:47 ` Kemeng Shi
  2023-08-26 17:47 ` [PATCH v3 05/13] ext4: fix typo " Kemeng Shi
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Kemeng Shi @ 2023-08-26 17:47 UTC (permalink / raw)
  To: tytso, adilger.kernel, linux-ext4, linux-kernel

Wrong check of gdb backup in meta bg as following:
first_group is the first group of meta_bg which contains target group, so
target group is always >= first_group. We check if target group has gdb
backup by comparing first_group with [group + 1] and [group +
EXT4_DESC_PER_BLOCK(sb) - 1]. As group >= first_group, then [group + N] is
> first_group. So no copy of gdb backup in meta bg is done in
setup_new_flex_group_blocks.

No need to do gdb backup copy in meta bg from setup_new_flex_group_blocks
as we always copy updated gdb block to backups at end of
ext4_flex_group_add as following:

ext4_flex_group_add
  /* no gdb backup copy for meta bg any more */
  setup_new_flex_group_blocks

  /* update current group number */
  ext4_update_super
    sbi->s_groups_count += flex_gd->count;

  /*
   * if group in meta bg contains backup is added, the primary gdb block
   * of the meta bg will be copy to backup in new added group here.
   */
  for (; gdb_num <= gdb_num_end; gdb_num++)
    update_backups(...)

In summary, we can remove wrong gdb backup copy code in
setup_new_flex_group_blocks.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Reviewed-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/resize.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 9cb9ed968599..667381180b26 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -560,13 +560,8 @@ static int setup_new_flex_group_blocks(struct super_block *sb,
 		if (meta_bg == 0 && !ext4_bg_has_super(sb, group))
 			goto handle_itb;
 
-		if (meta_bg == 1) {
-			ext4_group_t first_group;
-			first_group = ext4_meta_bg_first_group(sb, group);
-			if (first_group != group + 1 &&
-			    first_group != group + EXT4_DESC_PER_BLOCK(sb) - 1)
-				goto handle_itb;
-		}
+		if (meta_bg == 1)
+			goto handle_itb;
 
 		block = start + ext4_bg_has_super(sb, group);
 		/* Copy all of the GDT blocks into the backup in this group */
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 05/13] ext4: fix typo in setup_new_flex_group_blocks
  2023-08-26 17:46 [PATCH v3 00/13] fixes and cleanups to ext4 resize Kemeng Shi
                   ` (3 preceding siblings ...)
  2023-08-26 17:47 ` [PATCH v3 04/13] ext4: remove gdb backup copy for meta bg in setup_new_flex_group_blocks Kemeng Shi
@ 2023-08-26 17:47 ` Kemeng Shi
  2023-08-26 17:47 ` [PATCH v3 06/13] ext4: remove redundant check of count Kemeng Shi
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Kemeng Shi @ 2023-08-26 17:47 UTC (permalink / raw)
  To: tytso, adilger.kernel, linux-ext4, linux-kernel

grop -> group

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Reviewed-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/resize.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 667381180b26..5e4856d68c4a 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -609,7 +609,7 @@ static int setup_new_flex_group_blocks(struct super_block *sb,
 		}
 
 handle_itb:
-		/* Initialize group tables of the grop @group */
+		/* Initialize group tables of the group @group */
 		if (!(bg_flags[i] & EXT4_BG_INODE_ZEROED))
 			goto handle_bb;
 
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 06/13] ext4: remove redundant check of count
  2023-08-26 17:46 [PATCH v3 00/13] fixes and cleanups to ext4 resize Kemeng Shi
                   ` (4 preceding siblings ...)
  2023-08-26 17:47 ` [PATCH v3 05/13] ext4: fix typo " Kemeng Shi
@ 2023-08-26 17:47 ` Kemeng Shi
  2023-08-26 17:47 ` [PATCH v3 07/13] ext4: remove commented code in reserve_backup_gdb Kemeng Shi
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Kemeng Shi @ 2023-08-26 17:47 UTC (permalink / raw)
  To: tytso, adilger.kernel, linux-ext4, linux-kernel

Remove zero check of count which is always non-zero.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Reviewed-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/resize.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 5e4856d68c4a..c51994e22af5 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -699,16 +699,14 @@ static int setup_new_flex_group_blocks(struct super_block *sb,
 			block = start;
 		}
 
-		if (count) {
-			err = set_flexbg_block_bitmap(sb, handle,
-						      flex_gd,
-						      EXT4_B2C(sbi, start),
-						      EXT4_B2C(sbi,
-							       start + count
-							       - 1));
-			if (err)
-				goto out;
-		}
+		err = set_flexbg_block_bitmap(sb, handle,
+				flex_gd,
+				EXT4_B2C(sbi, start),
+				EXT4_B2C(sbi,
+					start + count
+					- 1));
+		if (err)
+			goto out;
 	}
 
 out:
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 07/13] ext4: remove commented code in reserve_backup_gdb
  2023-08-26 17:46 [PATCH v3 00/13] fixes and cleanups to ext4 resize Kemeng Shi
                   ` (5 preceding siblings ...)
  2023-08-26 17:47 ` [PATCH v3 06/13] ext4: remove redundant check of count Kemeng Shi
@ 2023-08-26 17:47 ` Kemeng Shi
  2023-08-26 17:47 ` [PATCH v3 08/13] ext4: calculate free_clusters_count in cluster unit in verify_group_input Kemeng Shi
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Kemeng Shi @ 2023-08-26 17:47 UTC (permalink / raw)
  To: tytso, adilger.kernel, linux-ext4, linux-kernel

Remove commented code in reserve_backup_gdb

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Reviewed-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/resize.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index c51994e22af5..a370ca1a2bd6 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -1080,9 +1080,6 @@ static int reserve_backup_gdb(handle_t *handle, struct inode *inode,
 	for (i = 0; i < reserved_gdb; i++) {
 		int err2;
 		data = (__le32 *)primary[i]->b_data;
-		/* printk("reserving backup %lu[%u] = %lu\n",
-		       primary[i]->b_blocknr, gdbackups,
-		       blk + primary[i]->b_blocknr); */
 		data[gdbackups] = cpu_to_le32(blk + primary[i]->b_blocknr);
 		err2 = ext4_handle_dirty_metadata(handle, NULL, primary[i]);
 		if (!err)
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 08/13] ext4: calculate free_clusters_count in cluster unit in verify_group_input
  2023-08-26 17:46 [PATCH v3 00/13] fixes and cleanups to ext4 resize Kemeng Shi
                   ` (6 preceding siblings ...)
  2023-08-26 17:47 ` [PATCH v3 07/13] ext4: remove commented code in reserve_backup_gdb Kemeng Shi
@ 2023-08-26 17:47 ` Kemeng Shi
  2023-08-26 17:47 ` [PATCH v3 09/13] ext4: remove EXT4FS_DEBUG defination in resize.c Kemeng Shi
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Kemeng Shi @ 2023-08-26 17:47 UTC (permalink / raw)
  To: tytso, adilger.kernel, linux-ext4, linux-kernel

The field free_cluster_count in struct ext4_new_group_data should be
in units of clusters.  In verify_group_input() this field is being
filled in units of blocks.  Fortunately, we don't support online
resizing of bigalloc file systems, and for non-bigalloc file systems,
the cluster size == block size.  But fix this in case we do support
online resizing of bigalloc file systems in the future.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Reviewed-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/resize.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index a370ca1a2bd6..3ad2b1a900ad 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -154,8 +154,9 @@ static int verify_group_input(struct super_block *sb,
 
 	overhead = ext4_group_overhead_blocks(sb, group);
 	metaend = start + overhead;
-	input->free_clusters_count = free_blocks_count =
-		input->blocks_count - 2 - overhead - sbi->s_itb_per_group;
+	free_blocks_count = input->blocks_count - 2 - overhead -
+			    sbi->s_itb_per_group;
+	input->free_clusters_count = EXT4_B2C(sbi, free_blocks_count);
 
 	if (test_opt(sb, DEBUG))
 		printk(KERN_DEBUG "EXT4-fs: adding %s group %u: %u blocks "
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 09/13] ext4: remove EXT4FS_DEBUG defination in resize.c
  2023-08-26 17:46 [PATCH v3 00/13] fixes and cleanups to ext4 resize Kemeng Shi
                   ` (7 preceding siblings ...)
  2023-08-26 17:47 ` [PATCH v3 08/13] ext4: calculate free_clusters_count in cluster unit in verify_group_input Kemeng Shi
@ 2023-08-26 17:47 ` Kemeng Shi
  2023-08-26 17:47 ` [PATCH v3 10/13] ext4: use saved local variable sbi instead of EXT4_SB(sb) Kemeng Shi
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Kemeng Shi @ 2023-08-26 17:47 UTC (permalink / raw)
  To: tytso, adilger.kernel, linux-ext4, linux-kernel

Remove EXT4FS_DEBUG defination in resize.c for following reasons:
1. EXT4FS_DEBUG will enable debug messages, it should only be defined
when debugging.
2. ext4.h included from ext4_jbd2.h after EXT4FS_DEBUG defination will
"#undef EXT4FS_DEBUG", then EXT4FS_DEBUG defination in resize.c can't
actually turn on ext4_debug messages.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Reviewed-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/resize.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 3ad2b1a900ad..d4ab2cde0a03 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -10,8 +10,6 @@
  */
 
 
-#define EXT4FS_DEBUG
-
 #include <linux/errno.h>
 #include <linux/slab.h>
 #include <linux/jiffies.h>
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 10/13] ext4: use saved local variable sbi instead of EXT4_SB(sb)
  2023-08-26 17:46 [PATCH v3 00/13] fixes and cleanups to ext4 resize Kemeng Shi
                   ` (8 preceding siblings ...)
  2023-08-26 17:47 ` [PATCH v3 09/13] ext4: remove EXT4FS_DEBUG defination in resize.c Kemeng Shi
@ 2023-08-26 17:47 ` Kemeng Shi
  2023-08-26 17:47 ` [PATCH v3 11/13] ext4: simplify the gdbblock calculation in add_new_gdb_meta_bg Kemeng Shi
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Kemeng Shi @ 2023-08-26 17:47 UTC (permalink / raw)
  To: tytso, adilger.kernel, linux-ext4, linux-kernel

We save EXT4_SB(sb) to local variable sbi at beginning of function
ext4_resize_begin. Use sbi directly instead of EXT4_SB(sb) to
remove unnecessary pointer dereference.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Reviewed-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/resize.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index d4ab2cde0a03..e2c81b589de8 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -55,7 +55,7 @@ int ext4_resize_begin(struct super_block *sb)
 	 * If the reserved GDT blocks is non-zero, the resize_inode feature
 	 * should always be set.
 	 */
-	if (EXT4_SB(sb)->s_es->s_reserved_gdt_blocks &&
+	if (sbi->s_es->s_reserved_gdt_blocks &&
 	    !ext4_has_feature_resize_inode(sb)) {
 		ext4_error(sb, "resize_inode disabled but reserved GDT blocks non-zero");
 		return -EFSCORRUPTED;
@@ -67,9 +67,9 @@ int ext4_resize_begin(struct super_block *sb)
          * bad time to do it anyways.
          */
 	if (EXT4_B2C(sbi, sbi->s_sbh->b_blocknr) !=
-	    le32_to_cpu(EXT4_SB(sb)->s_es->s_first_data_block)) {
+	    le32_to_cpu(sbi->s_es->s_first_data_block)) {
 		ext4_warning(sb, "won't resize using backup superblock at %llu",
-			(unsigned long long)EXT4_SB(sb)->s_sbh->b_blocknr);
+			(unsigned long long)sbi->s_sbh->b_blocknr);
 		return -EPERM;
 	}
 
@@ -77,7 +77,7 @@ int ext4_resize_begin(struct super_block *sb)
 	 * We are not allowed to do online-resizing on a filesystem mounted
 	 * with error, because it can destroy the filesystem easily.
 	 */
-	if (EXT4_SB(sb)->s_mount_state & EXT4_ERROR_FS) {
+	if (sbi->s_mount_state & EXT4_ERROR_FS) {
 		ext4_warning(sb, "There are errors in the filesystem, "
 			     "so online resizing is not allowed");
 		return -EPERM;
@@ -89,7 +89,7 @@ int ext4_resize_begin(struct super_block *sb)
 	}
 
 	if (test_and_set_bit_lock(EXT4_FLAGS_RESIZING,
-				  &EXT4_SB(sb)->s_ext4_flags))
+				  &sbi->s_ext4_flags))
 		ret = -EBUSY;
 
 	return ret;
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 11/13] ext4: simplify the gdbblock calculation in add_new_gdb_meta_bg
  2023-08-26 17:46 [PATCH v3 00/13] fixes and cleanups to ext4 resize Kemeng Shi
                   ` (9 preceding siblings ...)
  2023-08-26 17:47 ` [PATCH v3 10/13] ext4: use saved local variable sbi instead of EXT4_SB(sb) Kemeng Shi
@ 2023-08-26 17:47 ` Kemeng Shi
  2023-08-26 17:47 ` [PATCH v3 12/13] ext4: remove unnecessary check to avoid repeat update_backups for the same gdb Kemeng Shi
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Kemeng Shi @ 2023-08-26 17:47 UTC (permalink / raw)
  To: tytso, adilger.kernel, linux-ext4, linux-kernel

We always call add_new_gdb_meta_bg with first group in mete_bg. Remove the
unnecessary ext4_meta_bg_first_group conversion to simplify the gdbblock
calculation.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 fs/ext4/resize.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index e2c81b589de8..5e9084ff131c 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -104,18 +104,6 @@ int ext4_resize_end(struct super_block *sb, bool update_backups)
 	return 0;
 }
 
-static ext4_group_t ext4_meta_bg_first_group(struct super_block *sb,
-					     ext4_group_t group) {
-	return (group >> EXT4_DESC_PER_BLOCK_BITS(sb)) <<
-	       EXT4_DESC_PER_BLOCK_BITS(sb);
-}
-
-static ext4_fsblk_t ext4_meta_bg_first_block_no(struct super_block *sb,
-					     ext4_group_t group) {
-	group = ext4_meta_bg_first_group(sb, group);
-	return ext4_group_first_block_no(sb, group);
-}
-
 static ext4_grpblk_t ext4_group_overhead_blocks(struct super_block *sb,
 						ext4_group_t group) {
 	ext4_grpblk_t overhead;
@@ -944,7 +932,13 @@ static int add_new_gdb(handle_t *handle, struct inode *inode,
 }
 
 /*
- * add_new_gdb_meta_bg is the sister of add_new_gdb.
+ * If there is no available space in the existing block group descriptors for
+ * the new block group and there are no reserved block group descriptors, then
+ * the meta_bg feature will get enabled, and es->s_first_meta_bg will get set
+ * to the first block group that is managed using meta_bg and s_first_meta_bg
+ * must be a multiple of EXT4_DESC_PER_BLOCK(sb).
+ * This function will be called when first group of meta_bg is added to bring
+ * new group descriptors block of new added meta_bg.
  */
 static int add_new_gdb_meta_bg(struct super_block *sb,
 			       handle_t *handle, ext4_group_t group) {
@@ -954,8 +948,8 @@ static int add_new_gdb_meta_bg(struct super_block *sb,
 	unsigned long gdb_num = group / EXT4_DESC_PER_BLOCK(sb);
 	int err;
 
-	gdblock = ext4_meta_bg_first_block_no(sb, group) +
-		   ext4_bg_has_super(sb, group);
+	gdblock = ext4_group_first_block_no(sb, group) +
+		  ext4_bg_has_super(sb, group);
 	gdb_bh = ext4_sb_bread(sb, gdblock, 0);
 	if (IS_ERR(gdb_bh))
 		return PTR_ERR(gdb_bh);
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 12/13] ext4: remove unnecessary check to avoid repeat update_backups for the same gdb
  2023-08-26 17:46 [PATCH v3 00/13] fixes and cleanups to ext4 resize Kemeng Shi
                   ` (10 preceding siblings ...)
  2023-08-26 17:47 ` [PATCH v3 11/13] ext4: simplify the gdbblock calculation in add_new_gdb_meta_bg Kemeng Shi
@ 2023-08-26 17:47 ` Kemeng Shi
  2023-08-26 17:47 ` [PATCH v3 13/13] ext4: remove unnecessary initialization of count2 in set_flexbg_block_bitmap Kemeng Shi
  2023-10-06 18:06 ` [PATCH v3 00/13] fixes and cleanups to ext4 resize Theodore Ts'o
  13 siblings, 0 replies; 15+ messages in thread
From: Kemeng Shi @ 2023-08-26 17:47 UTC (permalink / raw)
  To: tytso, adilger.kernel, linux-ext4, linux-kernel

The sbi->s_group_desc contains array of bh's for block group descriptors
and continuous EXT4_DESC_PER_BLOCK(sb) bg descriptors in single block
share the same bh.
Simply call update_backups for each gdb_bh in sbi->s_group_desc will not
update same group descriptors block for multiple times.

Commit 0acdb8876fead ("ext4: don't call update_backups() multiple times for
the same bg") wrongly assumed each block group descriptor in the same block
has a individual bh and unnecessary check was added.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 fs/ext4/ext4.h   | 1 +
 fs/ext4/resize.c | 4 ----
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 84618c46f239..37ac39711cd1 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1488,6 +1488,7 @@ struct ext4_sb_info {
 	loff_t s_bitmap_maxbytes;	/* max bytes for bitmap files */
 	struct buffer_head * s_sbh;	/* Buffer containing the super block */
 	struct ext4_super_block *s_es;	/* Pointer to the super block in the buffer */
+	/* Array of bh's for the block group descriptors */
 	struct buffer_head * __rcu *s_group_desc;
 	unsigned int s_mount_opt;
 	unsigned int s_mount_opt2;
diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 5e9084ff131c..718b47e131e4 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -1588,7 +1588,6 @@ static int ext4_flex_group_add(struct super_block *sb,
 		int meta_bg = ext4_has_feature_meta_bg(sb);
 		sector_t padding_blocks = meta_bg ? 0 : sbi->s_sbh->b_blocknr -
 					 ext4_group_first_block_no(sb, 0);
-		sector_t old_gdb = 0;
 
 		update_backups(sb, ext4_group_first_block_no(sb, 0),
 			       (char *)es, sizeof(struct ext4_super_block), 0);
@@ -1597,11 +1596,8 @@ static int ext4_flex_group_add(struct super_block *sb,
 
 			gdb_bh = sbi_array_rcu_deref(sbi, s_group_desc,
 						     gdb_num);
-			if (old_gdb == gdb_bh->b_blocknr)
-				continue;
 			update_backups(sb, gdb_bh->b_blocknr - padding_blocks,
 				       gdb_bh->b_data, gdb_bh->b_size, meta_bg);
-			old_gdb = gdb_bh->b_blocknr;
 		}
 	}
 exit:
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 13/13] ext4: remove unnecessary initialization of count2 in set_flexbg_block_bitmap
  2023-08-26 17:46 [PATCH v3 00/13] fixes and cleanups to ext4 resize Kemeng Shi
                   ` (11 preceding siblings ...)
  2023-08-26 17:47 ` [PATCH v3 12/13] ext4: remove unnecessary check to avoid repeat update_backups for the same gdb Kemeng Shi
@ 2023-08-26 17:47 ` Kemeng Shi
  2023-10-06 18:06 ` [PATCH v3 00/13] fixes and cleanups to ext4 resize Theodore Ts'o
  13 siblings, 0 replies; 15+ messages in thread
From: Kemeng Shi @ 2023-08-26 17:47 UTC (permalink / raw)
  To: tytso, adilger.kernel, linux-ext4, linux-kernel

We always overwrite count2 to "EXT4_CLUSTERS_PER_GROUP(sb) -
(first_cluster - start)" after its initialization in for loop
initialization statement .
Just remove unnecessary initialization of count2.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Reviewed-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/resize.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 718b47e131e4..4fe061edefdd 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -447,8 +447,7 @@ static int set_flexbg_block_bitmap(struct super_block *sb, handle_t *handle,
 
 	ext4_debug("mark clusters [%llu-%llu] used\n", first_cluster,
 		   last_cluster);
-	for (count2 = count; count > 0;
-	     count -= count2, first_cluster += count2) {
+	for (; count > 0; count -= count2, first_cluster += count2) {
 		ext4_fsblk_t start;
 		struct buffer_head *bh;
 		ext4_group_t group;
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 00/13] fixes and cleanups to ext4 resize
  2023-08-26 17:46 [PATCH v3 00/13] fixes and cleanups to ext4 resize Kemeng Shi
                   ` (12 preceding siblings ...)
  2023-08-26 17:47 ` [PATCH v3 13/13] ext4: remove unnecessary initialization of count2 in set_flexbg_block_bitmap Kemeng Shi
@ 2023-10-06 18:06 ` Theodore Ts'o
  13 siblings, 0 replies; 15+ messages in thread
From: Theodore Ts'o @ 2023-10-06 18:06 UTC (permalink / raw)
  To: adilger.kernel, linux-ext4, linux-kernel, Kemeng Shi; +Cc: Theodore Ts'o


On Sun, 27 Aug 2023 01:46:59 +0800, Kemeng Shi wrote:
> This series contains some random cleanups and a few fixes to correct gdb
> backup copy, fix buffer_head leak and so on. More details can be found
> in respective log messages.
> I run kvm-xfstest with config "ext4/all" and "-g auto" together with
> mballoc patchset I sent before.
> 
> v2->v3:
> -Remove unused ext4_meta_bg_first_group defination in patch 11
> 
> [...]

Applied, thanks!

[01/13] ext4: correct offset of gdb backup in non meta_bg group to update_backups
        commit: 31f13421c004a420c0e9d288859c9ea9259ea0cc
[02/13] ext4: add missed brelse in update_backups
        commit: 9adac8b01f4be28acd5838aade42b8daa4f0b642
[03/13] ext4: correct return value of ext4_convert_meta_bg
        commit: 48f1551592c54f7d8e2befc72a99ff4e47f7dca0
[04/13] ext4: remove gdb backup copy for meta bg in setup_new_flex_group_blocks
        commit: 40dd7953f4d606c280074f10d23046b6812708ce
[05/13] ext4: fix typo in setup_new_flex_group_blocks
        commit: e44fc921b84ff08a9e2fb827a146fa4021d016f3
[06/13] ext4: remove redundant check of count
        commit: 7d4cd3b45af025befe3bca94f87359a6603b6e95
[07/13] ext4: remove commented code in reserve_backup_gdb
        commit: 31458077273b5f883d99bee33a7fb295f155712d
[08/13] ext4: calculate free_clusters_count in cluster unit in verify_group_input
        commit: 1fc1bd2d18bbade157f7b14270f509ebbd89881b
[09/13] ext4: remove EXT4FS_DEBUG defination in resize.c
        commit: 95b635689b58e0ebe5197bf99c82c681eabe17ee
[10/13] ext4: use saved local variable sbi instead of EXT4_SB(sb)
        commit: 70cbfd257995b3f23c2408fd893cc18b61e58b4a
[11/13] ext4: simplify the gdbblock calculation in add_new_gdb_meta_bg
        commit: 9dca529bdaad7a7242a36d04f73cb998b817ab48
[12/13] ext4: remove unnecessary check to avoid repeat update_backups for the same gdb
        commit: 350bb48b84b8f4ad4ea179dbb97f568d12626188
[13/13] ext4: remove unnecessary initialization of count2 in set_flexbg_block_bitmap
        commit: 248b45b621a77155f81129e6b572ec833edb4cf4

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2023-10-06 18:07 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-26 17:46 [PATCH v3 00/13] fixes and cleanups to ext4 resize Kemeng Shi
2023-08-26 17:47 ` [PATCH v3 01/13] ext4: correct offset of gdb backup in non meta_bg group to update_backups Kemeng Shi
2023-08-26 17:47 ` [PATCH v3 02/13] ext4: add missed brelse in update_backups Kemeng Shi
2023-08-26 17:47 ` [PATCH v3 03/13] ext4: correct return value of ext4_convert_meta_bg Kemeng Shi
2023-08-26 17:47 ` [PATCH v3 04/13] ext4: remove gdb backup copy for meta bg in setup_new_flex_group_blocks Kemeng Shi
2023-08-26 17:47 ` [PATCH v3 05/13] ext4: fix typo " Kemeng Shi
2023-08-26 17:47 ` [PATCH v3 06/13] ext4: remove redundant check of count Kemeng Shi
2023-08-26 17:47 ` [PATCH v3 07/13] ext4: remove commented code in reserve_backup_gdb Kemeng Shi
2023-08-26 17:47 ` [PATCH v3 08/13] ext4: calculate free_clusters_count in cluster unit in verify_group_input Kemeng Shi
2023-08-26 17:47 ` [PATCH v3 09/13] ext4: remove EXT4FS_DEBUG defination in resize.c Kemeng Shi
2023-08-26 17:47 ` [PATCH v3 10/13] ext4: use saved local variable sbi instead of EXT4_SB(sb) Kemeng Shi
2023-08-26 17:47 ` [PATCH v3 11/13] ext4: simplify the gdbblock calculation in add_new_gdb_meta_bg Kemeng Shi
2023-08-26 17:47 ` [PATCH v3 12/13] ext4: remove unnecessary check to avoid repeat update_backups for the same gdb Kemeng Shi
2023-08-26 17:47 ` [PATCH v3 13/13] ext4: remove unnecessary initialization of count2 in set_flexbg_block_bitmap Kemeng Shi
2023-10-06 18:06 ` [PATCH v3 00/13] fixes and cleanups to ext4 resize 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).