Linux-ext4 Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/8] ext4: use ASSERT() to replace J_ASSERT()
@ 2020-10-16  3:55 Chunguang Xu
  2020-10-16  3:55 ` [PATCH 2/8] ext4: remove redundant mb_regenerate_buddy() Chunguang Xu
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Chunguang Xu @ 2020-10-16  3:55 UTC (permalink / raw)
  To: tytso, adilger.kernel; +Cc: linux-ext4

From: Chunguang Xu <brookxu@tencent.com>

There are currently multiple forms of assertion, such as J_ASSERT().
J_ASEERT is provided for the jbd module, which is a public module.
Maybe we should use custom ASSERT() like other file systems, such as
xfs, which would be better.

Signed-off-by: Chunguang Xu <brookxu@tencent.com>
---
 fs/ext4/balloc.c   |  2 +-
 fs/ext4/ext4.h     | 10 ++++++++++
 fs/ext4/fsync.c    |  2 +-
 fs/ext4/indirect.c |  4 ++--
 fs/ext4/inode.c    |  6 +++---
 fs/ext4/namei.c    | 12 ++++--------
 fs/ext4/super.c    |  2 +-
 7 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index 48c3df4..db7fa3e 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -185,7 +185,7 @@ static int ext4_init_block_bitmap(struct super_block *sb,
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
 	ext4_fsblk_t start, tmp;
 
-	J_ASSERT_BH(bh, buffer_locked(bh));
+	ASSERT(buffer_locked(bh));
 
 	/* If checksum is bad mark all blocks used to prevent allocation
 	 * essentially implementing a per-group read-only flag. */
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index b883a78..85d6900 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -99,6 +99,16 @@
 #define ext_debug(ino, fmt, ...)	no_printk(fmt, ##__VA_ARGS__)
 #endif
 
+#define ASSERT(assert)							\
+do {									\
+	if (unlikely(!(assert))) {					\
+		printk(KERN_EMERG					\
+		       "Assertion failure in %s() at %s:%d: \"%s\"\n",	\
+		       __func__, __FILE__, __LINE__, #assert);		\
+		BUG();							\
+	}								\
+} while (0)
+
 /* data type for block offset of block group */
 typedef int ext4_grpblk_t;
 
diff --git a/fs/ext4/fsync.c b/fs/ext4/fsync.c
index 1d668c8..b21394d 100644
--- a/fs/ext4/fsync.c
+++ b/fs/ext4/fsync.c
@@ -136,7 +136,7 @@ int ext4_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
 	if (unlikely(ext4_forced_shutdown(sbi)))
 		return -EIO;
 
-	J_ASSERT(ext4_journal_current_handle() == NULL);
+	ASSERT(ext4_journal_current_handle() == NULL);
 
 	trace_ext4_sync_file_enter(file, datasync);
 
diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
index 80c9f33..7442490 100644
--- a/fs/ext4/indirect.c
+++ b/fs/ext4/indirect.c
@@ -534,8 +534,8 @@ int ext4_ind_map_blocks(handle_t *handle, struct inode *inode,
 	ext4_fsblk_t first_block = 0;
 
 	trace_ext4_ind_map_blocks_enter(inode, map->m_lblk, map->m_len, flags);
-	J_ASSERT(!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)));
-	J_ASSERT(handle != NULL || (flags & EXT4_GET_BLOCKS_CREATE) == 0);
+	ASSERT(!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)));
+	ASSERT(handle != NULL || (flags & EXT4_GET_BLOCKS_CREATE) == 0);
 	depth = ext4_block_to_path(inode, map->m_lblk, offsets,
 				   &blocks_to_boundary);
 
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index bf59646..6c70d8c 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -825,7 +825,7 @@ struct buffer_head *ext4_getblk(handle_t *handle, struct inode *inode,
 	int create = map_flags & EXT4_GET_BLOCKS_CREATE;
 	int err;
 
-	J_ASSERT(handle != NULL || create == 0);
+	ASSERT(handle != NULL || create == 0);
 
 	map.m_lblk = block;
 	map.m_len = 1;
@@ -840,8 +840,8 @@ struct buffer_head *ext4_getblk(handle_t *handle, struct inode *inode,
 	if (unlikely(!bh))
 		return ERR_PTR(-ENOMEM);
 	if (map.m_flags & EXT4_MAP_NEW) {
-		J_ASSERT(create != 0);
-		J_ASSERT(handle != NULL);
+		ASSERT(create != 0);
+		ASSERT(handle != NULL);
 
 		/*
 		 * Now that we do not always journal data, we should
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 153a9fb..f2ca312 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -182,10 +182,6 @@ static struct buffer_head *__ext4_read_dirblock(struct inode *inode,
 	return bh;
 }
 
-#ifndef assert
-#define assert(test) J_ASSERT(test)
-#endif
-
 #ifdef DX_DEBUG
 #define dxtrace(command) command
 #else
@@ -850,7 +846,7 @@ struct stats dx_show_entries(struct dx_hash_info *hinfo, struct inode *dir,
 					break;
 				}
 			}
-			assert (at == p - 1);
+			ASSERT(at == p - 1);
 		}
 
 		at = p - 1;
@@ -1266,8 +1262,8 @@ static void dx_insert_block(struct dx_frame *frame, u32 hash, ext4_lblk_t block)
 	struct dx_entry *old = frame->at, *new = old + 1;
 	int count = dx_get_count(entries);
 
-	assert(count < dx_get_limit(entries));
-	assert(old < entries + count);
+	ASSERT(count < dx_get_limit(entries));
+	ASSERT(old < entries + count);
 	memmove(new + 1, new, (char *)(entries + count) - (char *)(new));
 	dx_set_hash(new, hash);
 	dx_set_block(new, block);
@@ -2960,7 +2956,7 @@ int ext4_orphan_add(handle_t *handle, struct inode *inode)
 	 * hold i_mutex, or the inode can not be referenced from outside,
 	 * so i_nlink should not be bumped due to race
 	 */
-	J_ASSERT((S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
+	ASSERT((S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
 		  S_ISLNK(inode->i_mode)) || inode->i_nlink == 0);
 
 	BUFFER_TRACE(sbi->s_sbh, "get_write_access");
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index ea425b4..12f0b5d 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1069,7 +1069,7 @@ static void ext4_put_super(struct super_block *sb)
 	 * in-memory list had better be clean by this point. */
 	if (!list_empty(&sbi->s_orphan))
 		dump_orphan_list(sb, sbi);
-	J_ASSERT(list_empty(&sbi->s_orphan));
+	ASSERT(list_empty(&sbi->s_orphan));
 
 	sync_blockdev(sb->s_bdev);
 	invalidate_bdev(sb->s_bdev);
-- 
1.8.3.1


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

* [PATCH 2/8] ext4: remove redundant mb_regenerate_buddy()
  2020-10-16  3:55 [PATCH 1/8] ext4: use ASSERT() to replace J_ASSERT() Chunguang Xu
@ 2020-10-16  3:55 ` Chunguang Xu
  2020-10-16  3:55 ` [PATCH 3/8] ext4: use do_div() to calculate block offset Chunguang Xu
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Chunguang Xu @ 2020-10-16  3:55 UTC (permalink / raw)
  To: tytso, adilger.kernel; +Cc: linux-ext4

From: Chunguang Xu <brookxu@tencent.com>

After this patch (163a203), if an abnormal bitmap is detected, we
will mark the group as corrupt, and we will not use this group in
the future. Therefore, it should be meaningless to regenerate the
buddy bitmap of this group, It might be better to delete it.

Signed-off-by: Chunguang Xu <brookxu@tencent.com>
---
 fs/ext4/mballoc.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 2705a4c..5b8ce76 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -822,24 +822,6 @@ void ext4_mb_generate_buddy(struct super_block *sb,
 	spin_unlock(&sbi->s_bal_lock);
 }
 
-static void mb_regenerate_buddy(struct ext4_buddy *e4b)
-{
-	int count;
-	int order = 1;
-	void *buddy;
-
-	while ((buddy = mb_find_buddy(e4b, order++, &count))) {
-		ext4_set_bits(buddy, 0, count);
-	}
-	e4b->bd_info->bb_fragments = 0;
-	memset(e4b->bd_info->bb_counters, 0,
-		sizeof(*e4b->bd_info->bb_counters) *
-		(e4b->bd_sb->s_blocksize_bits + 2));
-
-	ext4_mb_generate_buddy(e4b->bd_sb, e4b->bd_buddy,
-		e4b->bd_bitmap, e4b->bd_group);
-}
-
 /* The buddy information is attached the buddy cache inode
  * for convenience. The information regarding each group
  * is loaded via ext4_mb_load_buddy. The information involve
@@ -1510,7 +1492,6 @@ static void mb_free_blocks(struct inode *inode, struct ext4_buddy *e4b,
 				      block);
 		ext4_mark_group_bitmap_corrupted(sb, e4b->bd_group,
 				EXT4_GROUP_INFO_BBITMAP_CORRUPT);
-		mb_regenerate_buddy(e4b);
 		goto done;
 	}
 
-- 
1.8.3.1


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

* [PATCH 3/8] ext4: use do_div() to calculate block offset
  2020-10-16  3:55 [PATCH 1/8] ext4: use ASSERT() to replace J_ASSERT() Chunguang Xu
  2020-10-16  3:55 ` [PATCH 2/8] ext4: remove redundant mb_regenerate_buddy() Chunguang Xu
