| Message ID | 20221115121638.192349-4-libaokun1@huawei.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fix some bugs in online resize | expand |
Hi Baokun,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on tytso-ext4/dev]
[also build test WARNING on linus/master v6.1-rc5 next-20221115]
[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#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Baokun-Li/fix-some-bugs-in-online-resize/20221115-195625
base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link: https://lore.kernel.org/r/20221115121638.192349-4-libaokun1%40huawei.com
patch subject: [PATCH 3/3] ext4: fix corruption when online resizing a 1K bigalloc fs
config: m68k-randconfig-s053-20221115
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# https://github.com/intel-lab-lkp/linux/commit/72fcaa65f215267f43525ac8cfe4ddb563c9db34
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Baokun-Li/fix-some-bugs-in-online-resize/20221115-195625
git checkout 72fcaa65f215267f43525ac8cfe4ddb563c9db34
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=m68k SHELL=/bin/bash fs/ext4/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> fs/ext4/resize.c:1594:38: sparse: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned long long [usertype] blk_off @@ got restricted __le32 [usertype] s_first_data_block @@
fs/ext4/resize.c:1594:38: sparse: expected unsigned long long [usertype] blk_off
fs/ext4/resize.c:1594:38: sparse: got restricted __le32 [usertype] s_first_data_block
fs/ext4/resize.c:1806:38: sparse: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned long long [usertype] blk_off @@ got restricted __le32 [usertype] s_first_data_block @@
fs/ext4/resize.c:1806:38: sparse: expected unsigned long long [usertype] blk_off
fs/ext4/resize.c:1806:38: sparse: got restricted __le32 [usertype] s_first_data_block
vim +1594 fs/ext4/resize.c
1516
1517 /* Add a flex group to an fs. Ensure we handle all possible error conditions
1518 * _before_ we start modifying the filesystem, because we cannot abort the
1519 * transaction and not have it write the data to disk.
1520 */
1521 static int ext4_flex_group_add(struct super_block *sb,
1522 struct inode *resize_inode,
1523 struct ext4_new_flex_group_data *flex_gd)
1524 {
1525 struct ext4_sb_info *sbi = EXT4_SB(sb);
1526 struct ext4_super_block *es = sbi->s_es;
1527 ext4_fsblk_t o_blocks_count;
1528 ext4_grpblk_t last;
1529 ext4_group_t group;
1530 handle_t *handle;
1531 unsigned reserved_gdb;
1532 int err = 0, err2 = 0, credit;
1533
1534 BUG_ON(!flex_gd->count || !flex_gd->groups || !flex_gd->bg_flags);
1535
1536 reserved_gdb = le16_to_cpu(es->s_reserved_gdt_blocks);
1537 o_blocks_count = ext4_blocks_count(es);
1538 ext4_get_group_no_and_offset(sb, o_blocks_count, &group, &last);
1539 BUG_ON(last);
1540
1541 err = setup_new_flex_group_blocks(sb, flex_gd);
1542 if (err)
1543 goto exit;
1544 /*
1545 * We will always be modifying at least the superblock and GDT
1546 * blocks. If we are adding a group past the last current GDT block,
1547 * we will also modify the inode and the dindirect block. If we
1548 * are adding a group with superblock/GDT backups we will also
1549 * modify each of the reserved GDT dindirect blocks.
1550 */
1551 credit = 3; /* sb, resize inode, resize inode dindirect */
1552 /* GDT blocks */
1553 credit += 1 + DIV_ROUND_UP(flex_gd->count, EXT4_DESC_PER_BLOCK(sb));
1554 credit += reserved_gdb; /* Reserved GDT dindirect blocks */
1555 handle = ext4_journal_start_sb(sb, EXT4_HT_RESIZE, credit);
1556 if (IS_ERR(handle)) {
1557 err = PTR_ERR(handle);
1558 goto exit;
1559 }
1560
1561 BUFFER_TRACE(sbi->s_sbh, "get_write_access");
1562 err = ext4_journal_get_write_access(handle, sb, sbi->s_sbh,
1563 EXT4_JTR_NONE);
1564 if (err)
1565 goto exit_journal;
1566
1567 group = flex_gd->groups[0].group;
1568 BUG_ON(group != sbi->s_groups_count);
1569 err = ext4_add_new_descs(handle, sb, group,
1570 resize_inode, flex_gd->count);
1571 if (err)
1572 goto exit_journal;
1573
1574 err = ext4_setup_new_descs(handle, sb, flex_gd);
1575 if (err)
1576 goto exit_journal;
1577
1578 ext4_update_super(sb, flex_gd);
1579
1580 err = ext4_handle_dirty_metadata(handle, NULL, sbi->s_sbh);
1581
1582 exit_journal:
1583 err2 = ext4_journal_stop(handle);
1584 if (!err)
1585 err = err2;
1586
1587 if (!err) {
1588 int gdb_num = group / EXT4_DESC_PER_BLOCK(sb);
1589 int gdb_num_end = ((group + flex_gd->count - 1) /
1590 EXT4_DESC_PER_BLOCK(sb));
1591 int meta_bg = ext4_has_feature_meta_bg(sb);
1592 sector_t old_gdb = 0;
1593
> 1594 update_backups(sb, es->s_first_data_block, (char *)es,
1595 sizeof(struct ext4_super_block), 0);
1596 for (; gdb_num <= gdb_num_end; gdb_num++) {
1597 struct buffer_head *gdb_bh;
1598
1599 gdb_bh = sbi_array_rcu_deref(sbi, s_group_desc,
1600 gdb_num);
1601 if (old_gdb == gdb_bh->b_blocknr)
1602 continue;
1603 update_backups(sb, gdb_bh->b_blocknr, gdb_bh->b_data,
1604 gdb_bh->b_size, meta_bg);
1605 old_gdb = gdb_bh->b_blocknr;
1606 }
1607 }
1608 exit:
1609 return err;
1610 }
1611
diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c index 32fbfc173571..cf75fdd3729d 100644 --- a/fs/ext4/resize.c +++ b/fs/ext4/resize.c @@ -1591,7 +1591,7 @@ static int ext4_flex_group_add(struct super_block *sb, int meta_bg = ext4_has_feature_meta_bg(sb); sector_t old_gdb = 0; - update_backups(sb, sbi->s_sbh->b_blocknr, (char *)es, + update_backups(sb, es->s_first_data_block, (char *)es, sizeof(struct ext4_super_block), 0); for (; gdb_num <= gdb_num_end; gdb_num++) { struct buffer_head *gdb_bh; @@ -1803,8 +1803,8 @@ static int ext4_group_extend_no_check(struct super_block *sb, if (test_opt(sb, DEBUG)) printk(KERN_DEBUG "EXT4-fs: extended group to %llu " "blocks\n", ext4_blocks_count(es)); - update_backups(sb, EXT4_SB(sb)->s_sbh->b_blocknr, - (char *)es, sizeof(struct ext4_super_block), 0); + update_backups(sb, es->s_first_data_block, (char *)es, + sizeof(struct ext4_super_block), 0); } return err; }
When a backup superblock is updated in update_backups(), the primary superblock's offset in the group (that is, sbi->s_sbh->b_blocknr) is used as the backup superblock's offset in its group. However, when the block size is 1K and bigalloc is enabled, the two offsets are not equal. This causes the backup group descriptors to be overwritten by the superblock in update_backups(). Moreover, if meta_bg is enabled, the file system will be corrupted because this feature uses backup group descriptors. To solve this issue, we use a more accurate s_first_data_block as the offset of the backup superblock in its group. Fixes: d77147ff443b ("ext4: add support for online resizing with bigalloc") Signed-off-by: Baokun Li <libaokun1@huawei.com> --- fs/ext4/resize.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)