@ 2020-10-16  3:55 ` Chunguang Xu
  2020-10-17  2:23   ` kernel test robot
  2020-10-16  3:55 ` [PATCH 4/8] ext4: simplify the code of mb_find_order_for_block Chunguang Xu
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 9+ messages in thread
From: Chunguang Xu @ 2020-10-16  3:55 UTC (permalink / raw)
  To: tytso, adilger.kernel; +Cc: linux-ext4

From: Chunguang Xu <brookxu@tencent.com>

Use do_div() to calculate the block offset and the offset
within the block, make the code more concise.

Signed-off-by: Chunguang Xu <brookxu@tencent.com>
---
 fs/ext4/balloc.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index db7fa3e..4013676 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -265,7 +265,7 @@ struct ext4_group_desc * ext4_get_group_desc(struct super_block *sb,
 					     ext4_group_t block_group,
 					     struct buffer_head **bh)
 {
-	unsigned int group_desc;
+	unsigned int group_desc = block_group;
 	unsigned int offset;
 	ext4_group_t ngroups = ext4_get_groups_count(sb);
 	struct ext4_group_desc *desc;
@@ -279,8 +279,7 @@ struct ext4_group_desc * ext4_get_group_desc(struct super_block *sb,
 		return NULL;
 	}
 
-	group_desc = block_group >> EXT4_DESC_PER_BLOCK_BITS(sb);
-	offset = block_group & (EXT4_DESC_PER_BLOCK(sb) - 1);
+	offset = do_div(group_desc, EXT4_DESC_PER_BLOCK(sb));
 	bh_p = sbi_array_rcu_deref(sbi, s_group_desc, group_desc);
 	/*
 	 * sbi_array_rcu_deref returns with rcu unlocked, this is ok since
-- 
1.8.3.1


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

* [PATCH 4/8] ext4: simplify the code of mb_find_order_for_block
  2020-10-16  3:55 [PATCH 1/8] ext4: use ASSERT() to replace J_ASSERT() Chunguang Xu
  2020-10-16  3:55 ` [PATCH 2/8] ext4: remove redundant mb_regenerate_buddy() Chunguang Xu
  2020-10-16  3:55 ` [PATCH 3/8] ext4: use do_div() to calculate block offset Chunguang Xu
@ 2020-10-16  3:55 ` Chunguang Xu
  2020-10-16  3:55 ` [PATCH 5/8] ext4: add the gdt block of meta_bg to system_zone Chunguang Xu
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Chunguang Xu @ 2020-10-16  3:55 UTC (permalink / raw)
  To: tytso, adilger.kernel; +Cc: linux-ext4

From: Chunguang Xu <brookxu@tencent.com>

The code of mb_find_order_for_block is a bit obscure, but we can
simplify it with mb_find_buddy(), make the code more concise.

Signed-off-by: Chunguang Xu <brookxu@tencent.com>
---
 fs/ext4/mballoc.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 5b8ce76..2eead37 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -1289,22 +1289,18 @@ static void ext4_mb_unload_buddy(struct ext4_buddy *e4b)
 
 static int mb_find_order_for_block(struct ext4_buddy *e4b, int block)
 {
-	int order = 1;
-	int bb_incr = 1 << (e4b->bd_blkbits - 1);
+	int order = 1, max;
 	void *bb;
 
 	BUG_ON(e4b->bd_bitmap == e4b->bd_buddy);
 	BUG_ON(block >= (1 << (e4b->bd_blkbits + 3)));
 
-	bb = e4b->bd_buddy;
 	while (order <= e4b->bd_blkbits + 1) {
-		block = block >> 1;
-		if (!mb_test_bit(block, bb)) {
+		bb = mb_find_buddy(e4b, order, &max);
+		if (!mb_test_bit(block >> order, bb)) {
 			/* this block is part of buddy of order 'order' */
 			return order;
 		}
-		bb += bb_incr;
-		bb_incr >>= 1;
 		order++;
 	}
 	return 0;
-- 
1.8.3.1


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

* [PATCH 5/8] ext4: add the gdt block of meta_bg to system_zone
  2020-10-16  3:55 [PATCH 1/8] ext4: use ASSERT() to replace J_ASSERT() Chunguang Xu
                   ` (2 preceding siblings ...)
  2020-10-16  3:55 ` [PATCH 4/8] ext4: simplify the code of mb_find_order_for_block Chunguang Xu
@ 2020-10-16  3:55 ` Chunguang Xu
  2020-10-16  3:55 ` [PATCH 6/8] ext4: update ext4_data_block_valid related comments Chunguang Xu
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Chunguang Xu @ 2020-10-16  3:55 UTC (permalink / raw)
  To: tytso, adilger.kernel; +Cc: linux-ext4

From: Chunguang Xu <brookxu@tencent.com>

In order to avoid poor search efficiency of system_zone, the
system only adds metadata of some sparse group to system_zone.
In the meta_bg scenario, the non-sparse group may contain gdt
blocks. Perhaps we should add these blocks to system_zone to
improve fault tolerance without significantly reducing system
performance.

Signed-off-by: Chunguang Xu <brookxu@tencent.com>
---
 fs/ext4/block_validity.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/fs/ext4/block_validity.c b/fs/ext4/block_validity.c
index c54ba52..5e98ca0 100644
--- a/fs/ext4/block_validity.c
+++ b/fs/ext4/block_validity.c
@@ -218,6 +218,7 @@ int ext4_setup_system_zone(struct super_block *sb)
 	struct ext4_group_desc *gdp;
 	ext4_group_t i;
 	int flex_size = ext4_flex_bg_size(sbi);
+	int gd_blks;
 	int ret;
 
 	system_blks = kzalloc(sizeof(*system_blks), GFP_KERNEL);
@@ -226,13 +227,16 @@ int ext4_setup_system_zone(struct super_block *sb)
 
 	for (i=0; i < ngroups; i++) {
 		cond_resched();
-		if (ext4_bg_has_super(sb, i) &&
-		    ((i < 5) || ((i % flex_size) == 0))) {
-			ret = add_system_zone(system_blks,
-					ext4_group_first_block_no(sb, i),
-					ext4_bg_num_gdb(sb, i) + 1, 0);
-			if (ret)
-				goto err;
+		if ((i < 5) || ((i % flex_size) == 0)) {
+			gd_blks = ext4_bg_has_super(sb, i) +
+				ext4_bg_num_gdb(sb, i);
+			if (gd_blks) {
+				ret = add_system_zone(system_blks,
+						ext4_group_first_block_no(sb, i),
+						gd_blks, 0);
+				if (ret)
+					goto err;
+			}
 		}
 		gdp = ext4_get_group_desc(sb, i, NULL);
 		ret = add_system_zone(system_blks,
-- 
1.8.3.1


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

* [PATCH 6/8] ext4: update ext4_data_block_valid related comments
  2020-10-16  3:55 [PATCH 1/8] ext4: use ASSERT() to replace J_ASSERT() Chunguang Xu
                   ` (3 preceding siblings ...)
  2020-10-16  3:55 ` [PATCH 5/8] ext4: add the gdt block of meta_bg to system_zone Chunguang Xu
@ 2020-10-16  3:55 ` Chunguang Xu
  2020-10-16  3:55 ` [PATCH 7/8] ext4: add a helper function to validate metadata block Chunguang Xu
  2020-10-16  3:55 ` [PATCH 8/8] ext4: delete invalid code inside ext4_xattr_block_set() Chunguang Xu
  6 siblings, 0 replies; 9+ messages in thread
From: Chunguang Xu @ 2020-10-16  3:55 UTC (permalink / raw)
  To: tytso, adilger.kernel; +Cc: linux-ext4

From: Chunguang Xu <brookxu@tencent.com>

Since ext4_data_block_valid() has been renamed to ext4_inode_block_valid(),
the related comments need to be updated.

Signed-off-by: Chunguang Xu <brookxu@tencent.com>
---
 fs/ext4/block_validity.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/block_validity.c b/fs/ext4/block_validity.c
index 5e98ca0..03b237f 100644
--- a/fs/ext4/block_validity.c
+++ b/fs/ext4/block_validity.c
@@ -206,7 +206,7 @@ static void ext4_destroy_system_zone(struct rcu_head *rcu)
  *
  * The update of system_blks pointer in this function is protected by
  * sb->s_umount semaphore. However we have to be careful as we can be
- * racing with ext4_data_block_valid() calls reading system_blks rbtree
+ * racing with ext4_inode_block_valid() calls reading system_blks rbtree
  * protected only by RCU. That's why we first build the rbtree and then
  * swap it in place.
  */
@@ -262,7 +262,7 @@ int ext4_setup_system_zone(struct super_block *sb)
 
 	/*
 	 * System blks rbtree complete, announce it once to prevent racing
-	 * with ext4_data_block_valid() accessing the rbtree at the same
+	 * with ext4_inode_block_valid() accessing the rbtree at the same
 	 * time.
 	 */
 	rcu_assign_pointer(sbi->system_blks, system_blks);
@@ -282,7 +282,7 @@ int ext4_setup_system_zone(struct super_block *sb)
  *
  * The update of system_blks pointer in this function is protected by
  * sb->s_umount semaphore. However we have to be careful as we can be
- * racing with ext4_data_block_valid() calls reading system_blks rbtree
+ * racing with ext4_inode_block_valid() calls reading system_blks rbtree
  * protected only by RCU. So we first clear the system_blks pointer and
  * then free the rbtree only after RCU grace period expires.
  */
-- 
1.8.3.1


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

* [PATCH 7/8] ext4: add a helper function to validate metadata block
  2020-10-16  3:55 [PATCH 1/8] ext4: use ASSERT() to replace J_ASSERT() Chunguang Xu
                   ` (4 preceding siblings ...)
  2020-10-16  3:55 ` [PATCH 6/8] ext4: update ext4_data_block_valid related comments Chunguang Xu
@ 2020-10-16  3:55 ` Chunguang Xu
  2020-10-16  3:55 ` [PATCH 8/8] ext4: delete invalid code inside ext4_xattr_block_set() Chunguang Xu
  6 siblings, 0 replies; 9+ messages in thread
From: Chunguang Xu @ 2020-10-16  3:55 UTC (permalink / raw)
  To: tytso, adilger.kernel; +Cc: linux-ext4

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.

Signed-off-by: Chunguang Xu <brookxu@tencent.com>
---
 fs/ext4/ext4.h    |  5 ++++-
 fs/ext4/mballoc.c | 57 +++++++++++++++++++++++++++++++++++++++++++------------
 2 files changed, 49 insertions(+), 13 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 85d6900..42be90e 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2702,7 +2702,10 @@ extern ext4_group_t ext4_mb_prefetch(struct super_block *sb,
 				     unsigned int nr, int *cnt);
 extern void ext4_mb_prefetch_fini(struct super_block *sb, ext4_group_t group,
 				  unsigned int nr);
-
+extern int ext4_metadata_block_overlaps(struct super_block *sb,
+					ext4_group_t block_group,
+					ext4_fsblk_t block,
+					unsigned long count);
 extern void ext4_free_blocks(handle_t *handle, struct inode *inode,
 			     struct buffer_head *bh, ext4_fsblk_t block,
 			     unsigned long count, int flags);
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 2eead37..e8df64d 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -5049,6 +5049,49 @@ static void ext4_try_merge_freed_extent(struct ext4_sb_info *sbi,
 	return 0;
 }
 
+/*
+ * 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))
+		ret = 1;
+
+	// check inode table
+	itable = ext4_inode_table(sb, gdp);
+	if (!(block >= itable + sbi->s_itb_per_group ||
+	      block + count - 1  < itable))
+		ret = 1;
+
+	/* check super_block and block group descriptor table, the
+	 * reserved space of the block group descriptor is managed
+	 * by resize_inode, it may not be processed now due to
+	 * performance.
+	 */
+	gd_blk = ext4_bg_has_super(sb, block_group) +
+		ext4_bg_num_gdb(sb, block_group);
+	if (gd_blk) {
+		if (!(block >= gd_first + gd_blk ||
+		      block + count - 1 < gd_first))
+			ret = 1;
+	}
+	return ret;
+}
+
 /**
  * ext4_free_blocks() -- Free given blocks and update quota
  * @handle:		handle for this transaction
@@ -5175,13 +5218,7 @@ void ext4_free_blocks(handle_t *handle, struct inode *inode,
 		goto error_return;
 	}
 
-	if (in_range(ext4_block_bitmap(sb, gdp), block, count) ||
-	    in_range(ext4_inode_bitmap(sb, gdp), block, count) ||
-	    in_range(block, ext4_inode_table(sb, gdp),
-		     sbi->s_itb_per_group) ||
-	    in_range(block + count - 1, ext4_inode_table(sb, gdp),
-		     sbi->s_itb_per_group)) {
-
+	if (ext4_metadata_block_overlaps(sb, block_group, block, count)) {
 		ext4_error(sb, "Freeing blocks in system zone - "
 			   "Block = %llu, count = %lu", block, count);
 		/* err = 0. ext4_std_error should be a no op */
@@ -5367,11 +5404,7 @@ int ext4_group_add_blocks(handle_t *handle, struct super_block *sb,
 		goto error_return;
 	}
 
-	if (in_range(ext4_block_bitmap(sb, desc), block, count) ||
-	    in_range(ext4_inode_bitmap(sb, desc), block, count) ||
-	    in_range(block, ext4_inode_table(sb, desc), sbi->s_itb_per_group) ||
-	    in_range(block + count - 1, ext4_inode_table(sb, desc),
-		     sbi->s_itb_per_group)) {
+	if (ext4_metadata_block_overlaps(sb, block_group, block, count)) {
 		ext4_error(sb, "Adding blocks in system zones - "
 			   "Block = %llu, count = %lu",
 			   block, count);
-- 
1.8.3.1


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

* [PATCH 8/8] ext4: delete invalid code inside ext4_xattr_block_set()
  2020-10-16  3:55 [PATCH 1/8] ext4: use ASSERT() to replace J_ASSERT() Chunguang Xu
                   ` (5 preceding siblings ...)
  2020-10-16  3:55 ` [PATCH 7/8] ext4: add a helper function to validate metadata block Chunguang Xu
@ 2020-10-16  3:55 ` Chunguang Xu
  6 siblings, 0 replies; 9+ messages in thread
From: Chunguang Xu @ 2020-10-16  3:55 UTC (permalink / raw)
  To: tytso, adilger.kernel; +Cc: linux-ext4

From: Chunguang Xu <brookxu@tencent.com>

Delete invalid code inside ext4_xattr_block_set().

Signed-off-by: Chunguang Xu <brookxu@tencent.com>
---
 fs/ext4/xattr.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index cba4b87..56728f3 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1927,7 +1927,6 @@ struct ext4_xattr_block_find {
 	} else {
 		/* Allocate a buffer where we construct the new block. */
 		s->base = kzalloc(sb->s_blocksize, GFP_NOFS);
-		/* assert(header == s->base) */
 		error = -ENOMEM;
 		if (s->base == NULL)
 			goto cleanup;
-- 
1.8.3.1


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

* Re: [PATCH 3/8] ext4: use do_div() to calculate block offset
  2020-10-16  3:55 ` [PATCH 3/8] ext4: use do_div() to calculate block offset Chunguang Xu
@ 2020-10-17  2:23   ` kernel test robot
  0 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2020-10-17  2:23 UTC (permalink / raw)
  To: Chunguang Xu, tytso, adilger.kernel; +Cc: kbuild-all, linux-ext4


[-- Attachment #1: Type: text/plain, Size: 3584 bytes --]

Hi Chunguang,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v5.9]
[also build test ERROR on next-20201016]
[cannot apply to ext4/dev tytso-fscrypt/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Chunguang-Xu/ext4-use-ASSERT-to-replace-J_ASSERT/20201016-115723
base:    bbf5c979011a099af5dc76498918ed7df445635b
config: arm-cerfcube_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/1bcce45cce439ec86a89aa8cfc895b5e9ad5046b
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Chunguang-Xu/ext4-use-ASSERT-to-replace-J_ASSERT/20201016-115723
        git checkout 1bcce45cce439ec86a89aa8cfc895b5e9ad5046b
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/linux/math64.h:7,
                    from include/linux/time.h:6,
                    from fs/ext4/balloc.c:15:
   fs/ext4/balloc.c: In function 'ext4_get_group_desc':
>> arch/arm/include/asm/div64.h:60:36: error: passing argument 1 of '__div64_32' from incompatible pointer type [-Werror=incompatible-pointer-types]
      60 | #define do_div(n, base) __div64_32(&(n), base)
         |                                    ^~~~
         |                                    |
         |                                    unsigned int *
   fs/ext4/balloc.c:282:11: note: in expansion of macro 'do_div'
     282 |  offset = do_div(group_desc, EXT4_DESC_PER_BLOCK(sb));
         |           ^~~~~~
   arch/arm/include/asm/div64.h:33:45: note: expected 'uint64_t *' {aka 'long long unsigned int *'} but argument is of type 'unsigned int *'
      33 | static inline uint32_t __div64_32(uint64_t *n, uint32_t base)
         |                                   ~~~~~~~~~~^
   cc1: some warnings being treated as errors

vim +/__div64_32 +60 arch/arm/include/asm/div64.h

fa4adc614922c24 include/asm-arm/div64.h      Nicolas Pitre 2006-12-06  53  
fa4adc614922c24 include/asm-arm/div64.h      Nicolas Pitre 2006-12-06  54  /*
040b323b5012b55 arch/arm/include/asm/div64.h Nicolas Pitre 2015-11-02  55   * In OABI configurations, some uses of the do_div function
040b323b5012b55 arch/arm/include/asm/div64.h Nicolas Pitre 2015-11-02  56   * cause gcc to run out of registers. To work around that,
040b323b5012b55 arch/arm/include/asm/div64.h Nicolas Pitre 2015-11-02  57   * we can force the use of the out-of-line version for
040b323b5012b55 arch/arm/include/asm/div64.h Nicolas Pitre 2015-11-02  58   * configurations that build a OABI kernel.
fa4adc614922c24 include/asm-arm/div64.h      Nicolas Pitre 2006-12-06  59   */
040b323b5012b55 arch/arm/include/asm/div64.h Nicolas Pitre 2015-11-02 @60  #define do_div(n, base) __div64_32(&(n), base)
fa4adc614922c24 include/asm-arm/div64.h      Nicolas Pitre 2006-12-06  61  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 13770 bytes --]

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

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-16  3:55 [PATCH 1/8] ext4: use ASSERT() to replace J_ASSERT() Chunguang Xu
2020-10-16  3:55 ` [PATCH 2/8] ext4: remove redundant mb_regenerate_buddy() Chunguang Xu
2020-10-16  3:55 ` [PATCH 3/8] ext4: use do_div() to calculate block offset Chunguang Xu
2020-10-17  2:23   ` kernel test robot
2020-10-16  3:55 ` [PATCH 4/8] ext4: simplify the code of mb_find_order_for_block Chunguang Xu
2020-10-16  3:55 ` [PATCH 5/8] ext4: add the gdt block of meta_bg to system_zone Chunguang Xu
2020-10-16  3:55 ` [PATCH 6/8] ext4: update ext4_data_block_valid related comments Chunguang Xu
2020-10-16  3:55 ` [PATCH 7/8] ext4: add a helper function to validate metadata block Chunguang Xu
2020-10-16  3:55 ` [PATCH 8/8] ext4: delete invalid code inside ext4_xattr_block_set() Chunguang Xu

Linux-ext4 Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-ext4/0 linux-ext4/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-ext4 linux-ext4/ https://lore.kernel.org/linux-ext4 \
		linux-ext4@vger.kernel.org
	public-inbox-index linux-ext4

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-ext4


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git