From 525b861a008143048535011f3816d407940f4bfa Mon Sep 17 00:00:00 2001 From: Juntong Deng Date: Mon, 2 Oct 2023 17:56:58 +0800 Subject: [PATCH 01/69] fs/jfs: Add check for negative db_l2nbperpage l2nbperpage is log2(number of blks per page), and the minimum legal value should be 0, not negative. In the case of l2nbperpage being negative, an error will occur when subsequently used as shift exponent. Syzbot reported this bug: UBSAN: shift-out-of-bounds in fs/jfs/jfs_dmap.c:799:12 shift exponent -16777216 is negative Reported-by: syzbot+debee9ab7ae2b34b0307@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=debee9ab7ae2b34b0307 Signed-off-by: Juntong Deng Signed-off-by: Dave Kleikamp --- fs/jfs/jfs_dmap.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c index 88afd108c2dd..3a1842348112 100644 --- a/fs/jfs/jfs_dmap.c +++ b/fs/jfs/jfs_dmap.c @@ -180,7 +180,8 @@ int dbMount(struct inode *ipbmap) bmp->db_nfree = le64_to_cpu(dbmp_le->dn_nfree); bmp->db_l2nbperpage = le32_to_cpu(dbmp_le->dn_l2nbperpage); - if (bmp->db_l2nbperpage > L2PSIZE - L2MINBLOCKSIZE) { + if (bmp->db_l2nbperpage > L2PSIZE - L2MINBLOCKSIZE || + bmp->db_l2nbperpage < 0) { err = -EINVAL; goto err_release_metapage; } From 64933ab7b04881c6c18b21ff206c12278341c72e Mon Sep 17 00:00:00 2001 From: Juntong Deng Date: Wed, 4 Oct 2023 02:06:41 +0800 Subject: [PATCH 02/69] fs/jfs: Add validity check for db_maxag and db_agpref Both db_maxag and db_agpref are used as the index of the db_agfree array, but there is currently no validity check for db_maxag and db_agpref, which can lead to errors. The following is related bug reported by Syzbot: UBSAN: array-index-out-of-bounds in fs/jfs/jfs_dmap.c:639:20 index 7936 is out of range for type 'atomic_t[128]' Add checking that the values of db_maxag and db_agpref are valid indexes for the db_agfree array. Reported-by: syzbot+38e876a8aa44b7115c76@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=38e876a8aa44b7115c76 Signed-off-by: Juntong Deng Signed-off-by: Dave Kleikamp --- fs/jfs/jfs_dmap.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c index 3a1842348112..4d59373f9e6c 100644 --- a/fs/jfs/jfs_dmap.c +++ b/fs/jfs/jfs_dmap.c @@ -195,6 +195,12 @@ int dbMount(struct inode *ipbmap) bmp->db_maxlevel = le32_to_cpu(dbmp_le->dn_maxlevel); bmp->db_maxag = le32_to_cpu(dbmp_le->dn_maxag); bmp->db_agpref = le32_to_cpu(dbmp_le->dn_agpref); + if (bmp->db_maxag >= MAXAG || bmp->db_maxag < 0 || + bmp->db_agpref >= MAXAG || bmp->db_agpref < 0) { + err = -EINVAL; + goto err_release_metapage; + } + bmp->db_aglevel = le32_to_cpu(dbmp_le->dn_aglevel); bmp->db_agheight = le32_to_cpu(dbmp_le->dn_agheight); bmp->db_agwidth = le32_to_cpu(dbmp_le->dn_agwidth); From 22cad8bc1d36547cdae0eef316c47d917ce3147c Mon Sep 17 00:00:00 2001 From: Manas Ghandat Date: Wed, 4 Oct 2023 11:17:18 +0530 Subject: [PATCH 03/69] jfs: fix array-index-out-of-bounds in dbFindLeaf Currently while searching for dmtree_t for sufficient free blocks there is an array out of bounds while getting element in tp->dm_stree. To add the required check for out of bound we first need to determine the type of dmtree. Thus added an extra parameter to dbFindLeaf so that the type of tree can be determined and the required check can be applied. Reported-by: syzbot+aea1ad91e854d0a83e04@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=aea1ad91e854d0a83e04 Signed-off-by: Manas Ghandat Signed-off-by: Dave Kleikamp --- fs/jfs/jfs_dmap.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c index 4d59373f9e6c..11c77757ead9 100644 --- a/fs/jfs/jfs_dmap.c +++ b/fs/jfs/jfs_dmap.c @@ -87,7 +87,7 @@ static int dbAllocCtl(struct bmap * bmp, s64 nblocks, int l2nb, s64 blkno, static int dbExtend(struct inode *ip, s64 blkno, s64 nblocks, s64 addnblocks); static int dbFindBits(u32 word, int l2nb); static int dbFindCtl(struct bmap * bmp, int l2nb, int level, s64 * blkno); -static int dbFindLeaf(dmtree_t * tp, int l2nb, int *leafidx); +static int dbFindLeaf(dmtree_t *tp, int l2nb, int *leafidx, bool is_ctl); static int dbFreeBits(struct bmap * bmp, struct dmap * dp, s64 blkno, int nblocks); static int dbFreeDmap(struct bmap * bmp, struct dmap * dp, s64 blkno, @@ -1717,7 +1717,7 @@ static int dbFindCtl(struct bmap * bmp, int l2nb, int level, s64 * blkno) * dbFindLeaf() returns the index of the leaf at which * free space was found. */ - rc = dbFindLeaf((dmtree_t *) dcp, l2nb, &leafidx); + rc = dbFindLeaf((dmtree_t *) dcp, l2nb, &leafidx, true); /* release the buffer. */ @@ -1964,7 +1964,7 @@ dbAllocDmapLev(struct bmap * bmp, * free space. if sufficient free space is found, dbFindLeaf() * returns the index of the leaf at which free space was found. */ - if (dbFindLeaf((dmtree_t *) & dp->tree, l2nb, &leafidx)) + if (dbFindLeaf((dmtree_t *) &dp->tree, l2nb, &leafidx, false)) return -ENOSPC; if (leafidx < 0) @@ -2928,14 +2928,18 @@ static void dbAdjTree(dmtree_t * tp, int leafno, int newval) * leafidx - return pointer to be set to the index of the leaf * describing at least l2nb free blocks if sufficient * free blocks are found. + * is_ctl - determines if the tree is of type ctl * * RETURN VALUES: * 0 - success * -ENOSPC - insufficient free blocks. */ -static int dbFindLeaf(dmtree_t * tp, int l2nb, int *leafidx) +static int dbFindLeaf(dmtree_t *tp, int l2nb, int *leafidx, bool is_ctl) { int ti, n = 0, k, x = 0; + int max_size; + + max_size = is_ctl ? CTLTREESIZE : TREESIZE; /* first check the root of the tree to see if there is * sufficient free space. @@ -2956,6 +2960,8 @@ static int dbFindLeaf(dmtree_t * tp, int l2nb, int *leafidx) /* sufficient free space found. move to the next * level (or quit if this is the last level). */ + if (x + n > max_size) + return -ENOSPC; if (l2nb <= tp->dmt_stree[x + n]) break; } From 05d9ea1ceb62a55af6727a69269a4fd310edf483 Mon Sep 17 00:00:00 2001 From: Manas Ghandat Date: Wed, 4 Oct 2023 13:10:40 +0530 Subject: [PATCH 04/69] jfs: fix array-index-out-of-bounds in diAlloc Currently there is not check against the agno of the iag while allocating new inodes to avoid fragmentation problem. Added the check which is required. Reported-by: syzbot+79d792676d8ac050949f@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=79d792676d8ac050949f Signed-off-by: Manas Ghandat Signed-off-by: Dave Kleikamp --- fs/jfs/jfs_imap.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c index 923a58422c46..1b267eec3f36 100644 --- a/fs/jfs/jfs_imap.c +++ b/fs/jfs/jfs_imap.c @@ -1320,7 +1320,7 @@ diInitInode(struct inode *ip, int iagno, int ino, int extno, struct iag * iagp) int diAlloc(struct inode *pip, bool dir, struct inode *ip) { int rc, ino, iagno, addext, extno, bitno, sword; - int nwords, rem, i, agno; + int nwords, rem, i, agno, dn_numag; u32 mask, inosmap, extsmap; struct inode *ipimap; struct metapage *mp; @@ -1356,6 +1356,9 @@ int diAlloc(struct inode *pip, bool dir, struct inode *ip) /* get the ag number of this iag */ agno = BLKTOAG(JFS_IP(pip)->agstart, JFS_SBI(pip->i_sb)); + dn_numag = JFS_SBI(pip->i_sb)->bmap->db_numag; + if (agno < 0 || agno > dn_numag) + return -EIO; if (atomic_read(&JFS_SBI(pip->i_sb)->bmap->db_active[agno])) { /* From ee6a12d0d4d85f3833d177cd382cd417f0ef011b Mon Sep 17 00:00:00 2001 From: Theodore Ts'o Date: Thu, 5 Oct 2023 21:42:47 -0400 Subject: [PATCH 05/69] ext4: add missing initialization of call_notify_error in update_super_work() Fixes: ff0722de896e ("ext4: add periodic superblock update check") Signed-off-by: Theodore Ts'o --- fs/ext4/super.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/ext4/super.c b/fs/ext4/super.c index dbebd8b3127e..6f48dec19f4a 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -768,7 +768,8 @@ static void update_super_work(struct work_struct *work) */ if (!sb_rdonly(sbi->s_sb) && journal) { struct buffer_head *sbh = sbi->s_sbh; - bool call_notify_err; + bool call_notify_err = false; + handle = jbd2_journal_start(journal, 1); if (IS_ERR(handle)) goto write_directly; From 745f17a4166e79315e4b7f33ce89d03e75a76983 Mon Sep 17 00:00:00 2001 From: Baokun Li Date: Wed, 24 May 2023 15:25:38 +0800 Subject: [PATCH 06/69] ext4: fix race between writepages and remount We got a WARNING in ext4_add_complete_io: ================================================================== WARNING: at fs/ext4/page-io.c:231 ext4_put_io_end_defer+0x182/0x250 CPU: 10 PID: 77 Comm: ksoftirqd/10 Tainted: 6.3.0-rc2 #85 RIP: 0010:ext4_put_io_end_defer+0x182/0x250 [ext4] [...] Call Trace: ext4_end_bio+0xa8/0x240 [ext4] bio_endio+0x195/0x310 blk_update_request+0x184/0x770 scsi_end_request+0x2f/0x240 scsi_io_completion+0x75/0x450 scsi_finish_command+0xef/0x160 scsi_complete+0xa3/0x180 blk_complete_reqs+0x60/0x80 blk_done_softirq+0x25/0x40 __do_softirq+0x119/0x4c8 run_ksoftirqd+0x42/0x70 smpboot_thread_fn+0x136/0x3c0 kthread+0x140/0x1a0 ret_from_fork+0x2c/0x50 ================================================================== Above issue may happen as follows: cpu1 cpu2 ----------------------------|---------------------------- mount -o dioread_lock ext4_writepages ext4_do_writepages *if (ext4_should_dioread_nolock(inode))* // rsv_blocks is not assigned here mount -o remount,dioread_nolock ext4_journal_start_with_reserve __ext4_journal_start __ext4_journal_start_sb jbd2__journal_start *if (rsv_blocks)* // h_rsv_handle is not initialized here mpage_map_and_submit_extent mpage_map_one_extent dioread_nolock = ext4_should_dioread_nolock(inode) if (dioread_nolock && (map->m_flags & EXT4_MAP_UNWRITTEN)) mpd->io_submit.io_end->handle = handle->h_rsv_handle ext4_set_io_unwritten_flag io_end->flag |= EXT4_IO_END_UNWRITTEN // now io_end->handle is NULL but has EXT4_IO_END_UNWRITTEN flag scsi_finish_command scsi_io_completion scsi_io_completion_action scsi_end_request blk_update_request req_bio_endio bio_endio bio->bi_end_io > ext4_end_bio ext4_put_io_end_defer ext4_add_complete_io // trigger WARN_ON(!io_end->handle && sbi->s_journal); The immediate cause of this problem is that ext4_should_dioread_nolock() function returns inconsistent values in the ext4_do_writepages() and mpage_map_one_extent(). There are four conditions in this function that can be changed at mount time to cause this problem. These four conditions can be divided into two categories: (1) journal_data and EXT4_EXTENTS_FL, which can be changed by ioctl (2) DELALLOC and DIOREAD_NOLOCK, which can be changed by remount The two in the first category have been fixed by commit c8585c6fcaf2 ("ext4: fix races between changing inode journal mode and ext4_writepages") and commit cb85f4d23f79 ("ext4: fix race between writepages and enabling EXT4_EXTENTS_FL") respectively. Two cases in the other category have not yet been fixed, and the above issue is caused by this situation. We refer to the fix for the first category, when applying options during remount, we grab s_writepages_rwsem to avoid racing with writepages ops to trigger this problem. Fixes: 6b523df4fb5a ("ext4: use transaction reservation for extent conversion in ext4_end_io") Cc: stable@vger.kernel.org Signed-off-by: Baokun Li Reviewed-by: Jan Kara Link: https://lore.kernel.org/r/20230524072538.2883391-1-libaokun1@huawei.com Signed-off-by: Theodore Ts'o --- fs/ext4/ext4.h | 3 ++- fs/ext4/super.c | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index 9418359b1d9d..cd4ccae1e28a 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -1676,7 +1676,8 @@ struct ext4_sb_info { /* * Barrier between writepages ops and changing any inode's JOURNAL_DATA - * or EXTENTS flag. + * or EXTENTS flag or between writepages ops and changing DELALLOC or + * DIOREAD_NOLOCK mount options on remount. */ struct percpu_rw_semaphore s_writepages_rwsem; struct dax_device *s_daxdev; diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 6f48dec19f4a..d062383ea50e 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -6443,6 +6443,7 @@ static int __ext4_remount(struct fs_context *fc, struct super_block *sb) struct ext4_mount_options old_opts; ext4_group_t g; int err = 0; + int alloc_ctx; #ifdef CONFIG_QUOTA int enable_quota = 0; int i, j; @@ -6483,7 +6484,16 @@ static int __ext4_remount(struct fs_context *fc, struct super_block *sb) } + /* + * Changing the DIOREAD_NOLOCK or DELALLOC mount options may cause + * two calls to ext4_should_dioread_nolock() to return inconsistent + * values, triggering WARN_ON in ext4_add_complete_io(). we grab + * here s_writepages_rwsem to avoid race between writepages ops and + * remount. + */ + alloc_ctx = ext4_writepages_down_write(sb); ext4_apply_options(fc, sb); + ext4_writepages_up_write(sb, alloc_ctx); if ((old_opts.s_mount_opt & EXT4_MOUNT_JOURNAL_CHECKSUM) ^ test_opt(sb, JOURNAL_CHECKSUM)) { @@ -6701,6 +6711,8 @@ restore_opts: if (sb_rdonly(sb) && !(old_sb_flags & SB_RDONLY) && sb_any_quota_suspended(sb)) dquot_resume(sb, -1); + + alloc_ctx = ext4_writepages_down_write(sb); sb->s_flags = old_sb_flags; sbi->s_mount_opt = old_opts.s_mount_opt; sbi->s_mount_opt2 = old_opts.s_mount_opt2; @@ -6709,6 +6721,8 @@ restore_opts: sbi->s_commit_interval = old_opts.s_commit_interval; sbi->s_min_batch_time = old_opts.s_min_batch_time; sbi->s_max_batch_time = old_opts.s_max_batch_time; + ext4_writepages_up_write(sb, alloc_ctx); + if (!test_opt(sb, BLOCK_VALIDITY) && sbi->s_system_blks) ext4_release_system_zone(sb); #ifdef CONFIG_QUOTA From a8c1eb77edfc92a64788dad70fedf9277cbafe76 Mon Sep 17 00:00:00 2001 From: Lu Hongfei Date: Mon, 29 May 2023 15:09:30 +0800 Subject: [PATCH 07/69] ext4: fix traditional comparison using max/min method It would be better to replace the traditional ternary conditional operator with max()/min() Signed-off-by: Lu Hongfei Reviewed-by: Kemeng Shi Reviewed-by: Jan Kara Link: https://lore.kernel.org/r/20230529070930.37949-1-luhongfei@vivo.com Signed-off-by: Theodore Ts'o --- fs/ext4/balloc.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c index 79b20d6ae39e..4d08bb2bd184 100644 --- a/fs/ext4/balloc.c +++ b/fs/ext4/balloc.c @@ -111,10 +111,8 @@ static unsigned ext4_num_overhead_clusters(struct super_block *sb, itbl_blk_start = ext4_inode_table(sb, gdp); itbl_blk_end = itbl_blk_start + sbi->s_itb_per_group - 1; if (itbl_blk_start <= end && itbl_blk_end >= start) { - itbl_blk_start = itbl_blk_start >= start ? - itbl_blk_start : start; - itbl_blk_end = itbl_blk_end <= end ? - itbl_blk_end : end; + itbl_blk_start = max(itbl_blk_start, start); + itbl_blk_end = min(itbl_blk_end, end); itbl_cluster_start = EXT4_B2C(sbi, itbl_blk_start - start); itbl_cluster_end = EXT4_B2C(sbi, itbl_blk_end - start); From ce774e5365e46be73ed055302c6de123a03394ea Mon Sep 17 00:00:00 2001 From: Jinke Han Date: Mon, 12 Jun 2023 20:40:17 +0800 Subject: [PATCH 08/69] ext4: make running and commit transaction have their own freed_data_list When releasing space in jbd, we traverse s_freed_data_list to get the free range belonging to the current commit transaction. In extreme cases, the time spent may not be small, and we have observed cases exceeding 10ms. This patch makes running and commit transactions manage their own free_data_list respectively, eliminating unnecessary traversal. And in the callback phase of the commit transaction, no one will touch it except the jbd thread itself, so s_md_lock is no longer needed. Signed-off-by: Jinke Han Reviewed-by: Zhang Yi Link: https://lore.kernel.org/r/20230612124017.14115-1-hanjinke.666@bytedance.com Signed-off-by: Theodore Ts'o --- fs/ext4/ext4.h | 2 +- fs/ext4/mballoc.c | 18 +++++------------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index cd4ccae1e28a..3b8bc44be528 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -1564,7 +1564,7 @@ struct ext4_sb_info { unsigned int *s_mb_maxs; unsigned int s_group_info_size; unsigned int s_mb_free_pending; - struct list_head s_freed_data_list; /* List of blocks to be freed + struct list_head s_freed_data_list[2]; /* List of blocks to be freed after commit completed */ struct list_head s_discard_list; struct work_struct s_discard_work; diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c index 1e599305d85f..1d65c738c4c9 100644 --- a/fs/ext4/mballoc.c +++ b/fs/ext4/mballoc.c @@ -3631,7 +3631,8 @@ int ext4_mb_init(struct super_block *sb) spin_lock_init(&sbi->s_md_lock); sbi->s_mb_free_pending = 0; - INIT_LIST_HEAD(&sbi->s_freed_data_list); + INIT_LIST_HEAD(&sbi->s_freed_data_list[0]); + INIT_LIST_HEAD(&sbi->s_freed_data_list[1]); INIT_LIST_HEAD(&sbi->s_discard_list); INIT_WORK(&sbi->s_discard_work, ext4_discard_work); atomic_set(&sbi->s_retry_alloc_pending, 0); @@ -3883,19 +3884,10 @@ void ext4_process_freed_data(struct super_block *sb, tid_t commit_tid) struct ext4_sb_info *sbi = EXT4_SB(sb); struct ext4_free_data *entry, *tmp; LIST_HEAD(freed_data_list); - struct list_head *cut_pos = NULL; + struct list_head *s_freed_head = &sbi->s_freed_data_list[commit_tid & 1]; bool wake; - spin_lock(&sbi->s_md_lock); - list_for_each_entry(entry, &sbi->s_freed_data_list, efd_list) { - if (entry->efd_tid != commit_tid) - break; - cut_pos = &entry->efd_list; - } - if (cut_pos) - list_cut_position(&freed_data_list, &sbi->s_freed_data_list, - cut_pos); - spin_unlock(&sbi->s_md_lock); + list_replace_init(s_freed_head, &freed_data_list); list_for_each_entry(entry, &freed_data_list, efd_list) ext4_free_data_in_buddy(sb, entry); @@ -6378,7 +6370,7 @@ ext4_mb_free_metadata(handle_t *handle, struct ext4_buddy *e4b, } spin_lock(&sbi->s_md_lock); - list_add_tail(&new_entry->efd_list, &sbi->s_freed_data_list); + list_add_tail(&new_entry->efd_list, &sbi->s_freed_data_list[new_entry->efd_tid & 1]); sbi->s_mb_free_pending += clusters; spin_unlock(&sbi->s_md_lock); } From 40ea98396a3659062267d1fe5f99af4f7e4f05e3 Mon Sep 17 00:00:00 2001 From: Zhang Yi Date: Thu, 24 Aug 2023 17:26:04 +0800 Subject: [PATCH 09/69] ext4: correct the start block of counting reserved clusters When big allocate feature is enabled, we need to count and update reserved clusters before removing a delayed only extent_status entry. {init|count|get}_rsvd() have already done this, but the start block number of this counting isn't correct in the following case. lblk end | | v v ------------------------- | | orig_es ------------------------- ^ ^ len1 is 0 | len2 | If the start block of the orig_es entry founded is bigger than lblk, we passed lblk as start block to count_rsvd(), but the length is correct, finally, the range to be counted is offset. This patch fix this by passing the start blocks to 'orig_es->lblk + len1'. Signed-off-by: Zhang Yi Cc: stable@kernel.org Link: https://lore.kernel.org/r/20230824092619.1327976-2-yi.zhang@huaweicloud.com Signed-off-by: Theodore Ts'o Reviewed-by: Jan Kara --- fs/ext4/extents_status.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c index 6f7de14c0fa8..5e625ea4545d 100644 --- a/fs/ext4/extents_status.c +++ b/fs/ext4/extents_status.c @@ -1405,8 +1405,8 @@ static int __es_remove_extent(struct inode *inode, ext4_lblk_t lblk, } } if (count_reserved) - count_rsvd(inode, lblk, orig_es.es_len - len1 - len2, - &orig_es, &rc); + count_rsvd(inode, orig_es.es_lblk + len1, + orig_es.es_len - len1 - len2, &orig_es, &rc); goto out_get_reserved; } From 8e387c89e96b9543a339f84043cf9df15fed2632 Mon Sep 17 00:00:00 2001 From: Zhang Yi Date: Thu, 24 Aug 2023 17:26:05 +0800 Subject: [PATCH 10/69] ext4: make sure allocate pending entry not fail __insert_pending() allocate memory in atomic context, so the allocation could fail, but we are not handling that failure now. It could lead ext4_es_remove_extent() to get wrong reserved clusters, and the global data blocks reservation count will be incorrect. The same to extents_status entry preallocation, preallocate pending entry out of the i_es_lock with __GFP_NOFAIL, make sure __insert_pending() and __revise_pending() always succeeds. Signed-off-by: Zhang Yi Cc: stable@kernel.org Link: https://lore.kernel.org/r/20230824092619.1327976-3-yi.zhang@huaweicloud.com Reviewed-by: Jan Kara Signed-off-by: Theodore Ts'o --- fs/ext4/extents_status.c | 123 ++++++++++++++++++++++++++++----------- 1 file changed, 89 insertions(+), 34 deletions(-) diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c index 5e625ea4545d..f4b50652f0cc 100644 --- a/fs/ext4/extents_status.c +++ b/fs/ext4/extents_status.c @@ -152,8 +152,9 @@ static int __es_remove_extent(struct inode *inode, ext4_lblk_t lblk, static int es_reclaim_extents(struct ext4_inode_info *ei, int *nr_to_scan); static int __es_shrink(struct ext4_sb_info *sbi, int nr_to_scan, struct ext4_inode_info *locked_ei); -static void __revise_pending(struct inode *inode, ext4_lblk_t lblk, - ext4_lblk_t len); +static int __revise_pending(struct inode *inode, ext4_lblk_t lblk, + ext4_lblk_t len, + struct pending_reservation **prealloc); int __init ext4_init_es(void) { @@ -448,6 +449,19 @@ static void ext4_es_list_del(struct inode *inode) spin_unlock(&sbi->s_es_lock); } +static inline struct pending_reservation *__alloc_pending(bool nofail) +{ + if (!nofail) + return kmem_cache_alloc(ext4_pending_cachep, GFP_ATOMIC); + + return kmem_cache_zalloc(ext4_pending_cachep, GFP_KERNEL | __GFP_NOFAIL); +} + +static inline void __free_pending(struct pending_reservation *pr) +{ + kmem_cache_free(ext4_pending_cachep, pr); +} + /* * Returns true if we cannot fail to allocate memory for this extent_status * entry and cannot reclaim it until its status changes. @@ -836,11 +850,12 @@ void ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk, { struct extent_status newes; ext4_lblk_t end = lblk + len - 1; - int err1 = 0; - int err2 = 0; + int err1 = 0, err2 = 0, err3 = 0; struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb); struct extent_status *es1 = NULL; struct extent_status *es2 = NULL; + struct pending_reservation *pr = NULL; + bool revise_pending = false; if (EXT4_SB(inode->i_sb)->s_mount_state & EXT4_FC_REPLAY) return; @@ -868,11 +883,17 @@ void ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk, ext4_es_insert_extent_check(inode, &newes); + revise_pending = sbi->s_cluster_ratio > 1 && + test_opt(inode->i_sb, DELALLOC) && + (status & (EXTENT_STATUS_WRITTEN | + EXTENT_STATUS_UNWRITTEN)); retry: if (err1 && !es1) es1 = __es_alloc_extent(true); if ((err1 || err2) && !es2) es2 = __es_alloc_extent(true); + if ((err1 || err2 || err3) && revise_pending && !pr) + pr = __alloc_pending(true); write_lock(&EXT4_I(inode)->i_es_lock); err1 = __es_remove_extent(inode, lblk, end, NULL, es1); @@ -897,13 +918,18 @@ retry: es2 = NULL; } - if (sbi->s_cluster_ratio > 1 && test_opt(inode->i_sb, DELALLOC) && - (status & EXTENT_STATUS_WRITTEN || - status & EXTENT_STATUS_UNWRITTEN)) - __revise_pending(inode, lblk, len); + if (revise_pending) { + err3 = __revise_pending(inode, lblk, len, &pr); + if (err3 != 0) + goto error; + if (pr) { + __free_pending(pr); + pr = NULL; + } + } error: write_unlock(&EXT4_I(inode)->i_es_lock); - if (err1 || err2) + if (err1 || err2 || err3) goto retry; ext4_es_print_tree(inode); @@ -1311,7 +1337,7 @@ static unsigned int get_rsvd(struct inode *inode, ext4_lblk_t end, rc->ndelonly--; node = rb_next(&pr->rb_node); rb_erase(&pr->rb_node, &tree->root); - kmem_cache_free(ext4_pending_cachep, pr); + __free_pending(pr); if (!node) break; pr = rb_entry(node, struct pending_reservation, @@ -1907,11 +1933,13 @@ static struct pending_reservation *__get_pending(struct inode *inode, * * @inode - file containing the cluster * @lblk - logical block in the cluster to be added + * @prealloc - preallocated pending entry * * Returns 0 on successful insertion and -ENOMEM on failure. If the * pending reservation is already in the set, returns successfully. */ -static int __insert_pending(struct inode *inode, ext4_lblk_t lblk) +static int __insert_pending(struct inode *inode, ext4_lblk_t lblk, + struct pending_reservation **prealloc) { struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb); struct ext4_pending_tree *tree = &EXT4_I(inode)->i_pending_tree; @@ -1937,10 +1965,15 @@ static int __insert_pending(struct inode *inode, ext4_lblk_t lblk) } } - pr = kmem_cache_alloc(ext4_pending_cachep, GFP_ATOMIC); - if (pr == NULL) { - ret = -ENOMEM; - goto out; + if (likely(*prealloc == NULL)) { + pr = __alloc_pending(false); + if (!pr) { + ret = -ENOMEM; + goto out; + } + } else { + pr = *prealloc; + *prealloc = NULL; } pr->lclu = lclu; @@ -1970,7 +2003,7 @@ static void __remove_pending(struct inode *inode, ext4_lblk_t lblk) if (pr != NULL) { tree = &EXT4_I(inode)->i_pending_tree; rb_erase(&pr->rb_node, &tree->root); - kmem_cache_free(ext4_pending_cachep, pr); + __free_pending(pr); } } @@ -2029,10 +2062,10 @@ void ext4_es_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk, bool allocated) { struct extent_status newes; - int err1 = 0; - int err2 = 0; + int err1 = 0, err2 = 0, err3 = 0; struct extent_status *es1 = NULL; struct extent_status *es2 = NULL; + struct pending_reservation *pr = NULL; if (EXT4_SB(inode->i_sb)->s_mount_state & EXT4_FC_REPLAY) return; @@ -2052,6 +2085,8 @@ retry: es1 = __es_alloc_extent(true); if ((err1 || err2) && !es2) es2 = __es_alloc_extent(true); + if ((err1 || err2 || err3) && allocated && !pr) + pr = __alloc_pending(true); write_lock(&EXT4_I(inode)->i_es_lock); err1 = __es_remove_extent(inode, lblk, lblk, NULL, es1); @@ -2074,11 +2109,18 @@ retry: es2 = NULL; } - if (allocated) - __insert_pending(inode, lblk); + if (allocated) { + err3 = __insert_pending(inode, lblk, &pr); + if (err3 != 0) + goto error; + if (pr) { + __free_pending(pr); + pr = NULL; + } + } error: write_unlock(&EXT4_I(inode)->i_es_lock); - if (err1 || err2) + if (err1 || err2 || err3) goto retry; ext4_es_print_tree(inode); @@ -2184,21 +2226,24 @@ unsigned int ext4_es_delayed_clu(struct inode *inode, ext4_lblk_t lblk, * @inode - file containing the range * @lblk - logical block defining the start of range * @len - length of range in blocks + * @prealloc - preallocated pending entry * * Used after a newly allocated extent is added to the extents status tree. * Requires that the extents in the range have either written or unwritten * status. Must be called while holding i_es_lock. */ -static void __revise_pending(struct inode *inode, ext4_lblk_t lblk, - ext4_lblk_t len) +static int __revise_pending(struct inode *inode, ext4_lblk_t lblk, + ext4_lblk_t len, + struct pending_reservation **prealloc) { struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb); ext4_lblk_t end = lblk + len - 1; ext4_lblk_t first, last; bool f_del = false, l_del = false; + int ret = 0; if (len == 0) - return; + return 0; /* * Two cases - block range within single cluster and block range @@ -2219,7 +2264,9 @@ static void __revise_pending(struct inode *inode, ext4_lblk_t lblk, f_del = __es_scan_range(inode, &ext4_es_is_delonly, first, lblk - 1); if (f_del) { - __insert_pending(inode, first); + ret = __insert_pending(inode, first, prealloc); + if (ret < 0) + goto out; } else { last = EXT4_LBLK_CMASK(sbi, end) + sbi->s_cluster_ratio - 1; @@ -2227,9 +2274,11 @@ static void __revise_pending(struct inode *inode, ext4_lblk_t lblk, l_del = __es_scan_range(inode, &ext4_es_is_delonly, end + 1, last); - if (l_del) - __insert_pending(inode, last); - else + if (l_del) { + ret = __insert_pending(inode, last, prealloc); + if (ret < 0) + goto out; + } else __remove_pending(inode, last); } } else { @@ -2237,18 +2286,24 @@ static void __revise_pending(struct inode *inode, ext4_lblk_t lblk, if (first != lblk) f_del = __es_scan_range(inode, &ext4_es_is_delonly, first, lblk - 1); - if (f_del) - __insert_pending(inode, first); - else + if (f_del) { + ret = __insert_pending(inode, first, prealloc); + if (ret < 0) + goto out; + } else __remove_pending(inode, first); last = EXT4_LBLK_CMASK(sbi, end) + sbi->s_cluster_ratio - 1; if (last != end) l_del = __es_scan_range(inode, &ext4_es_is_delonly, end + 1, last); - if (l_del) - __insert_pending(inode, last); - else + if (l_del) { + ret = __insert_pending(inode, last, prealloc); + if (ret < 0) + goto out; + } else __remove_pending(inode, last); } +out: + return ret; } From 8fedebb5ea183994aca39af3f80623f5db42fff7 Mon Sep 17 00:00:00 2001 From: Wang Jianjian Date: Thu, 24 Aug 2023 23:23:24 +0800 Subject: [PATCH 11/69] ext4: fix incorrect offset The last argument of ext4_check_dir_entry is dentry offset int the file. Luckily this error only results in the wrong offset being printed in the eventual error message. Signed-off-by: Wang Jianjian Reviewed-by: Jan Kara Link: https://lore.kernel.org/r/tencent_F992989953734FD5DE3F88ECB2191A856206@qq.com Signed-off-by: Theodore Ts'o --- fs/ext4/namei.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c index bbda587f76b8..cbe756144d8d 100644 --- a/fs/ext4/namei.c +++ b/fs/ext4/namei.c @@ -2280,8 +2280,7 @@ static int make_indexed_dir(handle_t *handle, struct ext4_filename *fname, top = data2 + len; while ((char *)(de2 = ext4_next_entry(de, blocksize)) < top) { if (ext4_check_dir_entry(dir, NULL, de, bh2, data2, len, - (data2 + (blocksize - csum_size) - - (char *) de))) { + (char *)de - data2)) { brelse(bh2); brelse(bh); return -EFSCORRUPTED; From ebf6cb7c6e1241984f75f29f1bdbfa2fe7168f88 Mon Sep 17 00:00:00 2001 From: Wang Jianjian Date: Thu, 24 Aug 2023 23:56:31 +0800 Subject: [PATCH 12/69] ext4: no need to generate from free list in mballoc Commit 7a2fcbf7f85 ("ext4: don't use blocks freed but not yet committed in buddy cache init") added a code to mark as used blocks in the list of not yet committed freed blocks during initialization of a buddy page. However ext4_mb_free_metadata() makes sure buddy page is already loaded and takes a reference to it so it cannot happen that ext4_mb_init_cache() is called when efd list is non-empty. Just remove the ext4_mb_generate_from_freelist() call. Fixes: 7a2fcbf7f85('ext4: don't use blocks freed but not yet committed in buddy cache init') Signed-off-by: Wang Jianjian Link: https://lore.kernel.org/r/tencent_53CBCB1668358AE862684E453DF37B722008@qq.com Signed-off-by: Theodore Ts'o Cc: stable@kernel.org --- fs/ext4/mballoc.c | 39 ++++++--------------------------------- 1 file changed, 6 insertions(+), 33 deletions(-) diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c index 1d65c738c4c9..6e304c18d390 100644 --- a/fs/ext4/mballoc.c +++ b/fs/ext4/mballoc.c @@ -417,8 +417,6 @@ static const char * const ext4_groupinfo_slab_names[NR_GRPINFO_CACHES] = { static void ext4_mb_generate_from_pa(struct super_block *sb, void *bitmap, ext4_group_t group); -static void ext4_mb_generate_from_freelist(struct super_block *sb, void *bitmap, - ext4_group_t group); static void ext4_mb_new_preallocation(struct ext4_allocation_context *ac); static bool ext4_mb_good_group(struct ext4_allocation_context *ac, @@ -1361,17 +1359,17 @@ static int ext4_mb_init_cache(struct page *page, char *incore, gfp_t gfp) * We place the buddy block and bitmap block * close together */ + grinfo = ext4_get_group_info(sb, group); + if (!grinfo) { + err = -EFSCORRUPTED; + goto out; + } if ((first_block + i) & 1) { /* this is block of buddy */ BUG_ON(incore == NULL); mb_debug(sb, "put buddy for group %u in page %lu/%x\n", group, page->index, i * blocksize); trace_ext4_mb_buddy_bitmap_load(sb, group); - grinfo = ext4_get_group_info(sb, group); - if (!grinfo) { - err = -EFSCORRUPTED; - goto out; - } grinfo->bb_fragments = 0; memset(grinfo->bb_counters, 0, sizeof(*grinfo->bb_counters) * @@ -1398,7 +1396,7 @@ static int ext4_mb_init_cache(struct page *page, char *incore, gfp_t gfp) /* mark all preallocated blks used in in-core bitmap */ ext4_mb_generate_from_pa(sb, data, group); - ext4_mb_generate_from_freelist(sb, data, group); + WARN_ON_ONCE(!RB_EMPTY_ROOT(&grinfo->bb_free_root)); ext4_unlock_group(sb, group); /* set incore so that the buddy information can be @@ -4950,31 +4948,6 @@ try_group_pa: return false; } -/* - * the function goes through all block freed in the group - * but not yet committed and marks them used in in-core bitmap. - * buddy must be generated from this bitmap - * Need to be called with the ext4 group lock held - */ -static void ext4_mb_generate_from_freelist(struct super_block *sb, void *bitmap, - ext4_group_t group) -{ - struct rb_node *n; - struct ext4_group_info *grp; - struct ext4_free_data *entry; - - grp = ext4_get_group_info(sb, group); - if (!grp) - return; - n = rb_first(&(grp->bb_free_root)); - - while (n) { - entry = rb_entry(n, struct ext4_free_data, efd_node); - mb_set_bits(bitmap, entry->efd_start_cluster, entry->efd_count); - n = rb_next(n); - } -} - /* * the function goes through all preallocation in this group and marks them * used in in-core bitmap. buddy must be generated from this bitmap From 31f13421c004a420c0e9d288859c9ea9259ea0cc Mon Sep 17 00:00:00 2001 From: Kemeng Shi Date: Sun, 27 Aug 2023 01:47:00 +0800 Subject: [PATCH 13/69] ext4: correct offset of gdb backup in non meta_bg group to update_backups 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 Reviewed-by: Theodore Ts'o Link: https://lore.kernel.org/r/20230826174712.4059355-2-shikemeng@huaweicloud.com Signed-off-by: Theodore Ts'o Cc: stable@kernel.org --- 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 @@ exit_journal: 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 @@ exit_journal: 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; } } From 9adac8b01f4be28acd5838aade42b8daa4f0b642 Mon Sep 17 00:00:00 2001 From: Kemeng Shi Date: Sun, 27 Aug 2023 01:47:01 +0800 Subject: [PATCH 14/69] ext4: add missed brelse in update_backups add missed brelse in update_backups Signed-off-by: Kemeng Shi Reviewed-by: Theodore Ts'o Link: https://lore.kernel.org/r/20230826174712.4059355-3-shikemeng@huaweicloud.com Signed-off-by: Theodore Ts'o Cc: stable@kernel.org --- 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) From 48f1551592c54f7d8e2befc72a99ff4e47f7dca0 Mon Sep 17 00:00:00 2001 From: Kemeng Shi Date: Sun, 27 Aug 2023 01:47:02 +0800 Subject: [PATCH 15/69] ext4: correct return value of ext4_convert_meta_bg Avoid to ignore error in "err". Signed-off-by: Kemeng Shi Link: https://lore.kernel.org/r/20230826174712.4059355-4-shikemeng@huaweicloud.com Signed-off-by: Theodore Ts'o Cc: stable@kernel.org --- 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"); From 40dd7953f4d606c280074f10d23046b6812708ce Mon Sep 17 00:00:00 2001 From: Kemeng Shi Date: Sun, 27 Aug 2023 01:47:03 +0800 Subject: [PATCH 16/69] ext4: remove gdb backup copy for meta bg in setup_new_flex_group_blocks 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 Reviewed-by: Theodore Ts'o Link: https://lore.kernel.org/r/20230826174712.4059355-5-shikemeng@huaweicloud.com Signed-off-by: Theodore Ts'o Cc: stable@kernel.org --- 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 */ From e44fc921b84ff08a9e2fb827a146fa4021d016f3 Mon Sep 17 00:00:00 2001 From: Kemeng Shi Date: Sun, 27 Aug 2023 01:47:04 +0800 Subject: [PATCH 17/69] ext4: fix typo in setup_new_flex_group_blocks grop -> group Signed-off-by: Kemeng Shi Reviewed-by: Theodore Ts'o Link: https://lore.kernel.org/r/20230826174712.4059355-6-shikemeng@huaweicloud.com Signed-off-by: Theodore Ts'o --- 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; From 7d4cd3b45af025befe3bca94f87359a6603b6e95 Mon Sep 17 00:00:00 2001 From: Kemeng Shi Date: Sun, 27 Aug 2023 01:47:05 +0800 Subject: [PATCH 18/69] ext4: remove redundant check of count Remove zero check of count which is always non-zero. Signed-off-by: Kemeng Shi Reviewed-by: Theodore Ts'o Link: https://lore.kernel.org/r/20230826174712.4059355-7-shikemeng@huaweicloud.com Signed-off-by: Theodore Ts'o --- 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 @@ handle_ib: 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: From 31458077273b5f883d99bee33a7fb295f155712d Mon Sep 17 00:00:00 2001 From: Kemeng Shi Date: Sun, 27 Aug 2023 01:47:06 +0800 Subject: [PATCH 19/69] ext4: remove commented code in reserve_backup_gdb Remove commented code in reserve_backup_gdb Signed-off-by: Kemeng Shi Reviewed-by: Theodore Ts'o Link: https://lore.kernel.org/r/20230826174712.4059355-8-shikemeng@huaweicloud.com Signed-off-by: Theodore Ts'o --- 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) From 1fc1bd2d18bbade157f7b14270f509ebbd89881b Mon Sep 17 00:00:00 2001 From: Kemeng Shi Date: Sun, 27 Aug 2023 01:47:07 +0800 Subject: [PATCH 20/69] ext4: calculate free_clusters_count in cluster unit in verify_group_input 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 Reviewed-by: Theodore Ts'o Link: https://lore.kernel.org/r/20230826174712.4059355-9-shikemeng@huaweicloud.com Signed-off-by: Theodore Ts'o --- 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 " From 95b635689b58e0ebe5197bf99c82c681eabe17ee Mon Sep 17 00:00:00 2001 From: Kemeng Shi Date: Sun, 27 Aug 2023 01:47:08 +0800 Subject: [PATCH 21/69] ext4: remove EXT4FS_DEBUG defination in resize.c 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 Reviewed-by: Theodore Ts'o Link: https://lore.kernel.org/r/20230826174712.4059355-10-shikemeng@huaweicloud.com Signed-off-by: Theodore Ts'o --- 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 #include #include From 70cbfd257995b3f23c2408fd893cc18b61e58b4a Mon Sep 17 00:00:00 2001 From: Kemeng Shi Date: Sun, 27 Aug 2023 01:47:09 +0800 Subject: [PATCH 22/69] ext4: use saved local variable sbi instead of EXT4_SB(sb) 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 Reviewed-by: Theodore Ts'o Link: https://lore.kernel.org/r/20230826174712.4059355-11-shikemeng@huaweicloud.com Signed-off-by: Theodore Ts'o --- 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; From 9dca529bdaad7a7242a36d04f73cb998b817ab48 Mon Sep 17 00:00:00 2001 From: Kemeng Shi Date: Sun, 27 Aug 2023 01:47:10 +0800 Subject: [PATCH 23/69] ext4: simplify the gdbblock calculation in add_new_gdb_meta_bg 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 Link: https://lore.kernel.org/r/20230826174712.4059355-12-shikemeng@huaweicloud.com Signed-off-by: Theodore Ts'o --- 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 @@ errout: } /* - * 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); From 350bb48b84b8f4ad4ea179dbb97f568d12626188 Mon Sep 17 00:00:00 2001 From: Kemeng Shi Date: Sun, 27 Aug 2023 01:47:11 +0800 Subject: [PATCH 24/69] ext4: remove unnecessary check to avoid repeat update_backups for the same gdb 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 Link: https://lore.kernel.org/r/20230826174712.4059355-13-shikemeng@huaweicloud.com Signed-off-by: Theodore Ts'o --- 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 3b8bc44be528..5ab49226cd9f 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -1494,6 +1494,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 @@ exit_journal: 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 @@ exit_journal: 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: From 248b45b621a77155f81129e6b572ec833edb4cf4 Mon Sep 17 00:00:00 2001 From: Kemeng Shi Date: Sun, 27 Aug 2023 01:47:12 +0800 Subject: [PATCH 25/69] ext4: remove unnecessary initialization of count2 in set_flexbg_block_bitmap 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 Reviewed-by: Theodore Ts'o Link: https://lore.kernel.org/r/20230826174712.4059355-14-shikemeng@huaweicloud.com Signed-off-by: Theodore Ts'o --- 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; From 71cd5a5aa0607073adba3852739b7f8c22bc7b50 Mon Sep 17 00:00:00 2001 From: Ye Bin Date: Mon, 4 Sep 2023 18:58:16 +0800 Subject: [PATCH 26/69] jbd2: print io_block if check data block checksum failed when do recovery Now, if check data block checksum failed only print data's block number then skip write data. However, one data block may in more than one transaction. In some scenarios, offline analysis is inconvenient. As a result, it is difficult to locate the areas where data is faulty. So print 'io_block' if check data block checksum failed. Signed-off-by: Ye Bin Reviewed-by: Jan Kara Link: https://lore.kernel.org/r/20230904105817.1728356-2-yebin10@huawei.com Signed-off-by: Theodore Ts'o --- fs/jbd2/recovery.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c index c269a7d29a46..11380ff1fe51 100644 --- a/fs/jbd2/recovery.c +++ b/fs/jbd2/recovery.c @@ -661,7 +661,8 @@ static int do_one_pass(journal_t *journal, printk(KERN_ERR "JBD2: Invalid " "checksum recovering " "data block %llu in " - "log\n", blocknr); + "journal block %lu\n", + blocknr, io_block); block_error = 1; goto skip_write; } From 8b6b562121f1981315e76b6ae1f8a5cbbcf14bd7 Mon Sep 17 00:00:00 2001 From: Ye Bin Date: Mon, 4 Sep 2023 18:58:17 +0800 Subject: [PATCH 27/69] jbd2: fix printk format type for 'io_block' in do_one_pass() 'io_block' is unsinged long but print it by '%ld'. Signed-off-by: Ye Bin Reviewed-by: Jan Kara Link: https://lore.kernel.org/r/20230904105817.1728356-3-yebin10@huawei.com Signed-off-by: Theodore Ts'o --- fs/jbd2/recovery.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c index 11380ff1fe51..0567f1aa189b 100644 --- a/fs/jbd2/recovery.c +++ b/fs/jbd2/recovery.c @@ -632,7 +632,7 @@ static int do_one_pass(journal_t *journal, success = err; printk(KERN_ERR "JBD2: IO error %d recovering " - "block %ld in log\n", + "block %lu in log\n", err, io_block); } else { unsigned long long blocknr; From af90a8f4a09ec4a3de20142e37f37205d4687f28 Mon Sep 17 00:00:00 2001 From: Gou Hao Date: Wed, 6 Sep 2023 09:33:41 +0800 Subject: [PATCH 28/69] ext4: move 'ix' sanity check to corrent position Check 'ix' before it is used. Fixes: 80e675f906db ("ext4: optimize memmmove lengths in extent/index insertions") Signed-off-by: Gou Hao Link: https://lore.kernel.org/r/20230906013341.7199-1-gouhao@uniontech.com Signed-off-by: Theodore Ts'o --- fs/ext4/extents.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index 202c76996b62..4d8496d1a8ac 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -1010,6 +1010,11 @@ static int ext4_ext_insert_index(handle_t *handle, struct inode *inode, ix = curp->p_idx; } + if (unlikely(ix > EXT_MAX_INDEX(curp->p_hdr))) { + EXT4_ERROR_INODE(inode, "ix > EXT_MAX_INDEX!"); + return -EFSCORRUPTED; + } + len = EXT_LAST_INDEX(curp->p_hdr) - ix + 1; BUG_ON(len < 0); if (len > 0) { @@ -1019,11 +1024,6 @@ static int ext4_ext_insert_index(handle_t *handle, struct inode *inode, memmove(ix + 1, ix, len * sizeof(struct ext4_extent_idx)); } - if (unlikely(ix > EXT_MAX_INDEX(curp->p_hdr))) { - EXT4_ERROR_INODE(inode, "ix > EXT_MAX_INDEX!"); - return -EFSCORRUPTED; - } - ix->ei_block = cpu_to_le32(logical); ext4_idx_store_pblock(ix, ptr); le16_add_cpu(&curp->p_hdr->eh_entries, 1); From 2cd8bdb5efc1e0d5b11a4b7ba6b922fd2736a87f Mon Sep 17 00:00:00 2001 From: Ojaswin Mujoo Date: Mon, 18 Sep 2023 16:15:50 +0530 Subject: [PATCH 29/69] ext4: mark buffer new if it is unwritten to avoid stale data exposure ** Short Version ** In ext4 with dioread_nolock, we could have a scenario where the bh returned by get_blocks (ext4_get_block_unwritten()) in __block_write_begin_int() has UNWRITTEN and MAPPED flag set. Since such a bh does not have NEW flag set we never zero out the range of bh that is not under write, causing whatever stale data is present in the folio at that time to be written out to disk. To fix this mark the buffer as new, in case it is unwritten, in ext4_get_block_unwritten(). ** Long Version ** The issue mentioned above was resulting in two different bugs: 1. On block size < page size case in ext4, generic/269 was reliably failing with dioread_nolock. The state of the write was as follows: * The write was extending i_size. * The last block of the file was fallocated and had an unwritten extent * We were near ENOSPC and hence we were switching to non-delayed alloc allocation. In this case, the back trace that triggers the bug is as follows: ext4_da_write_begin() /* switch to nodelalloc due to low space */ ext4_write_begin() ext4_should_dioread_nolock() // true since mount flags still have delalloc __block_write_begin(..., ext4_get_block_unwritten) __block_write_begin_int() for(each buffer head in page) { /* first iteration, this is bh1 which contains i_size */ if (!buffer_mapped) get_block() /* returns bh with only UNWRITTEN and MAPPED */ /* second iteration, bh2 */ if (!buffer_mapped) get_block() /* we fail here, could be ENOSPC */ } if (err) /* * this would zero out all new buffers and mark them uptodate. * Since bh1 was never marked new, we skip it here which causes * the bug later. */ folio_zero_new_buffers(); /* ext4_wrte_begin() error handling */ ext4_truncate_failed_write() ext4_truncate() ext4_block_truncate_page() __ext4_block_zero_page_range() if(!buffer_uptodate()) ext4_read_bh_lock() ext4_read_bh() -> ... ext4_submit_bh_wbc() BUG_ON(buffer_unwritten(bh)); /* !!! */ 2. The second issue is stale data exposure with page size >= blocksize with dioread_nolock. The conditions needed for it to happen are same as the previous issue ie dioread_nolock around ENOSPC condition. The issue is also similar where in __block_write_begin_int() when we call ext4_get_block_unwritten() on the buffer_head and the underlying extent is unwritten, we get an unwritten and mapped buffer head. Since it is not new, we never zero out the partial range which is not under write, thus writing stale data to disk. This can be easily observed with the following reproducer: fallocate -l 4k testfile xfs_io -c "pwrite 2k 2k" testfile # hexdump output will have stale data in from byte 0 to 2k in testfile hexdump -C testfile NOTE: To trigger this, we need dioread_nolock enabled and write happening via ext4_write_begin(), which is usually used when we have -o nodealloc. Since dioread_nolock is disabled with nodelalloc, the only alternate way to call ext4_write_begin() is to ensure that delayed alloc switches to nodelalloc ie ext4_da_write_begin() calls ext4_write_begin(). This will usually happen when ext4 is almost full like the way generic/269 was triggering it in Issue 1 above. This might make the issue harder to hit. Hence, for reliable replication, I used the below patch to temporarily allow dioread_nolock with nodelalloc and then mount the disk with -o nodealloc,dioread_nolock. With this you can hit the stale data issue 100% of times: @@ -508,8 +508,8 @@ static inline int ext4_should_dioread_nolock(struct inode *inode) if (ext4_should_journal_data(inode)) return 0; /* temporary fix to prevent generic/422 test failures */ - if (!test_opt(inode->i_sb, DELALLOC)) - return 0; + // if (!test_opt(inode->i_sb, DELALLOC)) + // return 0; return 1; } After applying this patch to mark buffer as NEW, both the above issues are fixed. Signed-off-by: Ojaswin Mujoo Cc: stable@kernel.org Reviewed-by: Jan Kara Reviewed-by: "Ritesh Harjani (IBM)" Link: https://lore.kernel.org/r/d0ed09d70a9733fbb5349c5c7b125caac186ecdf.1695033645.git.ojaswin@linux.ibm.com Signed-off-by: Theodore Ts'o --- fs/ext4/inode.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index 4ce35f1c8b0a..d7732320431a 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -789,10 +789,22 @@ int ext4_get_block(struct inode *inode, sector_t iblock, int ext4_get_block_unwritten(struct inode *inode, sector_t iblock, struct buffer_head *bh_result, int create) { + int ret = 0; + ext4_debug("ext4_get_block_unwritten: inode %lu, create flag %d\n", inode->i_ino, create); - return _ext4_get_block(inode, iblock, bh_result, + ret = _ext4_get_block(inode, iblock, bh_result, EXT4_GET_BLOCKS_CREATE_UNWRIT_EXT); + + /* + * If the buffer is marked unwritten, mark it as new to make sure it is + * zeroed out correctly in case of partial writes. Otherwise, there is + * a chance of stale data getting exposed. + */ + if (ret == 0 && buffer_unwritten(bh_result)) + set_buffer_new(bh_result); + + return ret; } /* Maximum number of blocks we map for direct IO at once. */ From 484fd6c1de13b336806a967908a927cc0356e312 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 19 Sep 2023 10:18:23 +0200 Subject: [PATCH 30/69] ext4: apply umask if ACL support is disabled The function ext4_init_acl() calls posix_acl_create() which is responsible for applying the umask. But without CONFIG_EXT4_FS_POSIX_ACL, ext4_init_acl() is an empty inline function, and nobody applies the umask. This fixes a bug which causes the umask to be ignored with O_TMPFILE on ext4: https://github.com/MusicPlayerDaemon/MPD/issues/558 https://bugs.gentoo.org/show_bug.cgi?id=686142#c3 https://bugzilla.kernel.org/show_bug.cgi?id=203625 Reviewed-by: "J. Bruce Fields" Cc: stable@vger.kernel.org Signed-off-by: Max Kellermann Link: https://lore.kernel.org/r/20230919081824.1096619-1-max.kellermann@ionos.com Signed-off-by: Theodore Ts'o --- fs/ext4/acl.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/fs/ext4/acl.h b/fs/ext4/acl.h index 0c5a79c3b5d4..ef4c19e5f570 100644 --- a/fs/ext4/acl.h +++ b/fs/ext4/acl.h @@ -68,6 +68,11 @@ extern int ext4_init_acl(handle_t *, struct inode *, struct inode *); static inline int ext4_init_acl(handle_t *handle, struct inode *inode, struct inode *dir) { + /* usually, the umask is applied by posix_acl_create(), but if + ext4 ACL support is disabled at compile time, we need to do + it here, because posix_acl_create() will never be called */ + inode->i_mode &= ~current_umask(); + return 0; } #endif /* CONFIG_EXT4_FS_POSIX_ACL */ From 61187fce8600e8ef90e601be84f9d0f3222c1206 Mon Sep 17 00:00:00 2001 From: Zhihao Cheng Date: Tue, 19 Sep 2023 09:25:25 +0800 Subject: [PATCH 31/69] jbd2: fix potential data lost in recovering journal raced with synchronizing fs bdev JBD2 makes sure journal data is fallen on fs device by sync_blockdev(), however, other process could intercept the EIO information from bdev's mapping, which leads journal recovering successful even EIO occurs during data written back to fs device. We found this problem in our product, iscsi + multipath is chosen for block device of ext4. Unstable network may trigger kpartx to rescan partitions in device mapper layer. Detailed process is shown as following: mount kpartx irq jbd2_journal_recover do_one_pass memcpy(nbh->b_data, obh->b_data) // copy data to fs dev from journal mark_buffer_dirty // mark bh dirty vfs_read generic_file_read_iter // dio filemap_write_and_wait_range __filemap_fdatawrite_range do_writepages block_write_full_folio submit_bh_wbc >> EIO occurs in disk << end_buffer_async_write mark_buffer_write_io_error mapping_set_error set_bit(AS_EIO, &mapping->flags) // set! filemap_check_errors test_and_clear_bit(AS_EIO, &mapping->flags) // clear! err2 = sync_blockdev filemap_write_and_wait filemap_check_errors test_and_clear_bit(AS_EIO, &mapping->flags) // false err2 = 0 Filesystem is mounted successfully even data from journal is failed written into disk, and ext4/ocfs2 could become corrupted. Fix it by comparing the wb_err state in fs block device before recovering and after recovering. A reproducer can be found in the kernel bugzilla referenced below. Link: https://bugzilla.kernel.org/show_bug.cgi?id=217888 Cc: stable@vger.kernel.org Signed-off-by: Zhihao Cheng Signed-off-by: Zhang Yi Reviewed-by: Jan Kara Link: https://lore.kernel.org/r/20230919012525.1783108-1-chengzhihao1@huawei.com Signed-off-by: Theodore Ts'o --- fs/jbd2/recovery.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c index 0567f1aa189b..01f744cb97a4 100644 --- a/fs/jbd2/recovery.c +++ b/fs/jbd2/recovery.c @@ -289,6 +289,8 @@ int jbd2_journal_recover(journal_t *journal) journal_superblock_t * sb; struct recovery_info info; + errseq_t wb_err; + struct address_space *mapping; memset(&info, 0, sizeof(info)); sb = journal->j_superblock; @@ -306,6 +308,9 @@ int jbd2_journal_recover(journal_t *journal) return 0; } + wb_err = 0; + mapping = journal->j_fs_dev->bd_inode->i_mapping; + errseq_check_and_advance(&mapping->wb_err, &wb_err); err = do_one_pass(journal, &info, PASS_SCAN); if (!err) err = do_one_pass(journal, &info, PASS_REVOKE); @@ -327,6 +332,9 @@ int jbd2_journal_recover(journal_t *journal) jbd2_journal_clear_revoke(journal); err2 = sync_blockdev(journal->j_fs_dev); + if (!err) + err = err2; + err2 = errseq_check_and_advance(&mapping->wb_err, &wb_err); if (!err) err = err2; /* Make sure all replayed data is on permanent storage */ From d2f7cf40ea89b486fb7a25f5ab9a5d3ce26fb4bb Mon Sep 17 00:00:00 2001 From: Kemeng Shi Date: Fri, 29 Sep 2023 00:03:56 +0800 Subject: [PATCH 32/69] ext4: make state in ext4_mb_mark_bb to be bool As state could only be either 0 or 1, just make it bool. Signed-off-by: Kemeng Shi Reviewed-by: "Ritesh Harjani (IBM)" Link: https://lore.kernel.org/r/20230928160407.142069-2-shikemeng@huaweicloud.com Signed-off-by: Theodore Ts'o --- fs/ext4/ext4.h | 2 +- fs/ext4/extents.c | 4 ++-- fs/ext4/fast_commit.c | 8 ++++---- fs/ext4/mballoc.c | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index 5ab49226cd9f..8ccebe0d4fbc 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -2926,7 +2926,7 @@ extern int ext4_group_add_blocks(handle_t *handle, struct super_block *sb, extern int ext4_trim_fs(struct super_block *, struct fstrim_range *); extern void ext4_process_freed_data(struct super_block *sb, tid_t commit_tid); extern void ext4_mb_mark_bb(struct super_block *sb, ext4_fsblk_t block, - int len, int state); + int len, bool state); static inline bool ext4_mb_cr_expensive(enum criteria cr) { return cr >= CR_GOAL_LEN_SLOW; diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index 4d8496d1a8ac..880f383df684 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -6080,13 +6080,13 @@ int ext4_ext_clear_bb(struct inode *inode) for (j = 0; j < path->p_depth; j++) { ext4_mb_mark_bb(inode->i_sb, - path[j].p_block, 1, 0); + path[j].p_block, 1, false); ext4_fc_record_regions(inode->i_sb, inode->i_ino, 0, path[j].p_block, 1, 1); } ext4_free_ext_path(path); } - ext4_mb_mark_bb(inode->i_sb, map.m_pblk, map.m_len, 0); + ext4_mb_mark_bb(inode->i_sb, map.m_pblk, map.m_len, false); ext4_fc_record_regions(inode->i_sb, inode->i_ino, map.m_lblk, map.m_pblk, map.m_len, 1); } diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c index b06de728b3b6..87c009e0c59a 100644 --- a/fs/ext4/fast_commit.c +++ b/fs/ext4/fast_commit.c @@ -1806,7 +1806,7 @@ static int ext4_fc_replay_add_range(struct super_block *sb, * at the end of the FC replay using our array of * modified inodes. */ - ext4_mb_mark_bb(inode->i_sb, map.m_pblk, map.m_len, 0); + ext4_mb_mark_bb(inode->i_sb, map.m_pblk, map.m_len, false); goto next; } @@ -1875,7 +1875,7 @@ ext4_fc_replay_del_range(struct super_block *sb, if (ret > 0) { remaining -= ret; cur += ret; - ext4_mb_mark_bb(inode->i_sb, map.m_pblk, map.m_len, 0); + ext4_mb_mark_bb(inode->i_sb, map.m_pblk, map.m_len, false); } else { remaining -= map.m_len; cur += map.m_len; @@ -1934,12 +1934,12 @@ static void ext4_fc_set_bitmaps_and_counters(struct super_block *sb) if (!IS_ERR(path)) { for (j = 0; j < path->p_depth; j++) ext4_mb_mark_bb(inode->i_sb, - path[j].p_block, 1, 1); + path[j].p_block, 1, true); ext4_free_ext_path(path); } cur += ret; ext4_mb_mark_bb(inode->i_sb, map.m_pblk, - map.m_len, 1); + map.m_len, true); } else { cur = cur + (map.m_len ? map.m_len : 1); } diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c index 6e304c18d390..dc2024dd3c27 100644 --- a/fs/ext4/mballoc.c +++ b/fs/ext4/mballoc.c @@ -4067,7 +4067,7 @@ out_err: * blocks in bitmaps and update counters. */ void ext4_mb_mark_bb(struct super_block *sb, ext4_fsblk_t block, - int len, int state) + int len, bool state) { struct buffer_head *bitmap_bh = NULL; struct ext4_group_desc *gdp; @@ -6095,7 +6095,7 @@ ext4_mb_new_blocks_simple(struct ext4_allocation_request *ar, int *errp) } block = ext4_group_first_block_no(sb, group) + EXT4_C2B(sbi, i); - ext4_mb_mark_bb(sb, block, 1, 1); + ext4_mb_mark_bb(sb, block, 1, true); ar->len = 1; return block; From f9e2d95a45321857bd0397aa07ae30cc20f0988f Mon Sep 17 00:00:00 2001 From: Kemeng Shi Date: Fri, 29 Sep 2023 00:03:57 +0800 Subject: [PATCH 33/69] ext4: factor out codes to update block bitmap and group descriptor on disk from ext4_mb_mark_bb There are several reasons to add a general function ext4_mb_mark_context to update block bitmap and group descriptor on disk: 1. pair behavior of alloc/free bits. For example, ext4_mb_new_blocks_simple will update free_clusters in struct flex_groups in ext4_mb_mark_bb while ext4_free_blocks_simple forgets this. 2. remove repeat code to read from disk, update and write back to disk. 3. reduce future unit test mocks to catch real IO to update structure on disk. Signed-off-by: Kemeng Shi Reviewed-by: "Ritesh Harjani (IBM)" Link: https://lore.kernel.org/r/20230928160407.142069-3-shikemeng@huaweicloud.com Signed-off-by: Theodore Ts'o --- fs/ext4/mballoc.c | 147 ++++++++++++++++++++++++---------------------- 1 file changed, 77 insertions(+), 70 deletions(-) diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c index dc2024dd3c27..1629bd494167 100644 --- a/fs/ext4/mballoc.c +++ b/fs/ext4/mballoc.c @@ -3943,6 +3943,80 @@ void ext4_exit_mballoc(void) ext4_groupinfo_destroy_slabs(); } +static int +ext4_mb_mark_context(struct super_block *sb, bool state, ext4_group_t group, + ext4_grpblk_t blkoff, ext4_grpblk_t len) +{ + struct ext4_sb_info *sbi = EXT4_SB(sb); + struct buffer_head *bitmap_bh = NULL; + struct ext4_group_desc *gdp; + struct buffer_head *gdp_bh; + int err; + unsigned int i, already, changed; + + bitmap_bh = ext4_read_block_bitmap(sb, group); + if (IS_ERR(bitmap_bh)) + return PTR_ERR(bitmap_bh); + + err = -EIO; + gdp = ext4_get_group_desc(sb, group, &gdp_bh); + if (!gdp) + goto out_err; + + ext4_lock_group(sb, group); + if (ext4_has_group_desc_csum(sb) && + (gdp->bg_flags & cpu_to_le16(EXT4_BG_BLOCK_UNINIT))) { + gdp->bg_flags &= cpu_to_le16(~EXT4_BG_BLOCK_UNINIT); + ext4_free_group_clusters_set(sb, gdp, + ext4_free_clusters_after_init(sb, group, gdp)); + } + + already = 0; + for (i = 0; i < len; i++) + if (mb_test_bit(blkoff + i, bitmap_bh->b_data) == + state) + already++; + changed = len - already; + + if (state) { + mb_set_bits(bitmap_bh->b_data, blkoff, len); + ext4_free_group_clusters_set(sb, gdp, + ext4_free_group_clusters(sb, gdp) - changed); + } else { + mb_clear_bits(bitmap_bh->b_data, blkoff, len); + ext4_free_group_clusters_set(sb, gdp, + ext4_free_group_clusters(sb, gdp) + changed); + } + + ext4_block_bitmap_csum_set(sb, gdp, bitmap_bh); + ext4_group_desc_csum_set(sb, group, gdp); + ext4_unlock_group(sb, group); + + if (sbi->s_log_groups_per_flex) { + ext4_group_t flex_group = ext4_flex_group(sbi, group); + struct flex_groups *fg = sbi_array_rcu_deref(sbi, + s_flex_groups, flex_group); + + if (state) + atomic64_sub(changed, &fg->free_clusters); + else + atomic64_add(changed, &fg->free_clusters); + } + + err = ext4_handle_dirty_metadata(NULL, NULL, bitmap_bh); + if (err) + goto out_err; + err = ext4_handle_dirty_metadata(NULL, NULL, gdp_bh); + if (err) + goto out_err; + + sync_dirty_buffer(bitmap_bh); + sync_dirty_buffer(gdp_bh); + +out_err: + brelse(bitmap_bh); + return err; +} /* * Check quota and mark chosen space (ac->ac_b_ex) non-free in bitmaps @@ -4069,15 +4143,11 @@ out_err: void ext4_mb_mark_bb(struct super_block *sb, ext4_fsblk_t block, int len, bool state) { - struct buffer_head *bitmap_bh = NULL; - struct ext4_group_desc *gdp; - struct buffer_head *gdp_bh; struct ext4_sb_info *sbi = EXT4_SB(sb); ext4_group_t group; ext4_grpblk_t blkoff; - int i, err = 0; - int already; - unsigned int clen, clen_changed, thisgrp_len; + int err = 0; + unsigned int clen, thisgrp_len; while (len > 0) { ext4_get_group_no_and_offset(sb, block, &group, &blkoff); @@ -4098,80 +4168,17 @@ void ext4_mb_mark_bb(struct super_block *sb, ext4_fsblk_t block, ext4_error(sb, "Marking blocks in system zone - " "Block = %llu, len = %u", block, thisgrp_len); - bitmap_bh = NULL; break; } - bitmap_bh = ext4_read_block_bitmap(sb, group); - if (IS_ERR(bitmap_bh)) { - err = PTR_ERR(bitmap_bh); - bitmap_bh = NULL; - break; - } - - err = -EIO; - gdp = ext4_get_group_desc(sb, group, &gdp_bh); - if (!gdp) - break; - - ext4_lock_group(sb, group); - already = 0; - for (i = 0; i < clen; i++) - if (!mb_test_bit(blkoff + i, bitmap_bh->b_data) == - !state) - already++; - - clen_changed = clen - already; - if (state) - mb_set_bits(bitmap_bh->b_data, blkoff, clen); - else - mb_clear_bits(bitmap_bh->b_data, blkoff, clen); - if (ext4_has_group_desc_csum(sb) && - (gdp->bg_flags & cpu_to_le16(EXT4_BG_BLOCK_UNINIT))) { - gdp->bg_flags &= cpu_to_le16(~EXT4_BG_BLOCK_UNINIT); - ext4_free_group_clusters_set(sb, gdp, - ext4_free_clusters_after_init(sb, group, gdp)); - } - if (state) - clen = ext4_free_group_clusters(sb, gdp) - clen_changed; - else - clen = ext4_free_group_clusters(sb, gdp) + clen_changed; - - ext4_free_group_clusters_set(sb, gdp, clen); - ext4_block_bitmap_csum_set(sb, gdp, bitmap_bh); - ext4_group_desc_csum_set(sb, group, gdp); - - ext4_unlock_group(sb, group); - - if (sbi->s_log_groups_per_flex) { - ext4_group_t flex_group = ext4_flex_group(sbi, group); - struct flex_groups *fg = sbi_array_rcu_deref(sbi, - s_flex_groups, flex_group); - - if (state) - atomic64_sub(clen_changed, &fg->free_clusters); - else - atomic64_add(clen_changed, &fg->free_clusters); - - } - - err = ext4_handle_dirty_metadata(NULL, NULL, bitmap_bh); - if (err) - break; - sync_dirty_buffer(bitmap_bh); - err = ext4_handle_dirty_metadata(NULL, NULL, gdp_bh); - sync_dirty_buffer(gdp_bh); + err = ext4_mb_mark_context(sb, state, group, blkoff, clen); if (err) break; block += thisgrp_len; len -= thisgrp_len; - brelse(bitmap_bh); BUG_ON(len < 0); } - - if (err) - brelse(bitmap_bh); } /* From 26d0f87b9fff37f9d4a04aa49f40b0d24502d2b1 Mon Sep 17 00:00:00 2001 From: Kemeng Shi Date: Fri, 29 Sep 2023 00:03:58 +0800 Subject: [PATCH 34/69] ext4: call ext4_mb_mark_context in ext4_free_blocks_simple call ext4_mb_mark_context in ext4_free_blocks_simple to: 1. remove repeat code 2. pair update of free_clusters in ext4_mb_new_blocks_simple. 3. add missing ext4_lock_group/ext4_unlock_group protection. Signed-off-by: Kemeng Shi Reviewed-by: "Ritesh Harjani (IBM)" Link: https://lore.kernel.org/r/20230928160407.142069-4-shikemeng@huaweicloud.com Signed-off-by: Theodore Ts'o --- fs/ext4/mballoc.c | 33 +-------------------------------- 1 file changed, 1 insertion(+), 32 deletions(-) diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c index 1629bd494167..3761934c9398 100644 --- a/fs/ext4/mballoc.c +++ b/fs/ext4/mballoc.c @@ -6358,43 +6358,12 @@ ext4_mb_free_metadata(handle_t *handle, struct ext4_buddy *e4b, static void ext4_free_blocks_simple(struct inode *inode, ext4_fsblk_t block, unsigned long count) { - struct buffer_head *bitmap_bh; struct super_block *sb = inode->i_sb; - struct ext4_group_desc *gdp; - struct buffer_head *gdp_bh; ext4_group_t group; ext4_grpblk_t blkoff; - int already_freed = 0, err, i; ext4_get_group_no_and_offset(sb, block, &group, &blkoff); - bitmap_bh = ext4_read_block_bitmap(sb, group); - if (IS_ERR(bitmap_bh)) { - pr_warn("Failed to read block bitmap\n"); - return; - } - gdp = ext4_get_group_desc(sb, group, &gdp_bh); - if (!gdp) - goto err_out; - - for (i = 0; i < count; i++) { - if (!mb_test_bit(blkoff + i, bitmap_bh->b_data)) - already_freed++; - } - mb_clear_bits(bitmap_bh->b_data, blkoff, count); - err = ext4_handle_dirty_metadata(NULL, NULL, bitmap_bh); - if (err) - goto err_out; - ext4_free_group_clusters_set( - sb, gdp, ext4_free_group_clusters(sb, gdp) + - count - already_freed); - ext4_block_bitmap_csum_set(sb, gdp, bitmap_bh); - ext4_group_desc_csum_set(sb, group, gdp); - ext4_handle_dirty_metadata(NULL, NULL, gdp_bh); - sync_dirty_buffer(bitmap_bh); - sync_dirty_buffer(gdp_bh); - -err_out: - brelse(bitmap_bh); + ext4_mb_mark_context(sb, false, group, blkoff, count); } /** From c431d3867e0a8258d1948408eb8297cdd2400e67 Mon Sep 17 00:00:00 2001 From: Kemeng Shi Date: Fri, 29 Sep 2023 00:03:59 +0800 Subject: [PATCH 35/69] ext4: extend ext4_mb_mark_context to support allocation under journal Previously, ext4_mb_mark_context is only called under fast commit replay path, so there is no valid handle when we update block bitmap and group descriptor. This patch try to extend ext4_mb_mark_context to be used by code under journal. There are several improvement: 1. Add "handle_t *handle" to struct ext4_mark_context to journal block bitmap and group descriptor update inside ext4_mb_mark_context (the added journal code is based on ext4_mb_mark_diskspace_used where ext4_mb_mark_context is going to be used.) 2. Adds a flag argument to ext4_mb_mark_context() which controls a. EXT4_MB_BITMAP_MARKED_CHECK - whether block bitmap checking is needed. b. EXT4_MB_SYNC_UPDATE - whether dirty buffers (bitmap and group descriptor) needs sync. Signed-off-by: Kemeng Shi Reviewed-by: "Ritesh Harjani (IBM)" Link: https://lore.kernel.org/r/20230928160407.142069-5-shikemeng@huaweicloud.com Signed-off-by: Theodore Ts'o --- fs/ext4/mballoc.c | 64 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 49 insertions(+), 15 deletions(-) diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c index 3761934c9398..5b3940e3afea 100644 --- a/fs/ext4/mballoc.c +++ b/fs/ext4/mballoc.c @@ -3943,26 +3943,47 @@ void ext4_exit_mballoc(void) ext4_groupinfo_destroy_slabs(); } +#define EXT4_MB_BITMAP_MARKED_CHECK 0x0001 +#define EXT4_MB_SYNC_UPDATE 0x0002 static int -ext4_mb_mark_context(struct super_block *sb, bool state, ext4_group_t group, - ext4_grpblk_t blkoff, ext4_grpblk_t len) +ext4_mb_mark_context(handle_t *handle, struct super_block *sb, bool state, + ext4_group_t group, ext4_grpblk_t blkoff, + ext4_grpblk_t len, int flags, ext4_grpblk_t *ret_changed) { struct ext4_sb_info *sbi = EXT4_SB(sb); struct buffer_head *bitmap_bh = NULL; struct ext4_group_desc *gdp; struct buffer_head *gdp_bh; int err; - unsigned int i, already, changed; + unsigned int i, already, changed = len; + if (ret_changed) + *ret_changed = 0; bitmap_bh = ext4_read_block_bitmap(sb, group); if (IS_ERR(bitmap_bh)) return PTR_ERR(bitmap_bh); + if (handle) { + BUFFER_TRACE(bitmap_bh, "getting write access"); + err = ext4_journal_get_write_access(handle, sb, bitmap_bh, + EXT4_JTR_NONE); + if (err) + goto out_err; + } + err = -EIO; gdp = ext4_get_group_desc(sb, group, &gdp_bh); if (!gdp) goto out_err; + if (handle) { + BUFFER_TRACE(gdp_bh, "get_write_access"); + err = ext4_journal_get_write_access(handle, sb, gdp_bh, + EXT4_JTR_NONE); + if (err) + goto out_err; + } + ext4_lock_group(sb, group); if (ext4_has_group_desc_csum(sb) && (gdp->bg_flags & cpu_to_le16(EXT4_BG_BLOCK_UNINIT))) { @@ -3971,12 +3992,14 @@ ext4_mb_mark_context(struct super_block *sb, bool state, ext4_group_t group, ext4_free_clusters_after_init(sb, group, gdp)); } - already = 0; - for (i = 0; i < len; i++) - if (mb_test_bit(blkoff + i, bitmap_bh->b_data) == - state) - already++; - changed = len - already; + if (flags & EXT4_MB_BITMAP_MARKED_CHECK) { + already = 0; + for (i = 0; i < len; i++) + if (mb_test_bit(blkoff + i, bitmap_bh->b_data) == + state) + already++; + changed = len - already; + } if (state) { mb_set_bits(bitmap_bh->b_data, blkoff, len); @@ -3991,6 +4014,8 @@ ext4_mb_mark_context(struct super_block *sb, bool state, ext4_group_t group, ext4_block_bitmap_csum_set(sb, gdp, bitmap_bh); ext4_group_desc_csum_set(sb, group, gdp); ext4_unlock_group(sb, group); + if (ret_changed) + *ret_changed = changed; if (sbi->s_log_groups_per_flex) { ext4_group_t flex_group = ext4_flex_group(sbi, group); @@ -4003,15 +4028,17 @@ ext4_mb_mark_context(struct super_block *sb, bool state, ext4_group_t group, atomic64_add(changed, &fg->free_clusters); } - err = ext4_handle_dirty_metadata(NULL, NULL, bitmap_bh); + err = ext4_handle_dirty_metadata(handle, NULL, bitmap_bh); if (err) goto out_err; - err = ext4_handle_dirty_metadata(NULL, NULL, gdp_bh); + err = ext4_handle_dirty_metadata(handle, NULL, gdp_bh); if (err) goto out_err; - sync_dirty_buffer(bitmap_bh); - sync_dirty_buffer(gdp_bh); + if (flags & EXT4_MB_SYNC_UPDATE) { + sync_dirty_buffer(bitmap_bh); + sync_dirty_buffer(gdp_bh); + } out_err: brelse(bitmap_bh); @@ -4171,7 +4198,11 @@ void ext4_mb_mark_bb(struct super_block *sb, ext4_fsblk_t block, break; } - err = ext4_mb_mark_context(sb, state, group, blkoff, clen); + err = ext4_mb_mark_context(NULL, sb, state, + group, blkoff, clen, + EXT4_MB_BITMAP_MARKED_CHECK | + EXT4_MB_SYNC_UPDATE, + NULL); if (err) break; @@ -6363,7 +6394,10 @@ static void ext4_free_blocks_simple(struct inode *inode, ext4_fsblk_t block, ext4_grpblk_t blkoff; ext4_get_group_no_and_offset(sb, block, &group, &blkoff); - ext4_mb_mark_context(sb, false, group, blkoff, count); + ext4_mb_mark_context(NULL, sb, false, group, blkoff, count, + EXT4_MB_BITMAP_MARKED_CHECK | + EXT4_MB_SYNC_UPDATE, + NULL); } /** From 2f94711b098bc55285ac74aba2f2b83fd72b555f Mon Sep 17 00:00:00 2001 From: Kemeng Shi Date: Fri, 29 Sep 2023 00:04:00 +0800 Subject: [PATCH 36/69] ext4: call ext4_mb_mark_context in ext4_mb_mark_diskspace_used Call ext4_mb_mark_context in ext4_mb_mark_diskspace_used to: 1. Remove repeat code to normally update bitmap and group descriptor on disk. 2. Now that we have a common API for marking blocks inuse/free in block bitmap, use that instead of open coding it in function ext4_mb_mark_diskspace_used(). The current code was not updating checksum and other counters. ext4_mb_mark_context() should fix these consistency problems. Signed-off-by: Kemeng Shi Reviewed-by: "Ritesh Harjani (IBM)" Link: https://lore.kernel.org/r/20230928160407.142069-6-shikemeng@huaweicloud.com Signed-off-by: Theodore Ts'o --- fs/ext4/mballoc.c | 86 +++++++++++------------------------------------ 1 file changed, 20 insertions(+), 66 deletions(-) diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c index 5b3940e3afea..7d1b78fd6c97 100644 --- a/fs/ext4/mballoc.c +++ b/fs/ext4/mballoc.c @@ -4053,13 +4053,13 @@ static noinline_for_stack int ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac, handle_t *handle, unsigned int reserv_clstrs) { - struct buffer_head *bitmap_bh = NULL; struct ext4_group_desc *gdp; - struct buffer_head *gdp_bh; struct ext4_sb_info *sbi; struct super_block *sb; ext4_fsblk_t block; int err, len; + int flags = 0; + ext4_grpblk_t changed; BUG_ON(ac->ac_status != AC_STATUS_FOUND); BUG_ON(ac->ac_b_ex.fe_len <= 0); @@ -4067,32 +4067,13 @@ ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac, sb = ac->ac_sb; sbi = EXT4_SB(sb); - bitmap_bh = ext4_read_block_bitmap(sb, ac->ac_b_ex.fe_group); - if (IS_ERR(bitmap_bh)) { - return PTR_ERR(bitmap_bh); - } - - BUFFER_TRACE(bitmap_bh, "getting write access"); - err = ext4_journal_get_write_access(handle, sb, bitmap_bh, - EXT4_JTR_NONE); - if (err) - goto out_err; - - err = -EIO; - gdp = ext4_get_group_desc(sb, ac->ac_b_ex.fe_group, &gdp_bh); + gdp = ext4_get_group_desc(sb, ac->ac_b_ex.fe_group, NULL); if (!gdp) - goto out_err; - + return -EIO; ext4_debug("using block group %u(%d)\n", ac->ac_b_ex.fe_group, ext4_free_group_clusters(sb, gdp)); - BUFFER_TRACE(gdp_bh, "get_write_access"); - err = ext4_journal_get_write_access(handle, sb, gdp_bh, EXT4_JTR_NONE); - if (err) - goto out_err; - block = ext4_grp_offs_to_block(sb, &ac->ac_b_ex); - len = EXT4_C2B(sbi, ac->ac_b_ex.fe_len); if (!ext4_inode_block_valid(ac->ac_inode, block, len)) { ext4_error(sb, "Allocating blocks %llu-%llu which overlap " @@ -4101,41 +4082,29 @@ ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac, * Fix the bitmap and return EFSCORRUPTED * We leak some of the blocks here. */ - ext4_lock_group(sb, ac->ac_b_ex.fe_group); - mb_set_bits(bitmap_bh->b_data, ac->ac_b_ex.fe_start, - ac->ac_b_ex.fe_len); - ext4_unlock_group(sb, ac->ac_b_ex.fe_group); - err = ext4_handle_dirty_metadata(handle, NULL, bitmap_bh); + err = ext4_mb_mark_context(handle, sb, true, + ac->ac_b_ex.fe_group, + ac->ac_b_ex.fe_start, + ac->ac_b_ex.fe_len, + 0, NULL); if (!err) err = -EFSCORRUPTED; - goto out_err; + return err; } - ext4_lock_group(sb, ac->ac_b_ex.fe_group); #ifdef AGGRESSIVE_CHECK - { - int i; - for (i = 0; i < ac->ac_b_ex.fe_len; i++) { - BUG_ON(mb_test_bit(ac->ac_b_ex.fe_start + i, - bitmap_bh->b_data)); - } - } + flags |= EXT4_MB_BITMAP_MARKED_CHECK; #endif - mb_set_bits(bitmap_bh->b_data, ac->ac_b_ex.fe_start, - ac->ac_b_ex.fe_len); - if (ext4_has_group_desc_csum(sb) && - (gdp->bg_flags & cpu_to_le16(EXT4_BG_BLOCK_UNINIT))) { - gdp->bg_flags &= cpu_to_le16(~EXT4_BG_BLOCK_UNINIT); - ext4_free_group_clusters_set(sb, gdp, - ext4_free_clusters_after_init(sb, - ac->ac_b_ex.fe_group, gdp)); - } - len = ext4_free_group_clusters(sb, gdp) - ac->ac_b_ex.fe_len; - ext4_free_group_clusters_set(sb, gdp, len); - ext4_block_bitmap_csum_set(sb, gdp, bitmap_bh); - ext4_group_desc_csum_set(sb, ac->ac_b_ex.fe_group, gdp); + err = ext4_mb_mark_context(handle, sb, true, ac->ac_b_ex.fe_group, + ac->ac_b_ex.fe_start, ac->ac_b_ex.fe_len, + flags, &changed); - ext4_unlock_group(sb, ac->ac_b_ex.fe_group); + if (err && changed == 0) + return err; + +#ifdef AGGRESSIVE_CHECK + BUG_ON(changed != ac->ac_b_ex.fe_len); +#endif percpu_counter_sub(&sbi->s_freeclusters_counter, ac->ac_b_ex.fe_len); /* * Now reduce the dirty block count also. Should not go negative @@ -4145,21 +4114,6 @@ ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac, percpu_counter_sub(&sbi->s_dirtyclusters_counter, reserv_clstrs); - if (sbi->s_log_groups_per_flex) { - ext4_group_t flex_group = ext4_flex_group(sbi, - ac->ac_b_ex.fe_group); - atomic64_sub(ac->ac_b_ex.fe_len, - &sbi_array_rcu_deref(sbi, s_flex_groups, - flex_group)->free_clusters); - } - - err = ext4_handle_dirty_metadata(handle, NULL, bitmap_bh); - if (err) - goto out_err; - err = ext4_handle_dirty_metadata(handle, NULL, gdp_bh); - -out_err: - brelse(bitmap_bh); return err; } From 33e728c67db6b7e72fc8d09f7a38f72302b8a98d Mon Sep 17 00:00:00 2001 From: Kemeng Shi Date: Fri, 29 Sep 2023 00:04:01 +0800 Subject: [PATCH 37/69] ext4: Separate block bitmap and buddy bitmap freeing in ext4_mb_clear_bb() This patch separates block bitmap and buddy bitmap freeing in order to update block bitmap with ext4_mb_mark_context in following patch. Separated freeing is safe with concurrent allocation as long as: 1. Firstly allocate block in buddy bitmap, and then in block bitmap. 2. Firstly free block in block bitmap, and then buddy bitmap. Then freed block will only be available to allocation when both buddy bitmap and block bitmap are updated by freeing. Allocation obeys rule 1 already, just do sperated freeing with rule 2. Separated freeing has no race with generate_buddy as: Once ext4_mb_load_buddy_gfp is executed successfully, the update-to-date buddy page can be found in sbi->s_buddy_cache and no more buddy initialization of the buddy page will be executed concurrently until buddy page is unloaded. As we always do free in "load buddy, free, unload buddy" sequence, separated freeing has no race with generate_buddy. Signed-off-by: Kemeng Shi Reviewed-by: "Ritesh Harjani (IBM)" Link: https://lore.kernel.org/r/20230928160407.142069-7-shikemeng@huaweicloud.com Signed-off-by: Theodore Ts'o --- fs/ext4/mballoc.c | 102 +++++++++++++++++++++++----------------------- 1 file changed, 51 insertions(+), 51 deletions(-) diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c index 7d1b78fd6c97..124bc5a90e79 100644 --- a/fs/ext4/mballoc.c +++ b/fs/ext4/mballoc.c @@ -6388,7 +6388,7 @@ static void ext4_mb_clear_bb(handle_t *handle, struct inode *inode, ext4_error(sb, "Freeing blocks in system zone - " "Block = %llu, count = %lu", block, count); /* err = 0. ext4_std_error should be a no op */ - goto error_return; + goto error_out; } flags |= EXT4_FREE_BLOCKS_VALIDATED; @@ -6412,31 +6412,39 @@ do_more: flags &= ~EXT4_FREE_BLOCKS_VALIDATED; } count_clusters = EXT4_NUM_B2C(sbi, count); - bitmap_bh = ext4_read_block_bitmap(sb, block_group); - if (IS_ERR(bitmap_bh)) { - err = PTR_ERR(bitmap_bh); - bitmap_bh = NULL; - goto error_return; - } - gdp = ext4_get_group_desc(sb, block_group, &gd_bh); - if (!gdp) { - err = -EIO; - goto error_return; - } + trace_ext4_mballoc_free(sb, inode, block_group, bit, count_clusters); + + /* __GFP_NOFAIL: retry infinitely, ignore TIF_MEMDIE and memcg limit. */ + err = ext4_mb_load_buddy_gfp(sb, block_group, &e4b, + GFP_NOFS|__GFP_NOFAIL); + if (err) + goto error_out; if (!(flags & EXT4_FREE_BLOCKS_VALIDATED) && !ext4_inode_block_valid(inode, 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 */ - goto error_return; + goto error_clean; + } + + bitmap_bh = ext4_read_block_bitmap(sb, block_group); + if (IS_ERR(bitmap_bh)) { + err = PTR_ERR(bitmap_bh); + bitmap_bh = NULL; + goto error_clean; + } + gdp = ext4_get_group_desc(sb, block_group, &gd_bh); + if (!gdp) { + err = -EIO; + goto error_clean; } BUFFER_TRACE(bitmap_bh, "getting write access"); err = ext4_journal_get_write_access(handle, sb, bitmap_bh, EXT4_JTR_NONE); if (err) - goto error_return; + goto error_clean; /* * We are about to modify some metadata. Call the journal APIs @@ -6446,7 +6454,7 @@ do_more: BUFFER_TRACE(gd_bh, "get_write_access"); err = ext4_journal_get_write_access(handle, sb, gd_bh, EXT4_JTR_NONE); if (err) - goto error_return; + goto error_clean; #ifdef AGGRESSIVE_CHECK { int i; @@ -6454,13 +6462,30 @@ do_more: BUG_ON(!mb_test_bit(bit + i, bitmap_bh->b_data)); } #endif - trace_ext4_mballoc_free(sb, inode, block_group, bit, count_clusters); + ext4_lock_group(sb, block_group); + mb_clear_bits(bitmap_bh->b_data, bit, count_clusters); + ret = ext4_free_group_clusters(sb, gdp) + count_clusters; + ext4_free_group_clusters_set(sb, gdp, ret); + ext4_block_bitmap_csum_set(sb, gdp, bitmap_bh); + ext4_group_desc_csum_set(sb, block_group, gdp); + ext4_unlock_group(sb, block_group); - /* __GFP_NOFAIL: retry infinitely, ignore TIF_MEMDIE and memcg limit. */ - err = ext4_mb_load_buddy_gfp(sb, block_group, &e4b, - GFP_NOFS|__GFP_NOFAIL); - if (err) - goto error_return; + if (sbi->s_log_groups_per_flex) { + ext4_group_t flex_group = ext4_flex_group(sbi, block_group); + atomic64_add(count_clusters, + &sbi_array_rcu_deref(sbi, s_flex_groups, + flex_group)->free_clusters); + } + + /* We dirtied the bitmap block */ + BUFFER_TRACE(bitmap_bh, "dirtied bitmap block"); + err = ext4_handle_dirty_metadata(handle, NULL, bitmap_bh); + + /* And the group descriptor block */ + BUFFER_TRACE(gd_bh, "dirtied group descriptor block"); + ret = ext4_handle_dirty_metadata(handle, NULL, gd_bh); + if (!err) + err = ret; /* * We need to make sure we don't reuse the freed block until after the @@ -6484,13 +6509,8 @@ do_more: new_entry->efd_tid = handle->h_transaction->t_tid; ext4_lock_group(sb, block_group); - mb_clear_bits(bitmap_bh->b_data, bit, count_clusters); ext4_mb_free_metadata(handle, &e4b, new_entry); } else { - /* need to update group_info->bb_free and bitmap - * with group lock held. generate_buddy look at - * them with group lock_held - */ if (test_opt(sb, DISCARD)) { err = ext4_issue_discard(sb, block_group, bit, count_clusters, NULL); @@ -6503,23 +6523,11 @@ do_more: EXT4_MB_GRP_CLEAR_TRIMMED(e4b.bd_info); ext4_lock_group(sb, block_group); - mb_clear_bits(bitmap_bh->b_data, bit, count_clusters); mb_free_blocks(inode, &e4b, bit, count_clusters); } - ret = ext4_free_group_clusters(sb, gdp) + count_clusters; - ext4_free_group_clusters_set(sb, gdp, ret); - ext4_block_bitmap_csum_set(sb, gdp, bitmap_bh); - ext4_group_desc_csum_set(sb, block_group, gdp); ext4_unlock_group(sb, block_group); - if (sbi->s_log_groups_per_flex) { - ext4_group_t flex_group = ext4_flex_group(sbi, block_group); - atomic64_add(count_clusters, - &sbi_array_rcu_deref(sbi, s_flex_groups, - flex_group)->free_clusters); - } - /* * on a bigalloc file system, defer the s_freeclusters_counter * update to the caller (ext4_remove_space and friends) so they @@ -6532,28 +6540,20 @@ do_more: count_clusters); } - ext4_mb_unload_buddy(&e4b); - - /* We dirtied the bitmap block */ - BUFFER_TRACE(bitmap_bh, "dirtied bitmap block"); - err = ext4_handle_dirty_metadata(handle, NULL, bitmap_bh); - - /* And the group descriptor block */ - BUFFER_TRACE(gd_bh, "dirtied group descriptor block"); - ret = ext4_handle_dirty_metadata(handle, NULL, gd_bh); - if (!err) - err = ret; - if (overflow && !err) { block += count; count = overflow; + ext4_mb_unload_buddy(&e4b); put_bh(bitmap_bh); /* The range changed so it's no longer validated */ flags &= ~EXT4_FREE_BLOCKS_VALIDATED; goto do_more; } -error_return: + +error_clean: + ext4_mb_unload_buddy(&e4b); brelse(bitmap_bh); +error_out: ext4_std_error(sb, err); } From 38b8f70cd28c6ad21192d82f271b9e32bdcb8600 Mon Sep 17 00:00:00 2001 From: Kemeng Shi Date: Fri, 29 Sep 2023 00:04:02 +0800 Subject: [PATCH 38/69] ext4: call ext4_mb_mark_context in ext4_mb_clear_bb Call ext4_mb_mark_context in ext4_mb_clear_bb to remove repeat code. Signed-off-by: Kemeng Shi Reviewed-by: "Ritesh Harjani (IBM)" Link: https://lore.kernel.org/r/20230928160407.142069-8-shikemeng@huaweicloud.com Signed-off-by: Theodore Ts'o --- fs/ext4/mballoc.c | 69 +++++++---------------------------------------- 1 file changed, 10 insertions(+), 59 deletions(-) diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c index 124bc5a90e79..d90fb5660429 100644 --- a/fs/ext4/mballoc.c +++ b/fs/ext4/mballoc.c @@ -6367,19 +6367,17 @@ static void ext4_mb_clear_bb(handle_t *handle, struct inode *inode, ext4_fsblk_t block, unsigned long count, int flags) { - struct buffer_head *bitmap_bh = NULL; struct super_block *sb = inode->i_sb; - struct ext4_group_desc *gdp; struct ext4_group_info *grp; unsigned int overflow; ext4_grpblk_t bit; - struct buffer_head *gd_bh; ext4_group_t block_group; struct ext4_sb_info *sbi; struct ext4_buddy e4b; unsigned int count_clusters; int err = 0; - int ret; + int mark_flags = 0; + ext4_grpblk_t changed; sbi = EXT4_SB(sb); @@ -6428,64 +6426,19 @@ do_more: goto error_clean; } - bitmap_bh = ext4_read_block_bitmap(sb, block_group); - if (IS_ERR(bitmap_bh)) { - err = PTR_ERR(bitmap_bh); - bitmap_bh = NULL; - goto error_clean; - } - gdp = ext4_get_group_desc(sb, block_group, &gd_bh); - if (!gdp) { - err = -EIO; - goto error_clean; - } - - BUFFER_TRACE(bitmap_bh, "getting write access"); - err = ext4_journal_get_write_access(handle, sb, bitmap_bh, - EXT4_JTR_NONE); - if (err) - goto error_clean; - - /* - * We are about to modify some metadata. Call the journal APIs - * to unshare ->b_data if a currently-committing transaction is - * using it - */ - BUFFER_TRACE(gd_bh, "get_write_access"); - err = ext4_journal_get_write_access(handle, sb, gd_bh, EXT4_JTR_NONE); - if (err) - goto error_clean; #ifdef AGGRESSIVE_CHECK - { - int i; - for (i = 0; i < count_clusters; i++) - BUG_ON(!mb_test_bit(bit + i, bitmap_bh->b_data)); - } + mark_flags |= EXT4_MB_BITMAP_MARKED_CHECK; #endif - ext4_lock_group(sb, block_group); - mb_clear_bits(bitmap_bh->b_data, bit, count_clusters); - ret = ext4_free_group_clusters(sb, gdp) + count_clusters; - ext4_free_group_clusters_set(sb, gdp, ret); - ext4_block_bitmap_csum_set(sb, gdp, bitmap_bh); - ext4_group_desc_csum_set(sb, block_group, gdp); - ext4_unlock_group(sb, block_group); + err = ext4_mb_mark_context(handle, sb, false, block_group, bit, + count_clusters, mark_flags, &changed); - if (sbi->s_log_groups_per_flex) { - ext4_group_t flex_group = ext4_flex_group(sbi, block_group); - atomic64_add(count_clusters, - &sbi_array_rcu_deref(sbi, s_flex_groups, - flex_group)->free_clusters); - } - /* We dirtied the bitmap block */ - BUFFER_TRACE(bitmap_bh, "dirtied bitmap block"); - err = ext4_handle_dirty_metadata(handle, NULL, bitmap_bh); + if (err && changed == 0) + goto error_clean; - /* And the group descriptor block */ - BUFFER_TRACE(gd_bh, "dirtied group descriptor block"); - ret = ext4_handle_dirty_metadata(handle, NULL, gd_bh); - if (!err) - err = ret; +#ifdef AGGRESSIVE_CHECK + BUG_ON(changed != count_clusters); +#endif /* * We need to make sure we don't reuse the freed block until after the @@ -6544,7 +6497,6 @@ do_more: block += count; count = overflow; ext4_mb_unload_buddy(&e4b); - put_bh(bitmap_bh); /* The range changed so it's no longer validated */ flags &= ~EXT4_FREE_BLOCKS_VALIDATED; goto do_more; @@ -6552,7 +6504,6 @@ do_more: error_clean: ext4_mb_unload_buddy(&e4b); - brelse(bitmap_bh); error_out: ext4_std_error(sb, err); } From 03c7fc39a677cdeae076d8b9ecc10920dd13f6a8 Mon Sep 17 00:00:00 2001 From: Kemeng Shi Date: Fri, 29 Sep 2023 00:04:03 +0800 Subject: [PATCH 39/69] ext4: Separate block bitmap and buddy bitmap freeing in ext4_group_add_blocks() This patch separates block bitmap and buddy bitmap freeing in order to update block bitmap with ext4_mb_mark_context in following patch. The reason why this can be sperated is explained in previous submit. Put the explanation here to simplify the code archeology to ext4_group_add_blocks(): Separated freeing is safe with concurrent allocation as long as: 1. Firstly allocate block in buddy bitmap, and then in block bitmap. 2. Firstly free block in block bitmap, and then buddy bitmap. Then freed block will only be available to allocation when both buddy bitmap and block bitmap are updated by freeing. Allocation obeys rule 1 already, just do sperated freeing with rule 2. Separated freeing has no race with generate_buddy as: Once ext4_mb_load_buddy_gfp is executed successfully, the update-to-date buddy page can be found in sbi->s_buddy_cache and no more buddy initialization of the buddy page will be executed concurrently until buddy page is unloaded. As we always do free in "load buddy, free, unload buddy" sequence, separated freeing has no race with generate_buddy. Signed-off-by: Kemeng Shi Reviewed-by: "Ritesh Harjani (IBM)" Link: https://lore.kernel.org/r/20230928160407.142069-9-shikemeng@huaweicloud.com Signed-off-by: Theodore Ts'o --- fs/ext4/mballoc.c | 60 +++++++++++++++++++++++------------------------ 1 file changed, 29 insertions(+), 31 deletions(-) diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c index d90fb5660429..e866929a9760 100644 --- a/fs/ext4/mballoc.c +++ b/fs/ext4/mballoc.c @@ -6650,35 +6650,39 @@ int ext4_group_add_blocks(handle_t *handle, struct super_block *sb, ext4_warning(sb, "too many blocks added to group %u", block_group); err = -EINVAL; - goto error_return; + goto error_out; } - bitmap_bh = ext4_read_block_bitmap(sb, block_group); - if (IS_ERR(bitmap_bh)) { - err = PTR_ERR(bitmap_bh); - bitmap_bh = NULL; - goto error_return; - } - - desc = ext4_get_group_desc(sb, block_group, &gd_bh); - if (!desc) { - err = -EIO; - goto error_return; - } + err = ext4_mb_load_buddy(sb, block_group, &e4b); + if (err) + goto error_out; if (!ext4_sb_block_valid(sb, NULL, block, count)) { ext4_error(sb, "Adding blocks in system zones - " "Block = %llu, count = %lu", block, count); err = -EINVAL; - goto error_return; + goto error_clean; + } + + bitmap_bh = ext4_read_block_bitmap(sb, block_group); + if (IS_ERR(bitmap_bh)) { + err = PTR_ERR(bitmap_bh); + bitmap_bh = NULL; + goto error_clean; + } + + desc = ext4_get_group_desc(sb, block_group, &gd_bh); + if (!desc) { + err = -EIO; + goto error_clean; } BUFFER_TRACE(bitmap_bh, "getting write access"); err = ext4_journal_get_write_access(handle, sb, bitmap_bh, EXT4_JTR_NONE); if (err) - goto error_return; + goto error_clean; /* * We are about to modify some metadata. Call the journal APIs @@ -6688,7 +6692,7 @@ int ext4_group_add_blocks(handle_t *handle, struct super_block *sb, BUFFER_TRACE(gd_bh, "get_write_access"); err = ext4_journal_get_write_access(handle, sb, gd_bh, EXT4_JTR_NONE); if (err) - goto error_return; + goto error_clean; for (i = 0, clusters_freed = 0; i < cluster_count; i++) { BUFFER_TRACE(bitmap_bh, "clear bit"); @@ -6701,26 +6705,14 @@ int ext4_group_add_blocks(handle_t *handle, struct super_block *sb, } } - err = ext4_mb_load_buddy(sb, block_group, &e4b); - if (err) - goto error_return; - - /* - * need to update group_info->bb_free and bitmap - * with group lock held. generate_buddy look at - * them with group lock_held - */ ext4_lock_group(sb, block_group); mb_clear_bits(bitmap_bh->b_data, bit, cluster_count); - mb_free_blocks(NULL, &e4b, bit, cluster_count); free_clusters_count = clusters_freed + ext4_free_group_clusters(sb, desc); ext4_free_group_clusters_set(sb, desc, free_clusters_count); ext4_block_bitmap_csum_set(sb, desc, bitmap_bh); ext4_group_desc_csum_set(sb, block_group, desc); ext4_unlock_group(sb, block_group); - percpu_counter_add(&sbi->s_freeclusters_counter, - clusters_freed); if (sbi->s_log_groups_per_flex) { ext4_group_t flex_group = ext4_flex_group(sbi, block_group); @@ -6729,8 +6721,6 @@ int ext4_group_add_blocks(handle_t *handle, struct super_block *sb, flex_group)->free_clusters); } - ext4_mb_unload_buddy(&e4b); - /* We dirtied the bitmap block */ BUFFER_TRACE(bitmap_bh, "dirtied bitmap block"); err = ext4_handle_dirty_metadata(handle, NULL, bitmap_bh); @@ -6741,8 +6731,16 @@ int ext4_group_add_blocks(handle_t *handle, struct super_block *sb, if (!err) err = ret; -error_return: + ext4_lock_group(sb, block_group); + mb_free_blocks(NULL, &e4b, bit, cluster_count); + ext4_unlock_group(sb, block_group); + percpu_counter_add(&sbi->s_freeclusters_counter, + clusters_freed); + +error_clean: brelse(bitmap_bh); + ext4_mb_unload_buddy(&e4b); +error_out: ext4_std_error(sb, err); return err; } From 5c657db46d9ebc63e57be1408f1986c9b23368e1 Mon Sep 17 00:00:00 2001 From: Kemeng Shi Date: Fri, 29 Sep 2023 00:04:04 +0800 Subject: [PATCH 40/69] ext4: call ext4_mb_mark_context in ext4_group_add_blocks() Call ext4_mb_mark_context in ext4_group_add_blocks() to remove repeat code. Signed-off-by: Kemeng Shi Reviewed-by: "Ritesh Harjani (IBM)" Link: https://lore.kernel.org/r/20230928160407.142069-10-shikemeng@huaweicloud.com Signed-off-by: Theodore Ts'o --- fs/ext4/mballoc.c | 82 ++++++----------------------------------------- 1 file changed, 10 insertions(+), 72 deletions(-) diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c index e866929a9760..b26e4d64cd4c 100644 --- a/fs/ext4/mballoc.c +++ b/fs/ext4/mballoc.c @@ -6622,23 +6622,19 @@ void ext4_free_blocks(handle_t *handle, struct inode *inode, int ext4_group_add_blocks(handle_t *handle, struct super_block *sb, ext4_fsblk_t block, unsigned long count) { - struct buffer_head *bitmap_bh = NULL; - struct buffer_head *gd_bh; ext4_group_t block_group; ext4_grpblk_t bit; - unsigned int i; - struct ext4_group_desc *desc; struct ext4_sb_info *sbi = EXT4_SB(sb); struct ext4_buddy e4b; - int err = 0, ret, free_clusters_count; - ext4_grpblk_t clusters_freed; + int err = 0; ext4_fsblk_t first_cluster = EXT4_B2C(sbi, block); ext4_fsblk_t last_cluster = EXT4_B2C(sbi, block + count - 1); unsigned long cluster_count = last_cluster - first_cluster + 1; + ext4_grpblk_t changed; ext4_debug("Adding block(s) %llu-%llu\n", block, block + count - 1); - if (count == 0) + if (cluster_count == 0) return 0; ext4_get_group_no_and_offset(sb, block, &block_group, &bit); @@ -6665,80 +6661,22 @@ int ext4_group_add_blocks(handle_t *handle, struct super_block *sb, goto error_clean; } - bitmap_bh = ext4_read_block_bitmap(sb, block_group); - if (IS_ERR(bitmap_bh)) { - err = PTR_ERR(bitmap_bh); - bitmap_bh = NULL; - goto error_clean; - } - - desc = ext4_get_group_desc(sb, block_group, &gd_bh); - if (!desc) { - err = -EIO; - goto error_clean; - } - - BUFFER_TRACE(bitmap_bh, "getting write access"); - err = ext4_journal_get_write_access(handle, sb, bitmap_bh, - EXT4_JTR_NONE); - if (err) + err = ext4_mb_mark_context(handle, sb, false, block_group, bit, + cluster_count, EXT4_MB_BITMAP_MARKED_CHECK, + &changed); + if (err && changed == 0) goto error_clean; - /* - * We are about to modify some metadata. Call the journal APIs - * to unshare ->b_data if a currently-committing transaction is - * using it - */ - BUFFER_TRACE(gd_bh, "get_write_access"); - err = ext4_journal_get_write_access(handle, sb, gd_bh, EXT4_JTR_NONE); - if (err) - goto error_clean; - - for (i = 0, clusters_freed = 0; i < cluster_count; i++) { - BUFFER_TRACE(bitmap_bh, "clear bit"); - if (!mb_test_bit(bit + i, bitmap_bh->b_data)) { - ext4_error(sb, "bit already cleared for block %llu", - (ext4_fsblk_t)(block + i)); - BUFFER_TRACE(bitmap_bh, "bit already cleared"); - } else { - clusters_freed++; - } - } - - ext4_lock_group(sb, block_group); - mb_clear_bits(bitmap_bh->b_data, bit, cluster_count); - free_clusters_count = clusters_freed + - ext4_free_group_clusters(sb, desc); - ext4_free_group_clusters_set(sb, desc, free_clusters_count); - ext4_block_bitmap_csum_set(sb, desc, bitmap_bh); - ext4_group_desc_csum_set(sb, block_group, desc); - ext4_unlock_group(sb, block_group); - - if (sbi->s_log_groups_per_flex) { - ext4_group_t flex_group = ext4_flex_group(sbi, block_group); - atomic64_add(clusters_freed, - &sbi_array_rcu_deref(sbi, s_flex_groups, - flex_group)->free_clusters); - } - - /* We dirtied the bitmap block */ - BUFFER_TRACE(bitmap_bh, "dirtied bitmap block"); - err = ext4_handle_dirty_metadata(handle, NULL, bitmap_bh); - - /* And the group descriptor block */ - BUFFER_TRACE(gd_bh, "dirtied group descriptor block"); - ret = ext4_handle_dirty_metadata(handle, NULL, gd_bh); - if (!err) - err = ret; + if (changed != cluster_count) + ext4_error(sb, "bit already cleared in group %u", block_group); ext4_lock_group(sb, block_group); mb_free_blocks(NULL, &e4b, bit, cluster_count); ext4_unlock_group(sb, block_group); percpu_counter_add(&sbi->s_freeclusters_counter, - clusters_freed); + changed); error_clean: - brelse(bitmap_bh); ext4_mb_unload_buddy(&e4b); error_out: ext4_std_error(sb, err); From bdefd689b7ff0eadc3b29dc6c66556617bd1ed42 Mon Sep 17 00:00:00 2001 From: Kemeng Shi Date: Fri, 29 Sep 2023 00:04:05 +0800 Subject: [PATCH 41/69] ext4: add some kunit stub for mballoc kunit test Multiblocks allocation will read and write block bitmap and group descriptor which reside on disk. Add kunit stub to function ext4_get_group_desc, ext4_read_block_bitmap_nowait, ext4_wait_block_bitmap and ext4_mb_mark_context to avoid real IO to disk. Signed-off-by: Kemeng Shi Reviewed-by: "Ritesh Harjani (IBM)" Link: https://lore.kernel.org/r/20230928160407.142069-11-shikemeng@huaweicloud.com Signed-off-by: Theodore Ts'o --- fs/ext4/balloc.c | 10 ++++++++++ fs/ext4/mballoc.c | 5 +++++ 2 files changed, 15 insertions(+) diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c index 4d08bb2bd184..591fb3f710be 100644 --- a/fs/ext4/balloc.c +++ b/fs/ext4/balloc.c @@ -22,6 +22,7 @@ #include "mballoc.h" #include +#include static unsigned ext4_num_base_meta_clusters(struct super_block *sb, ext4_group_t block_group); @@ -272,6 +273,9 @@ struct ext4_group_desc * ext4_get_group_desc(struct super_block *sb, struct ext4_sb_info *sbi = EXT4_SB(sb); struct buffer_head *bh_p; + KUNIT_STATIC_STUB_REDIRECT(ext4_get_group_desc, + sb, block_group, bh); + if (block_group >= ngroups) { ext4_error(sb, "block_group >= groups_count - block_group = %u," " groups_count = %u", block_group, ngroups); @@ -466,6 +470,9 @@ ext4_read_block_bitmap_nowait(struct super_block *sb, ext4_group_t block_group, ext4_fsblk_t bitmap_blk; int err; + KUNIT_STATIC_STUB_REDIRECT(ext4_read_block_bitmap_nowait, + sb, block_group, ignore_locked); + desc = ext4_get_group_desc(sb, block_group, NULL); if (!desc) return ERR_PTR(-EFSCORRUPTED); @@ -561,6 +568,9 @@ int ext4_wait_block_bitmap(struct super_block *sb, ext4_group_t block_group, { struct ext4_group_desc *desc; + KUNIT_STATIC_STUB_REDIRECT(ext4_wait_block_bitmap, + sb, block_group, bh); + if (!buffer_new(bh)) return 0; desc = ext4_get_group_desc(sb, block_group, NULL); diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c index b26e4d64cd4c..401b8329d1e1 100644 --- a/fs/ext4/mballoc.c +++ b/fs/ext4/mballoc.c @@ -18,6 +18,7 @@ #include #include #include +#include /* * MUSTDO: @@ -3957,6 +3958,10 @@ ext4_mb_mark_context(handle_t *handle, struct super_block *sb, bool state, int err; unsigned int i, already, changed = len; + KUNIT_STATIC_STUB_REDIRECT(ext4_mb_mark_context, + handle, sb, state, group, blkoff, len, + flags, ret_changed); + if (ret_changed) *ret_changed = 0; bitmap_bh = ext4_read_block_bitmap(sb, group); From 7c9fa399a369546c0e0375ea4b354d642a8fe501 Mon Sep 17 00:00:00 2001 From: Kemeng Shi Date: Fri, 29 Sep 2023 00:04:06 +0800 Subject: [PATCH 42/69] ext4: add first unit test for ext4_mb_new_blocks_simple in mballoc Here are prepared work: 1. Include mballoc-test.c to mballoc.c to be able test static function in mballoc.c. 2. Implement static stub to avoid read IO to disk. 3. Construct fake super_block. Only partial members are set, more members will be set when more functions are tested. Then unit test for ext4_mb_new_blocks_simple is added. Signed-off-by: Kemeng Shi Reviewed-by: "Ritesh Harjani (IBM)" Link: https://lore.kernel.org/r/20230928160407.142069-12-shikemeng@huaweicloud.com Signed-off-by: Theodore Ts'o --- fs/ext4/mballoc-test.c | 325 +++++++++++++++++++++++++++++++++++++++++ fs/ext4/mballoc.c | 4 + 2 files changed, 329 insertions(+) create mode 100644 fs/ext4/mballoc-test.c diff --git a/fs/ext4/mballoc-test.c b/fs/ext4/mballoc-test.c new file mode 100644 index 000000000000..120c4944d2e1 --- /dev/null +++ b/fs/ext4/mballoc-test.c @@ -0,0 +1,325 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * KUnit test of ext4 multiblocks allocation. + */ + +#include +#include + +#include "ext4.h" + +struct mbt_grp_ctx { + struct buffer_head bitmap_bh; + /* desc and gd_bh are just the place holders for now */ + struct ext4_group_desc desc; + struct buffer_head gd_bh; +}; + +struct mbt_ctx { + struct mbt_grp_ctx *grp_ctx; +}; + +struct mbt_ext4_super_block { + struct super_block sb; + struct mbt_ctx mbt_ctx; +}; + +#define MBT_CTX(_sb) (&(container_of((_sb), struct mbt_ext4_super_block, sb)->mbt_ctx)) +#define MBT_GRP_CTX(_sb, _group) (&MBT_CTX(_sb)->grp_ctx[_group]) + +static struct super_block *mbt_ext4_alloc_super_block(void) +{ + struct ext4_super_block *es = kzalloc(sizeof(*es), GFP_KERNEL); + struct ext4_sb_info *sbi = kzalloc(sizeof(*sbi), GFP_KERNEL); + struct mbt_ext4_super_block *fsb = kzalloc(sizeof(*fsb), GFP_KERNEL); + + if (fsb == NULL || sbi == NULL || es == NULL) + goto out; + + sbi->s_es = es; + fsb->sb.s_fs_info = sbi; + return &fsb->sb; + +out: + kfree(fsb); + kfree(sbi); + kfree(es); + return NULL; +} + +static void mbt_ext4_free_super_block(struct super_block *sb) +{ + struct mbt_ext4_super_block *fsb = + container_of(sb, struct mbt_ext4_super_block, sb); + struct ext4_sb_info *sbi = EXT4_SB(sb); + + kfree(sbi->s_es); + kfree(sbi); + kfree(fsb); +} + +struct mbt_ext4_block_layout { + unsigned char blocksize_bits; + unsigned int cluster_bits; + uint32_t blocks_per_group; + ext4_group_t group_count; + uint16_t desc_size; +}; + +static void mbt_init_sb_layout(struct super_block *sb, + struct mbt_ext4_block_layout *layout) +{ + struct ext4_sb_info *sbi = EXT4_SB(sb); + struct ext4_super_block *es = sbi->s_es; + + sb->s_blocksize = 1UL << layout->blocksize_bits; + sb->s_blocksize_bits = layout->blocksize_bits; + + sbi->s_groups_count = layout->group_count; + sbi->s_blocks_per_group = layout->blocks_per_group; + sbi->s_cluster_bits = layout->cluster_bits; + sbi->s_cluster_ratio = 1U << layout->cluster_bits; + sbi->s_clusters_per_group = layout->blocks_per_group >> + layout->cluster_bits; + sbi->s_desc_size = layout->desc_size; + + es->s_first_data_block = cpu_to_le32(0); + es->s_blocks_count_lo = cpu_to_le32(layout->blocks_per_group * + layout->group_count); +} + +static int mbt_grp_ctx_init(struct super_block *sb, + struct mbt_grp_ctx *grp_ctx) +{ + grp_ctx->bitmap_bh.b_data = kzalloc(EXT4_BLOCK_SIZE(sb), GFP_KERNEL); + if (grp_ctx->bitmap_bh.b_data == NULL) + return -ENOMEM; + + return 0; +} + +static void mbt_grp_ctx_release(struct mbt_grp_ctx *grp_ctx) +{ + kfree(grp_ctx->bitmap_bh.b_data); + grp_ctx->bitmap_bh.b_data = NULL; +} + +static void mbt_ctx_mark_used(struct super_block *sb, ext4_group_t group, + unsigned int start, unsigned int len) +{ + struct mbt_grp_ctx *grp_ctx = MBT_GRP_CTX(sb, group); + + mb_set_bits(grp_ctx->bitmap_bh.b_data, start, len); +} + +/* called after mbt_init_sb_layout */ +static int mbt_ctx_init(struct super_block *sb) +{ + struct mbt_ctx *ctx = MBT_CTX(sb); + ext4_group_t i, ngroups = ext4_get_groups_count(sb); + + ctx->grp_ctx = kcalloc(ngroups, sizeof(struct mbt_grp_ctx), + GFP_KERNEL); + if (ctx->grp_ctx == NULL) + return -ENOMEM; + + for (i = 0; i < ngroups; i++) + if (mbt_grp_ctx_init(sb, &ctx->grp_ctx[i])) + goto out; + + /* + * first data block(first cluster in first group) is used by + * metadata, mark it used to avoid to alloc data block at first + * block which will fail ext4_sb_block_valid check. + */ + mb_set_bits(ctx->grp_ctx[0].bitmap_bh.b_data, 0, 1); + + return 0; +out: + while (i-- > 0) + mbt_grp_ctx_release(&ctx->grp_ctx[i]); + kfree(ctx->grp_ctx); + return -ENOMEM; +} + +static void mbt_ctx_release(struct super_block *sb) +{ + struct mbt_ctx *ctx = MBT_CTX(sb); + ext4_group_t i, ngroups = ext4_get_groups_count(sb); + + for (i = 0; i < ngroups; i++) + mbt_grp_ctx_release(&ctx->grp_ctx[i]); + kfree(ctx->grp_ctx); +} + +static struct buffer_head * +ext4_read_block_bitmap_nowait_stub(struct super_block *sb, ext4_group_t block_group, + bool ignore_locked) +{ + struct mbt_grp_ctx *grp_ctx = MBT_GRP_CTX(sb, block_group); + + /* paired with brelse from caller of ext4_read_block_bitmap_nowait */ + get_bh(&grp_ctx->bitmap_bh); + return &grp_ctx->bitmap_bh; +} + +static int ext4_wait_block_bitmap_stub(struct super_block *sb, + ext4_group_t block_group, + struct buffer_head *bh) +{ + return 0; +} + +static struct ext4_group_desc * +ext4_get_group_desc_stub(struct super_block *sb, ext4_group_t block_group, + struct buffer_head **bh) +{ + struct mbt_grp_ctx *grp_ctx = MBT_GRP_CTX(sb, block_group); + + if (bh != NULL) + *bh = &grp_ctx->gd_bh; + + return &grp_ctx->desc; +} + +static int +ext4_mb_mark_context_stub(handle_t *handle, struct super_block *sb, bool state, + ext4_group_t group, ext4_grpblk_t blkoff, + ext4_grpblk_t len, int flags, + ext4_grpblk_t *ret_changed) +{ + struct mbt_grp_ctx *grp_ctx = MBT_GRP_CTX(sb, group); + struct buffer_head *bitmap_bh = &grp_ctx->bitmap_bh; + + if (state) + mb_set_bits(bitmap_bh->b_data, blkoff, len); + else + mb_clear_bits(bitmap_bh->b_data, blkoff, len); + + return 0; +} + +#define TEST_BLOCKSIZE_BITS 10 +#define TEST_CLUSTER_BITS 3 +#define TEST_BLOCKS_PER_GROUP 8192 +#define TEST_GROUP_COUNT 4 +#define TEST_DESC_SIZE 64 +#define TEST_GOAL_GROUP 1 +static int mbt_kunit_init(struct kunit *test) +{ + struct mbt_ext4_block_layout layout = { + .blocksize_bits = TEST_BLOCKSIZE_BITS, + .cluster_bits = TEST_CLUSTER_BITS, + .blocks_per_group = TEST_BLOCKS_PER_GROUP, + .group_count = TEST_GROUP_COUNT, + .desc_size = TEST_DESC_SIZE, + }; + struct super_block *sb; + int ret; + + sb = mbt_ext4_alloc_super_block(); + if (sb == NULL) + return -ENOMEM; + + mbt_init_sb_layout(sb, &layout); + + ret = mbt_ctx_init(sb); + if (ret != 0) { + mbt_ext4_free_super_block(sb); + return ret; + } + + test->priv = sb; + kunit_activate_static_stub(test, + ext4_read_block_bitmap_nowait, + ext4_read_block_bitmap_nowait_stub); + kunit_activate_static_stub(test, + ext4_wait_block_bitmap, + ext4_wait_block_bitmap_stub); + kunit_activate_static_stub(test, + ext4_get_group_desc, + ext4_get_group_desc_stub); + kunit_activate_static_stub(test, + ext4_mb_mark_context, + ext4_mb_mark_context_stub); + return 0; +} + +static void mbt_kunit_exit(struct kunit *test) +{ + struct super_block *sb = (struct super_block *)test->priv; + + mbt_ctx_release(sb); + mbt_ext4_free_super_block(sb); +} + +static void test_new_blocks_simple(struct kunit *test) +{ + struct super_block *sb = (struct super_block *)test->priv; + struct inode inode = { .i_sb = sb, }; + struct ext4_allocation_request ar; + ext4_group_t i, goal_group = TEST_GOAL_GROUP; + int err = 0; + ext4_fsblk_t found; + struct ext4_sb_info *sbi = EXT4_SB(sb); + + ar.inode = &inode; + + /* get block at goal */ + ar.goal = ext4_group_first_block_no(sb, goal_group); + found = ext4_mb_new_blocks_simple(&ar, &err); + KUNIT_ASSERT_EQ_MSG(test, ar.goal, found, + "failed to alloc block at goal, expected %llu found %llu", + ar.goal, found); + + /* get block after goal in goal group */ + ar.goal = ext4_group_first_block_no(sb, goal_group); + found = ext4_mb_new_blocks_simple(&ar, &err); + KUNIT_ASSERT_EQ_MSG(test, ar.goal + EXT4_C2B(sbi, 1), found, + "failed to alloc block after goal in goal group, expected %llu found %llu", + ar.goal + 1, found); + + /* get block after goal group */ + mbt_ctx_mark_used(sb, goal_group, 0, EXT4_CLUSTERS_PER_GROUP(sb)); + ar.goal = ext4_group_first_block_no(sb, goal_group); + found = ext4_mb_new_blocks_simple(&ar, &err); + KUNIT_ASSERT_EQ_MSG(test, + ext4_group_first_block_no(sb, goal_group + 1), found, + "failed to alloc block after goal group, expected %llu found %llu", + ext4_group_first_block_no(sb, goal_group + 1), found); + + /* get block before goal group */ + for (i = goal_group; i < ext4_get_groups_count(sb); i++) + mbt_ctx_mark_used(sb, i, 0, EXT4_CLUSTERS_PER_GROUP(sb)); + ar.goal = ext4_group_first_block_no(sb, goal_group); + found = ext4_mb_new_blocks_simple(&ar, &err); + KUNIT_ASSERT_EQ_MSG(test, + ext4_group_first_block_no(sb, 0) + EXT4_C2B(sbi, 1), found, + "failed to alloc block before goal group, expected %llu found %llu", + ext4_group_first_block_no(sb, 0 + EXT4_C2B(sbi, 1)), found); + + /* no block available, fail to allocate block */ + for (i = 0; i < ext4_get_groups_count(sb); i++) + mbt_ctx_mark_used(sb, i, 0, EXT4_CLUSTERS_PER_GROUP(sb)); + ar.goal = ext4_group_first_block_no(sb, goal_group); + found = ext4_mb_new_blocks_simple(&ar, &err); + KUNIT_ASSERT_NE_MSG(test, err, 0, + "unexpectedly get block when no block is available"); +} + + +static struct kunit_case mbt_test_cases[] = { + KUNIT_CASE(test_new_blocks_simple), + {} +}; + +static struct kunit_suite mbt_test_suite = { + .name = "ext4_mballoc_test", + .init = mbt_kunit_init, + .exit = mbt_kunit_exit, + .test_cases = mbt_test_cases, +}; + +kunit_test_suites(&mbt_test_suite); + +MODULE_LICENSE("GPL"); diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c index 401b8329d1e1..454d5612641e 100644 --- a/fs/ext4/mballoc.c +++ b/fs/ext4/mballoc.c @@ -6991,3 +6991,7 @@ out_unload: return error; } + +#ifdef CONFIG_EXT4_KUNIT_TESTS +#include "mballoc-test.c" +#endif From 28b95ee868072f1cd029245bf96cf02844322f37 Mon Sep 17 00:00:00 2001 From: Kemeng Shi Date: Fri, 29 Sep 2023 00:04:07 +0800 Subject: [PATCH 43/69] ext4: run mballoc test with different layouts setting Use KUNIT_CASE_PARAM to run mballoc test with different layouts setting. Signed-off-by: Kemeng Shi Reviewed-by: "Ritesh Harjani (IBM)" Link: https://lore.kernel.org/r/20230928160407.142069-13-shikemeng@huaweicloud.com Signed-off-by: Theodore Ts'o --- fs/ext4/mballoc-test.c | 52 ++++++++++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/fs/ext4/mballoc-test.c b/fs/ext4/mballoc-test.c index 120c4944d2e1..f94901fd3835 100644 --- a/fs/ext4/mballoc-test.c +++ b/fs/ext4/mballoc-test.c @@ -199,21 +199,11 @@ ext4_mb_mark_context_stub(handle_t *handle, struct super_block *sb, bool state, return 0; } -#define TEST_BLOCKSIZE_BITS 10 -#define TEST_CLUSTER_BITS 3 -#define TEST_BLOCKS_PER_GROUP 8192 -#define TEST_GROUP_COUNT 4 -#define TEST_DESC_SIZE 64 #define TEST_GOAL_GROUP 1 static int mbt_kunit_init(struct kunit *test) { - struct mbt_ext4_block_layout layout = { - .blocksize_bits = TEST_BLOCKSIZE_BITS, - .cluster_bits = TEST_CLUSTER_BITS, - .blocks_per_group = TEST_BLOCKS_PER_GROUP, - .group_count = TEST_GROUP_COUNT, - .desc_size = TEST_DESC_SIZE, - }; + struct mbt_ext4_block_layout *layout = + (struct mbt_ext4_block_layout *)(test->param_value); struct super_block *sb; int ret; @@ -221,7 +211,7 @@ static int mbt_kunit_init(struct kunit *test) if (sb == NULL) return -ENOMEM; - mbt_init_sb_layout(sb, &layout); + mbt_init_sb_layout(sb, layout); ret = mbt_ctx_init(sb); if (ret != 0) { @@ -307,9 +297,43 @@ static void test_new_blocks_simple(struct kunit *test) "unexpectedly get block when no block is available"); } +static const struct mbt_ext4_block_layout mbt_test_layouts[] = { + { + .blocksize_bits = 10, + .cluster_bits = 3, + .blocks_per_group = 8192, + .group_count = 4, + .desc_size = 64, + }, + { + .blocksize_bits = 12, + .cluster_bits = 3, + .blocks_per_group = 8192, + .group_count = 4, + .desc_size = 64, + }, + { + .blocksize_bits = 16, + .cluster_bits = 3, + .blocks_per_group = 8192, + .group_count = 4, + .desc_size = 64, + }, +}; + +static void mbt_show_layout(const struct mbt_ext4_block_layout *layout, + char *desc) +{ + snprintf(desc, KUNIT_PARAM_DESC_SIZE, "block_bits=%d cluster_bits=%d " + "blocks_per_group=%d group_count=%d desc_size=%d\n", + layout->blocksize_bits, layout->cluster_bits, + layout->blocks_per_group, layout->group_count, + layout->desc_size); +} +KUNIT_ARRAY_PARAM(mbt_layouts, mbt_test_layouts, mbt_show_layout); static struct kunit_case mbt_test_cases[] = { - KUNIT_CASE(test_new_blocks_simple), + KUNIT_CASE_PARAM(test_new_blocks_simple, mbt_layouts_gen_params), {} }; From 78a50b6a41665efeabeec5edbae245d8be93278c Mon Sep 17 00:00:00 2001 From: Tiwei Bie Date: Tue, 10 Oct 2023 19:39:15 +0800 Subject: [PATCH 44/69] erofs: fix inode metadata space layout description in documentation Xattrs, extents, data inline are _placed after_, not _followed by_ the corresponding inode. This patch fixes it. Signed-off-by: Tiwei Bie Reviewed-by: Gao Xiang Reviewed-by: Jingbo Xu Reviewed-by: Chao Yu Link: https://lore.kernel.org/r/20231010113915.436591-1-tiwei.btw@antgroup.com Signed-off-by: Gao Xiang --- Documentation/filesystems/erofs.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/filesystems/erofs.rst b/Documentation/filesystems/erofs.rst index f200d7874495..57c6ae23b3fc 100644 --- a/Documentation/filesystems/erofs.rst +++ b/Documentation/filesystems/erofs.rst @@ -199,7 +199,7 @@ may not. All metadatas can be now observed in two different spaces (views): | | |__________________| 64 bytes - Xattrs, extents, data inline are followed by the corresponding inode with + Xattrs, extents, data inline are placed after the corresponding inode with proper alignment, and they could be optional for different data mappings. _currently_ total 5 data layouts are supported: From 19b3102c0b5350621e7492281f2be0f071fb7e31 Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Tue, 10 Oct 2023 18:04:41 -0400 Subject: [PATCH 45/69] fs: dlm: Simplify buffer size computation in dlm_create_debug_file() Use sizeof(name) instead of the equivalent, but hard coded, DLM_LOCKSPACE_LEN + 8. This is less verbose and more future proof. Signed-off-by: Christophe JAILLET Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/debug_fs.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/fs/dlm/debug_fs.c b/fs/dlm/debug_fs.c index 5aabcb6f0f15..e9726c6cbdf2 100644 --- a/fs/dlm/debug_fs.c +++ b/fs/dlm/debug_fs.c @@ -986,7 +986,7 @@ void dlm_create_debug_file(struct dlm_ls *ls) /* format 2 */ memset(name, 0, sizeof(name)); - snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_locks", ls->ls_name); + snprintf(name, sizeof(name), "%s_locks", ls->ls_name); ls->ls_debug_locks_dentry = debugfs_create_file(name, 0644, @@ -997,7 +997,7 @@ void dlm_create_debug_file(struct dlm_ls *ls) /* format 3 */ memset(name, 0, sizeof(name)); - snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_all", ls->ls_name); + snprintf(name, sizeof(name), "%s_all", ls->ls_name); ls->ls_debug_all_dentry = debugfs_create_file(name, S_IFREG | S_IRUGO, @@ -1008,7 +1008,7 @@ void dlm_create_debug_file(struct dlm_ls *ls) /* format 4 */ memset(name, 0, sizeof(name)); - snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_toss", ls->ls_name); + snprintf(name, sizeof(name), "%s_toss", ls->ls_name); ls->ls_debug_toss_dentry = debugfs_create_file(name, S_IFREG | S_IRUGO, @@ -1017,7 +1017,7 @@ void dlm_create_debug_file(struct dlm_ls *ls) &format4_fops); memset(name, 0, sizeof(name)); - snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_waiters", ls->ls_name); + snprintf(name, sizeof(name), "%s_waiters", ls->ls_name); ls->ls_debug_waiters_dentry = debugfs_create_file(name, 0644, @@ -1028,7 +1028,7 @@ void dlm_create_debug_file(struct dlm_ls *ls) /* format 5 */ memset(name, 0, sizeof(name)); - snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_queued_asts", ls->ls_name); + snprintf(name, sizeof(name), "%s_queued_asts", ls->ls_name); ls->ls_debug_queued_asts_dentry = debugfs_create_file(name, 0644, From b859e01054354033f480d9df41b0ebc2c7537379 Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Tue, 10 Oct 2023 18:04:42 -0400 Subject: [PATCH 46/69] fs: dlm: Fix the size of a buffer in dlm_create_debug_file() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 8 is not the maximum size of the suffix used when creating debugfs files. Let the compiler compute the correct size, and only give a hint about the longest possible string that is used. When building with W=1, this fixes the following warnings: fs/dlm/debug_fs.c: In function ‘dlm_create_debug_file’: fs/dlm/debug_fs.c:1020:58: error: ‘snprintf’ output may be truncated before the last format character [-Werror=format-truncation=] 1020 | snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_waiters", ls->ls_name); | ^ fs/dlm/debug_fs.c:1020:9: note: ‘snprintf’ output between 9 and 73 bytes into a destination of size 72 1020 | snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_waiters", ls->ls_name); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ fs/dlm/debug_fs.c:1031:50: error: ‘_queued_asts’ directive output may be truncated writing 12 bytes into a region of size between 8 and 72 [-Werror=format-truncation=] 1031 | snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_queued_asts", ls->ls_name); | ^~~~~~~~~~~~ fs/dlm/debug_fs.c:1031:9: note: ‘snprintf’ output between 13 and 77 bytes into a destination of size 72 1031 | snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_queued_asts", ls->ls_name); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Fixes: 541adb0d4d10b ("fs: dlm: debugfs for queued callbacks") Signed-off-by: Christophe JAILLET Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/debug_fs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/dlm/debug_fs.c b/fs/dlm/debug_fs.c index e9726c6cbdf2..c93359ceaae6 100644 --- a/fs/dlm/debug_fs.c +++ b/fs/dlm/debug_fs.c @@ -973,7 +973,8 @@ void dlm_delete_debug_comms_file(void *ctx) void dlm_create_debug_file(struct dlm_ls *ls) { - char name[DLM_LOCKSPACE_LEN + 8]; + /* Reserve enough space for the longest file name */ + char name[DLM_LOCKSPACE_LEN + sizeof("_queued_asts")]; /* format 1 */ From bc15bec1f8833c2cc166c7713eb89a84e2ecf8a2 Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Tue, 10 Oct 2023 18:04:43 -0400 Subject: [PATCH 47/69] fs: dlm: Remove some useless memset() There is no need to clear the buffer used to build the file name. snprintf() already guarantees that it is NULL terminated and such a (useless) precaution was not done for the first string (i.e ls_debug_rsb_dentry) So, save a few LoC. Signed-off-by: Christophe JAILLET Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/debug_fs.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/fs/dlm/debug_fs.c b/fs/dlm/debug_fs.c index c93359ceaae6..42f332f46359 100644 --- a/fs/dlm/debug_fs.c +++ b/fs/dlm/debug_fs.c @@ -986,7 +986,6 @@ void dlm_create_debug_file(struct dlm_ls *ls) /* format 2 */ - memset(name, 0, sizeof(name)); snprintf(name, sizeof(name), "%s_locks", ls->ls_name); ls->ls_debug_locks_dentry = debugfs_create_file(name, @@ -997,7 +996,6 @@ void dlm_create_debug_file(struct dlm_ls *ls) /* format 3 */ - memset(name, 0, sizeof(name)); snprintf(name, sizeof(name), "%s_all", ls->ls_name); ls->ls_debug_all_dentry = debugfs_create_file(name, @@ -1008,7 +1006,6 @@ void dlm_create_debug_file(struct dlm_ls *ls) /* format 4 */ - memset(name, 0, sizeof(name)); snprintf(name, sizeof(name), "%s_toss", ls->ls_name); ls->ls_debug_toss_dentry = debugfs_create_file(name, @@ -1017,7 +1014,6 @@ void dlm_create_debug_file(struct dlm_ls *ls) ls, &format4_fops); - memset(name, 0, sizeof(name)); snprintf(name, sizeof(name), "%s_waiters", ls->ls_name); ls->ls_debug_waiters_dentry = debugfs_create_file(name, @@ -1028,7 +1024,6 @@ void dlm_create_debug_file(struct dlm_ls *ls) /* format 5 */ - memset(name, 0, sizeof(name)); snprintf(name, sizeof(name), "%s_queued_asts", ls->ls_name); ls->ls_debug_queued_asts_dentry = debugfs_create_file(name, From fe9b619e6e94acf0b068fb1a8f658f5a96b8fad7 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Tue, 10 Oct 2023 18:04:44 -0400 Subject: [PATCH 48/69] dlm: fix creating multiple node structures This patch will lookup existing nodes instead of always creating them when dlm_midcomms_addr() is called. The idea is here to create midcomms nodes when user space getting informed that nodes joins the cluster. This is the case when dlm_midcomms_addr() is called, however it can be called multiple times by user space to add several address configurations to one node e.g. when using SCTP. Those multiple times need to be filtered out and we doing that by looking up if the node exists before. Due configfs entry it is safe that this function gets only called once at a time. Fixes: 63e711b08160 ("fs: dlm: create midcomms nodes when configure") Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/midcomms.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c index f641b36a36db..455265c6ba53 100644 --- a/fs/dlm/midcomms.c +++ b/fs/dlm/midcomms.c @@ -337,13 +337,21 @@ static struct midcomms_node *nodeid2node(int nodeid) int dlm_midcomms_addr(int nodeid, struct sockaddr_storage *addr, int len) { - int ret, r = nodeid_hash(nodeid); + int ret, idx, r = nodeid_hash(nodeid); struct midcomms_node *node; ret = dlm_lowcomms_addr(nodeid, addr, len); if (ret) return ret; + idx = srcu_read_lock(&nodes_srcu); + node = __find_node(nodeid, r); + if (node) { + srcu_read_unlock(&nodes_srcu, idx); + return 0; + } + srcu_read_unlock(&nodes_srcu, idx); + node = kmalloc(sizeof(*node), GFP_NOFS); if (!node) return -ENOMEM; From 2776635edc7fcd62e03cb2efb93c31f685887460 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Tue, 10 Oct 2023 18:04:45 -0400 Subject: [PATCH 49/69] dlm: fix remove member after close call The idea of commit 63e711b08160 ("fs: dlm: create midcomms nodes when configure") is to set the midcomms node lifetime when a node joins or leaves the cluster. Currently we can hit the following warning: [10844.611495] ------------[ cut here ]------------ [10844.615913] WARNING: CPU: 4 PID: 84304 at fs/dlm/midcomms.c:1263 dlm_midcomms_remove_member+0x13f/0x180 [dlm] or running in a state where we hit a midcomms node usage count in a negative value: [ 260.830782] node 2 users dec count -1 The first warning happens when the a specific node does not exists and it was probably removed but dlm_midcomms_close() which is called when a node leaves the cluster. The second kernel log message is probably in a case when dlm_midcomms_addr() is called when a joined the cluster but due fencing a node leaved the cluster without getting removed from the lockspace. If the node joins the cluster and it was removed from the cluster due fencing the first call is to remove the node from lockspaces triggered by the user space. In both cases if the node wasn't found or the user count is zero, we should ignore any additional midcomms handling of dlm_midcomms_remove_member(). Fixes: 63e711b08160 ("fs: dlm: create midcomms nodes when configure") Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/midcomms.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c index 455265c6ba53..4ad71e97cec2 100644 --- a/fs/dlm/midcomms.c +++ b/fs/dlm/midcomms.c @@ -1268,12 +1268,23 @@ void dlm_midcomms_remove_member(int nodeid) idx = srcu_read_lock(&nodes_srcu); node = nodeid2node(nodeid); - if (WARN_ON_ONCE(!node)) { + /* in case of dlm_midcomms_close() removes node */ + if (!node) { srcu_read_unlock(&nodes_srcu, idx); return; } spin_lock(&node->state_lock); + /* case of dlm_midcomms_addr() created node but + * was not added before because dlm_midcomms_close() + * removed the node + */ + if (!node->users) { + spin_unlock(&node->state_lock); + srcu_read_unlock(&nodes_srcu, idx); + return; + } + node->users--; pr_debug("node %d users dec count %d\n", nodeid, node->users); From e759eb3e27e5b624930548f1c0eda90da6e26ee9 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Tue, 10 Oct 2023 18:04:46 -0400 Subject: [PATCH 50/69] dlm: be sure we reset all nodes at forced shutdown In case we running in a force shutdown in either midcomms or lowcomms implementation we will make sure we reset all per midcomms node information. Fixes: 63e711b08160 ("fs: dlm: create midcomms nodes when configure") Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/midcomms.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c index 4ad71e97cec2..6bc8d7f89b2c 100644 --- a/fs/dlm/midcomms.c +++ b/fs/dlm/midcomms.c @@ -1405,10 +1405,16 @@ void dlm_midcomms_shutdown(void) midcomms_shutdown(node); } } - srcu_read_unlock(&nodes_srcu, idx); - mutex_unlock(&close_lock); dlm_lowcomms_shutdown(); + + for (i = 0; i < CONN_HASH_SIZE; i++) { + hlist_for_each_entry_rcu(node, &node_hash[i], hlist) { + midcomms_node_reset(node); + } + } + srcu_read_unlock(&nodes_srcu, idx); + mutex_unlock(&close_lock); } int dlm_midcomms_close(int nodeid) From 6212e4528b248a4bc9b4fe68e029a84689c67461 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Tue, 10 Oct 2023 18:04:47 -0400 Subject: [PATCH 51/69] dlm: fix no ack after final message In case of an final DLM message we can't should not send an ack out after the final message. This patch moves the ack message before the messages will be transmitted. If it's the final message and the receiving node turns into DLM_CLOSED state another ack messages will being received and turning the receiving node into DLM_ESTABLISHED again. Fixes: 1696c75f1864 ("fs: dlm: add send ack threshold and append acks to msgs") Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/midcomms.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c index 6bc8d7f89b2c..2247ebb61be1 100644 --- a/fs/dlm/midcomms.c +++ b/fs/dlm/midcomms.c @@ -1038,15 +1038,15 @@ struct dlm_mhandle *dlm_midcomms_get_mhandle(int nodeid, int len, break; case DLM_VERSION_3_2: + /* send ack back if necessary */ + dlm_send_ack_threshold(node, DLM_SEND_ACK_BACK_MSG_THRESHOLD); + msg = dlm_midcomms_get_msg_3_2(mh, nodeid, len, allocation, ppc); if (!msg) { dlm_free_mhandle(mh); goto err; } - - /* send ack back if necessary */ - dlm_send_ack_threshold(node, DLM_SEND_ACK_BACK_MSG_THRESHOLD); break; default: dlm_free_mhandle(mh); From a470cb2a06e4cc6b8a5011b0c8da18705429120b Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Tue, 10 Oct 2023 18:04:48 -0400 Subject: [PATCH 52/69] dlm: slow down filling up processing queue If there is a burst of message the receive worker will filling up the processing queue but where are too slow to process dlm messages. This patch will slow down the receiver worker to keep the buffer on the socket layer to tell the sender to backoff. This is done by a threshold to get the next buffers from the socket after all messages were processed done by a flush_workqueue(). This however only occurs when we have a message burst when we e.g. create 1 million locks. If we put more and more new messages to process in the processqueue we will soon run out of memory. Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/lowcomms.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c index f7bc22e74db2..67f8dd8a05ef 100644 --- a/fs/dlm/lowcomms.c +++ b/fs/dlm/lowcomms.c @@ -63,6 +63,7 @@ #include "config.h" #define DLM_SHUTDOWN_WAIT_TIMEOUT msecs_to_jiffies(5000) +#define DLM_MAX_PROCESS_BUFFERS 24 #define NEEDED_RMEM (4*1024*1024) struct connection { @@ -194,6 +195,7 @@ static const struct dlm_proto_ops *dlm_proto_ops; #define DLM_IO_END 1 #define DLM_IO_EOF 2 #define DLM_IO_RESCHED 3 +#define DLM_IO_FLUSH 4 static void process_recv_sockets(struct work_struct *work); static void process_send_sockets(struct work_struct *work); @@ -202,6 +204,7 @@ static void process_dlm_messages(struct work_struct *work); static DECLARE_WORK(process_work, process_dlm_messages); static DEFINE_SPINLOCK(processqueue_lock); static bool process_dlm_messages_pending; +static atomic_t processqueue_count; static LIST_HEAD(processqueue); bool dlm_lowcomms_is_running(void) @@ -874,6 +877,7 @@ static void process_dlm_messages(struct work_struct *work) } list_del(&pentry->list); + atomic_dec(&processqueue_count); spin_unlock(&processqueue_lock); for (;;) { @@ -891,6 +895,7 @@ static void process_dlm_messages(struct work_struct *work) } list_del(&pentry->list); + atomic_dec(&processqueue_count); spin_unlock(&processqueue_lock); } } @@ -962,6 +967,7 @@ again: con->rx_leftover); spin_lock(&processqueue_lock); + ret = atomic_inc_return(&processqueue_count); list_add_tail(&pentry->list, &processqueue); if (!process_dlm_messages_pending) { process_dlm_messages_pending = true; @@ -969,6 +975,9 @@ again: } spin_unlock(&processqueue_lock); + if (ret > DLM_MAX_PROCESS_BUFFERS) + return DLM_IO_FLUSH; + return DLM_IO_SUCCESS; } @@ -1503,6 +1512,9 @@ static void process_recv_sockets(struct work_struct *work) wake_up(&con->shutdown_wait); /* CF_RECV_PENDING cleared */ break; + case DLM_IO_FLUSH: + flush_workqueue(process_workqueue); + fallthrough; case DLM_IO_RESCHED: cond_resched(); queue_work(io_workqueue, &con->rwork); From eb53c01873cab75d0bf42e6d685dc5ef3b68d884 Mon Sep 17 00:00:00 2001 From: David Teigland Date: Thu, 12 Oct 2023 15:25:37 -0500 Subject: [PATCH 53/69] MAINTAINERS: Update dlm maintainer and web page Signed-off-by: David Teigland --- MAINTAINERS | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 6c4cce45a09d..15de36f9a376 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6114,11 +6114,11 @@ F: drivers/video/fbdev/udlfb.c F: include/video/udlfb.h DISTRIBUTED LOCK MANAGER (DLM) -M: Christine Caulfield +M: Alexander Aring M: David Teigland L: gfs2@lists.linux.dev S: Supported -W: http://sources.redhat.com/cluster/ +W: https://pagure.io/dlm T: git git://git.kernel.org/pub/scm/linux/kernel/git/teigland/linux-dlm.git F: fs/dlm/ From a779ed754e52d582b8c0e17959df063108bd0656 Mon Sep 17 00:00:00 2001 From: Dave Kleikamp Date: Thu, 5 Oct 2023 09:16:14 -0500 Subject: [PATCH 54/69] jfs: define xtree root and page independently In order to make array bounds checking sane, provide a separate definition of the in-inode xtree root and the external xtree page. Signed-off-by: Dave Kleikamp Tested-by: Manas Ghandat --- fs/jfs/jfs_dinode.h | 2 +- fs/jfs/jfs_imap.c | 6 +++--- fs/jfs/jfs_incore.h | 2 +- fs/jfs/jfs_txnmgr.c | 4 ++-- fs/jfs/jfs_xtree.c | 4 ++-- fs/jfs/jfs_xtree.h | 37 +++++++++++++++++++++++-------------- 6 files changed, 32 insertions(+), 23 deletions(-) diff --git a/fs/jfs/jfs_dinode.h b/fs/jfs/jfs_dinode.h index 6b231d0d0071..603aae17a693 100644 --- a/fs/jfs/jfs_dinode.h +++ b/fs/jfs/jfs_dinode.h @@ -96,7 +96,7 @@ struct dinode { #define di_gengen u._file._u1._imap._gengen union { - xtpage_t _xtroot; + xtroot_t _xtroot; struct { u8 unused[16]; /* 16: */ dxd_t _dxd; /* 16: */ diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c index 1b267eec3f36..394e0af0e5df 100644 --- a/fs/jfs/jfs_imap.c +++ b/fs/jfs/jfs_imap.c @@ -670,7 +670,7 @@ int diWrite(tid_t tid, struct inode *ip) * This is the special xtree inside the directory for storing * the directory table */ - xtpage_t *p, *xp; + xtroot_t *p, *xp; xad_t *xad; jfs_ip->xtlid = 0; @@ -684,7 +684,7 @@ int diWrite(tid_t tid, struct inode *ip) * copy xtree root from inode to dinode: */ p = &jfs_ip->i_xtroot; - xp = (xtpage_t *) &dp->di_dirtable; + xp = (xtroot_t *) &dp->di_dirtable; lv = ilinelock->lv; for (n = 0; n < ilinelock->index; n++, lv++) { memcpy(&xp->xad[lv->offset], &p->xad[lv->offset], @@ -713,7 +713,7 @@ int diWrite(tid_t tid, struct inode *ip) * regular file: 16 byte (XAD slot) granularity */ if (type & tlckXTREE) { - xtpage_t *p, *xp; + xtroot_t *p, *xp; xad_t *xad; /* diff --git a/fs/jfs/jfs_incore.h b/fs/jfs/jfs_incore.h index 721def69e732..dd4264aa9bed 100644 --- a/fs/jfs/jfs_incore.h +++ b/fs/jfs/jfs_incore.h @@ -66,7 +66,7 @@ struct jfs_inode_info { lid_t xtlid; /* lid of xtree lock on directory */ union { struct { - xtpage_t _xtroot; /* 288: xtree root */ + xtroot_t _xtroot; /* 288: xtree root */ struct inomap *_imap; /* 4: inode map header */ } file; struct { diff --git a/fs/jfs/jfs_txnmgr.c b/fs/jfs/jfs_txnmgr.c index ce4b4760fcb1..dccc8b3f1045 100644 --- a/fs/jfs/jfs_txnmgr.c +++ b/fs/jfs/jfs_txnmgr.c @@ -783,7 +783,7 @@ struct tlock *txLock(tid_t tid, struct inode *ip, struct metapage * mp, if (mp->xflag & COMMIT_PAGE) p = (xtpage_t *) mp->data; else - p = &jfs_ip->i_xtroot; + p = (xtpage_t *) &jfs_ip->i_xtroot; xtlck->lwm.offset = le16_to_cpu(p->header.nextindex); } @@ -1676,7 +1676,7 @@ static void xtLog(struct jfs_log * log, struct tblock * tblk, struct lrd * lrd, if (tlck->type & tlckBTROOT) { lrd->log.redopage.type |= cpu_to_le16(LOG_BTROOT); - p = &JFS_IP(ip)->i_xtroot; + p = (xtpage_t *) &JFS_IP(ip)->i_xtroot; if (S_ISDIR(ip->i_mode)) lrd->log.redopage.type |= cpu_to_le16(LOG_DIR_XTREE); diff --git a/fs/jfs/jfs_xtree.c b/fs/jfs/jfs_xtree.c index 2d304cee884c..5ee618d17e77 100644 --- a/fs/jfs/jfs_xtree.c +++ b/fs/jfs/jfs_xtree.c @@ -1213,7 +1213,7 @@ xtSplitRoot(tid_t tid, struct xtlock *xtlck; int rc; - sp = &JFS_IP(ip)->i_xtroot; + sp = (xtpage_t *) &JFS_IP(ip)->i_xtroot; INCREMENT(xtStat.split); @@ -2098,7 +2098,7 @@ int xtAppend(tid_t tid, /* transaction id */ */ void xtInitRoot(tid_t tid, struct inode *ip) { - xtpage_t *p; + xtroot_t *p; /* * acquire a transaction lock on the root diff --git a/fs/jfs/jfs_xtree.h b/fs/jfs/jfs_xtree.h index ad7592191d76..0f6cf5a1ce75 100644 --- a/fs/jfs/jfs_xtree.h +++ b/fs/jfs/jfs_xtree.h @@ -65,24 +65,33 @@ struct xadlist { #define XTPAGEMAXSLOT 256 #define XTENTRYSTART 2 +struct xtheader { + __le64 next; /* 8: */ + __le64 prev; /* 8: */ + + u8 flag; /* 1: */ + u8 rsrvd1; /* 1: */ + __le16 nextindex; /* 2: next index = number of entries */ + __le16 maxentry; /* 2: max number of entries */ + __le16 rsrvd2; /* 2: */ + + pxd_t self; /* 8: self */ +}; + +/* + * xtree root (in inode): + */ +typedef union { + struct xtheader header; + xad_t xad[XTROOTMAXSLOT]; /* 16 * maxentry: xad array */ +} xtroot_t; + /* * xtree page: */ typedef union { - struct xtheader { - __le64 next; /* 8: */ - __le64 prev; /* 8: */ - - u8 flag; /* 1: */ - u8 rsrvd1; /* 1: */ - __le16 nextindex; /* 2: next index = number of entries */ - __le16 maxentry; /* 2: max number of entries */ - __le16 rsrvd2; /* 2: */ - - pxd_t self; /* 8: self */ - } header; /* (32) */ - - xad_t xad[XTROOTMAXSLOT]; /* 16 * maxentry: xad array */ + struct xtheader header; + xad_t xad[XTPAGEMAXSLOT]; /* 16 * maxentry: xad array */ } xtpage_t; /* From 798eecaea0f0366306cbc76986a83041a7e8669f Mon Sep 17 00:00:00 2001 From: Gao Xiang Date: Sat, 21 Oct 2023 10:01:37 +0800 Subject: [PATCH 55/69] erofs: don't warn MicroLZMA format anymore The LZMA algorithm support has been landed for more than one year since Linux 5.16. Besides, the new XZ Utils 5.4 has been available in most Linux distributions. Reviewed-by: Chao Yu Signed-off-by: Gao Xiang Link: https://lore.kernel.org/r/20231021020137.1646959-1-hsiangkao@linux.alibaba.com --- fs/erofs/Kconfig | 7 ++----- fs/erofs/decompressor_lzma.c | 2 -- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/fs/erofs/Kconfig b/fs/erofs/Kconfig index f6dc961e6c2b..e540648dedc2 100644 --- a/fs/erofs/Kconfig +++ b/fs/erofs/Kconfig @@ -91,13 +91,10 @@ config EROFS_FS_ZIP_LZMA select XZ_DEC_MICROLZMA help Saying Y here includes support for reading EROFS file systems - containing LZMA compressed data, specifically called microLZMA. it - gives better compression ratios than the LZ4 algorithm, at the + containing LZMA compressed data, specifically called microLZMA. It + gives better compression ratios than the default LZ4 format, at the expense of more CPU overhead. - LZMA support is an experimental feature for now and so most file - systems will be readable without selecting this option. - If unsure, say N. config EROFS_FS_ZIP_DEFLATE diff --git a/fs/erofs/decompressor_lzma.c b/fs/erofs/decompressor_lzma.c index dee10d22ada9..5f413f19a064 100644 --- a/fs/erofs/decompressor_lzma.c +++ b/fs/erofs/decompressor_lzma.c @@ -96,8 +96,6 @@ int z_erofs_load_lzma_config(struct super_block *sb, return -EINVAL; } - erofs_info(sb, "EXPERIMENTAL MicroLZMA in use. Use at your own risk!"); - /* in case 2 z_erofs_load_lzma_config() race to avoid deadlock */ mutex_lock(&lzma_resize_mutex); From efb4fb02cef3ab410b603c8f0e1c67f61d55f542 Mon Sep 17 00:00:00 2001 From: Gao Xiang Date: Sun, 22 Oct 2023 21:09:57 +0800 Subject: [PATCH 56/69] erofs: simplify compression configuration parser Move erofs_load_compr_cfgs() into decompressor.c as well as introduce a callback instead of a hard-coded switch for each algorithm for simplicity. Reviewed-by: Chao Yu Signed-off-by: Gao Xiang Link: https://lore.kernel.org/r/20231022130957.11398-1-xiang@kernel.org --- fs/erofs/compress.h | 6 +++ fs/erofs/decompressor.c | 62 ++++++++++++++++++++++++++-- fs/erofs/decompressor_deflate.c | 5 ++- fs/erofs/decompressor_lzma.c | 4 +- fs/erofs/internal.h | 38 +---------------- fs/erofs/super.c | 72 ++++----------------------------- 6 files changed, 79 insertions(+), 108 deletions(-) diff --git a/fs/erofs/compress.h b/fs/erofs/compress.h index 349c3316ae6b..279933e007d2 100644 --- a/fs/erofs/compress.h +++ b/fs/erofs/compress.h @@ -21,6 +21,8 @@ struct z_erofs_decompress_req { }; struct z_erofs_decompressor { + int (*config)(struct super_block *sb, struct erofs_super_block *dsb, + void *data, int size); int (*decompress)(struct z_erofs_decompress_req *rq, struct page **pagepool); char *name; @@ -92,6 +94,10 @@ int z_erofs_fixup_insize(struct z_erofs_decompress_req *rq, const char *padbuf, extern const struct z_erofs_decompressor erofs_decompressors[]; /* prototypes for specific algorithms */ +int z_erofs_load_lzma_config(struct super_block *sb, + struct erofs_super_block *dsb, void *data, int size); +int z_erofs_load_deflate_config(struct super_block *sb, + struct erofs_super_block *dsb, void *data, int size); int z_erofs_lzma_decompress(struct z_erofs_decompress_req *rq, struct page **pagepool); int z_erofs_deflate_decompress(struct z_erofs_decompress_req *rq, diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c index 332ec5f74002..e75edc8f1753 100644 --- a/fs/erofs/decompressor.c +++ b/fs/erofs/decompressor.c @@ -24,11 +24,11 @@ struct z_erofs_lz4_decompress_ctx { unsigned int oend; }; -int z_erofs_load_lz4_config(struct super_block *sb, - struct erofs_super_block *dsb, - struct z_erofs_lz4_cfgs *lz4, int size) +static int z_erofs_load_lz4_config(struct super_block *sb, + struct erofs_super_block *dsb, void *data, int size) { struct erofs_sb_info *sbi = EROFS_SB(sb); + struct z_erofs_lz4_cfgs *lz4 = data; u16 distance; if (lz4) { @@ -370,19 +370,75 @@ const struct z_erofs_decompressor erofs_decompressors[] = { .name = "interlaced" }, [Z_EROFS_COMPRESSION_LZ4] = { + .config = z_erofs_load_lz4_config, .decompress = z_erofs_lz4_decompress, .name = "lz4" }, #ifdef CONFIG_EROFS_FS_ZIP_LZMA [Z_EROFS_COMPRESSION_LZMA] = { + .config = z_erofs_load_lzma_config, .decompress = z_erofs_lzma_decompress, .name = "lzma" }, #endif #ifdef CONFIG_EROFS_FS_ZIP_DEFLATE [Z_EROFS_COMPRESSION_DEFLATE] = { + .config = z_erofs_load_deflate_config, .decompress = z_erofs_deflate_decompress, .name = "deflate" }, #endif }; + +int z_erofs_parse_cfgs(struct super_block *sb, struct erofs_super_block *dsb) +{ + struct erofs_sb_info *sbi = EROFS_SB(sb); + struct erofs_buf buf = __EROFS_BUF_INITIALIZER; + unsigned int algs, alg; + erofs_off_t offset; + int size, ret = 0; + + if (!erofs_sb_has_compr_cfgs(sbi)) { + sbi->available_compr_algs = Z_EROFS_COMPRESSION_LZ4; + return z_erofs_load_lz4_config(sb, dsb, NULL, 0); + } + + sbi->available_compr_algs = le16_to_cpu(dsb->u1.available_compr_algs); + if (sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS) { + erofs_err(sb, "unidentified algorithms %x, please upgrade kernel", + sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS); + return -EOPNOTSUPP; + } + + erofs_init_metabuf(&buf, sb); + offset = EROFS_SUPER_OFFSET + sbi->sb_size; + alg = 0; + for (algs = sbi->available_compr_algs; algs; algs >>= 1, ++alg) { + void *data; + + if (!(algs & 1)) + continue; + + data = erofs_read_metadata(sb, &buf, &offset, &size); + if (IS_ERR(data)) { + ret = PTR_ERR(data); + break; + } + + if (alg >= ARRAY_SIZE(erofs_decompressors) || + !erofs_decompressors[alg].config) { + erofs_err(sb, "algorithm %d isn't enabled on this kernel", + alg); + ret = -EOPNOTSUPP; + } else { + ret = erofs_decompressors[alg].config(sb, + dsb, data, size); + } + + kfree(data); + if (ret) + break; + } + erofs_put_metabuf(&buf); + return ret; +} diff --git a/fs/erofs/decompressor_deflate.c b/fs/erofs/decompressor_deflate.c index 19e5bdeb30b6..0e1946a6bda5 100644 --- a/fs/erofs/decompressor_deflate.c +++ b/fs/erofs/decompressor_deflate.c @@ -77,9 +77,10 @@ out_failed: } int z_erofs_load_deflate_config(struct super_block *sb, - struct erofs_super_block *dsb, - struct z_erofs_deflate_cfgs *dfl, int size) + struct erofs_super_block *dsb, void *data, int size) { + struct z_erofs_deflate_cfgs *dfl = data; + if (!dfl || size < sizeof(struct z_erofs_deflate_cfgs)) { erofs_err(sb, "invalid deflate cfgs, size=%u", size); return -EINVAL; diff --git a/fs/erofs/decompressor_lzma.c b/fs/erofs/decompressor_lzma.c index 5f413f19a064..852dd8eac5df 100644 --- a/fs/erofs/decompressor_lzma.c +++ b/fs/erofs/decompressor_lzma.c @@ -72,10 +72,10 @@ int __init z_erofs_lzma_init(void) } int z_erofs_load_lzma_config(struct super_block *sb, - struct erofs_super_block *dsb, - struct z_erofs_lzma_cfgs *lzma, int size) + struct erofs_super_block *dsb, void *data, int size) { static DEFINE_MUTEX(lzma_resize_mutex); + struct z_erofs_lzma_cfgs *lzma = data; unsigned int dict_size, i; struct z_erofs_lzma *strm, *head = NULL; int err; diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h index 4ff88d0dd980..d8de61350dc0 100644 --- a/fs/erofs/internal.h +++ b/fs/erofs/internal.h @@ -469,9 +469,6 @@ int __init z_erofs_init_zip_subsystem(void); void z_erofs_exit_zip_subsystem(void); int erofs_try_to_free_all_cached_pages(struct erofs_sb_info *sbi, struct erofs_workgroup *egrp); -int z_erofs_load_lz4_config(struct super_block *sb, - struct erofs_super_block *dsb, - struct z_erofs_lz4_cfgs *lz4, int len); int z_erofs_map_blocks_iter(struct inode *inode, struct erofs_map_blocks *map, int flags); void *erofs_get_pcpubuf(unsigned int requiredpages); @@ -480,6 +477,7 @@ int erofs_pcpubuf_growsize(unsigned int nrpages); void __init erofs_pcpubuf_init(void); void erofs_pcpubuf_exit(void); int erofs_init_managed_cache(struct super_block *sb); +int z_erofs_parse_cfgs(struct super_block *sb, struct erofs_super_block *dsb); #else static inline void erofs_shrinker_register(struct super_block *sb) {} static inline void erofs_shrinker_unregister(struct super_block *sb) {} @@ -487,16 +485,6 @@ static inline int erofs_init_shrinker(void) { return 0; } static inline void erofs_exit_shrinker(void) {} static inline int z_erofs_init_zip_subsystem(void) { return 0; } static inline void z_erofs_exit_zip_subsystem(void) {} -static inline int z_erofs_load_lz4_config(struct super_block *sb, - struct erofs_super_block *dsb, - struct z_erofs_lz4_cfgs *lz4, int len) -{ - if (lz4 || dsb->u1.lz4_max_distance) { - erofs_err(sb, "lz4 algorithm isn't enabled"); - return -EINVAL; - } - return 0; -} static inline void erofs_pcpubuf_init(void) {} static inline void erofs_pcpubuf_exit(void) {} static inline int erofs_init_managed_cache(struct super_block *sb) { return 0; } @@ -505,41 +493,17 @@ static inline int erofs_init_managed_cache(struct super_block *sb) { return 0; } #ifdef CONFIG_EROFS_FS_ZIP_LZMA int __init z_erofs_lzma_init(void); void z_erofs_lzma_exit(void); -int z_erofs_load_lzma_config(struct super_block *sb, - struct erofs_super_block *dsb, - struct z_erofs_lzma_cfgs *lzma, int size); #else static inline int z_erofs_lzma_init(void) { return 0; } static inline int z_erofs_lzma_exit(void) { return 0; } -static inline int z_erofs_load_lzma_config(struct super_block *sb, - struct erofs_super_block *dsb, - struct z_erofs_lzma_cfgs *lzma, int size) { - if (lzma) { - erofs_err(sb, "lzma algorithm isn't enabled"); - return -EINVAL; - } - return 0; -} #endif /* !CONFIG_EROFS_FS_ZIP_LZMA */ #ifdef CONFIG_EROFS_FS_ZIP_DEFLATE int __init z_erofs_deflate_init(void); void z_erofs_deflate_exit(void); -int z_erofs_load_deflate_config(struct super_block *sb, - struct erofs_super_block *dsb, - struct z_erofs_deflate_cfgs *dfl, int size); #else static inline int z_erofs_deflate_init(void) { return 0; } static inline int z_erofs_deflate_exit(void) { return 0; } -static inline int z_erofs_load_deflate_config(struct super_block *sb, - struct erofs_super_block *dsb, - struct z_erofs_deflate_cfgs *dfl, int size) { - if (dfl) { - erofs_err(sb, "deflate algorithm isn't enabled"); - return -EINVAL; - } - return 0; -} #endif /* !CONFIG_EROFS_FS_ZIP_DEFLATE */ #ifdef CONFIG_EROFS_FS_ONDEMAND diff --git a/fs/erofs/super.c b/fs/erofs/super.c index 3700af9ee173..cc44fb2e001e 100644 --- a/fs/erofs/super.c +++ b/fs/erofs/super.c @@ -156,68 +156,15 @@ void *erofs_read_metadata(struct super_block *sb, struct erofs_buf *buf, return buffer; } -#ifdef CONFIG_EROFS_FS_ZIP -static int erofs_load_compr_cfgs(struct super_block *sb, - struct erofs_super_block *dsb) +#ifndef CONFIG_EROFS_FS_ZIP +static int z_erofs_parse_cfgs(struct super_block *sb, + struct erofs_super_block *dsb) { - struct erofs_sb_info *sbi = EROFS_SB(sb); - struct erofs_buf buf = __EROFS_BUF_INITIALIZER; - unsigned int algs, alg; - erofs_off_t offset; - int size, ret = 0; + if (!dsb->u1.available_compr_algs) + return 0; - sbi->available_compr_algs = le16_to_cpu(dsb->u1.available_compr_algs); - if (sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS) { - erofs_err(sb, "try to load compressed fs with unsupported algorithms %x", - sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS); - return -EINVAL; - } - - erofs_init_metabuf(&buf, sb); - offset = EROFS_SUPER_OFFSET + sbi->sb_size; - alg = 0; - for (algs = sbi->available_compr_algs; algs; algs >>= 1, ++alg) { - void *data; - - if (!(algs & 1)) - continue; - - data = erofs_read_metadata(sb, &buf, &offset, &size); - if (IS_ERR(data)) { - ret = PTR_ERR(data); - break; - } - - switch (alg) { - case Z_EROFS_COMPRESSION_LZ4: - ret = z_erofs_load_lz4_config(sb, dsb, data, size); - break; - case Z_EROFS_COMPRESSION_LZMA: - ret = z_erofs_load_lzma_config(sb, dsb, data, size); - break; - case Z_EROFS_COMPRESSION_DEFLATE: - ret = z_erofs_load_deflate_config(sb, dsb, data, size); - break; - default: - DBG_BUGON(1); - ret = -EFAULT; - } - kfree(data); - if (ret) - break; - } - erofs_put_metabuf(&buf); - return ret; -} -#else -static int erofs_load_compr_cfgs(struct super_block *sb, - struct erofs_super_block *dsb) -{ - if (dsb->u1.available_compr_algs) { - erofs_err(sb, "try to load compressed fs when compression is disabled"); - return -EINVAL; - } - return 0; + erofs_err(sb, "compression disabled, unable to mount compressed EROFS"); + return -EOPNOTSUPP; } #endif @@ -406,10 +353,7 @@ static int erofs_read_superblock(struct super_block *sb) } /* parse on-disk compression configurations */ - if (erofs_sb_has_compr_cfgs(sbi)) - ret = erofs_load_compr_cfgs(sb, dsb); - else - ret = z_erofs_load_lz4_config(sb, dsb, NULL, 0); + ret = z_erofs_parse_cfgs(sb, dsb); if (ret < 0) goto out; From 6b8a113cae6cc517579a33ad484355c3e4b3d8e7 Mon Sep 17 00:00:00 2001 From: Ferry Meng Date: Thu, 26 Oct 2023 10:16:26 +0800 Subject: [PATCH 57/69] erofs: get rid of ROOT_NID() Let's open code this helper for simplicity. Signed-off-by: Ferry Meng Reviewed-by: Gao Xiang Reviewed-by: Yue Hu Reviewed-by: Chao Yu Link: https://lore.kernel.org/r/20231026021627.23284-1-mengferry@linux.alibaba.com Signed-off-by: Gao Xiang --- fs/erofs/internal.h | 2 -- fs/erofs/super.c | 6 +++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h index d8de61350dc0..9586674ba55c 100644 --- a/fs/erofs/internal.h +++ b/fs/erofs/internal.h @@ -228,8 +228,6 @@ struct erofs_buf { }; #define __EROFS_BUF_INITIALIZER ((struct erofs_buf){ .page = NULL }) -#define ROOT_NID(sb) ((sb)->root_nid) - #define erofs_blknr(sb, addr) ((addr) >> (sb)->s_blocksize_bits) #define erofs_blkoff(sb, addr) ((addr) & ((sb)->s_blocksize - 1)) #define erofs_pos(sb, blk) ((erofs_off_t)(blk) << (sb)->s_blocksize_bits) diff --git a/fs/erofs/super.c b/fs/erofs/super.c index cc44fb2e001e..b1518df3af00 100644 --- a/fs/erofs/super.c +++ b/fs/erofs/super.c @@ -668,13 +668,13 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc) xa_init(&sbi->managed_pslots); #endif - inode = erofs_iget(sb, ROOT_NID(sbi)); + inode = erofs_iget(sb, sbi->root_nid); if (IS_ERR(inode)) return PTR_ERR(inode); if (!S_ISDIR(inode->i_mode)) { erofs_err(sb, "rootino(nid %llu) is not a directory(i_mode %o)", - ROOT_NID(sbi), inode->i_mode); + sbi->root_nid, inode->i_mode); iput(inode); return -EINVAL; } @@ -704,7 +704,7 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc) if (err) return err; - erofs_info(sb, "mounted with root inode @ nid %llu.", ROOT_NID(sbi)); + erofs_info(sb, "mounted with root inode @ nid %llu.", sbi->root_nid); return 0; } From f5deddce60b50b55bcafeebaab1408d203b0f204 Mon Sep 17 00:00:00 2001 From: Ferry Meng Date: Thu, 26 Oct 2023 10:16:27 +0800 Subject: [PATCH 58/69] erofs: tidy up redundant includes - Remove unused includes like and ; - Move common includes into "internal.h". Signed-off-by: Ferry Meng Reviewed-by: Gao Xiang Reviewed-by: Yue Hu Reviewed-by: Chao Yu Link: https://lore.kernel.org/r/20231026021627.23284-2-mengferry@linux.alibaba.com Signed-off-by: Gao Xiang --- fs/erofs/data.c | 2 -- fs/erofs/decompressor.c | 1 - fs/erofs/decompressor_deflate.c | 1 - fs/erofs/decompressor_lzma.c | 1 - fs/erofs/internal.h | 2 ++ fs/erofs/super.c | 3 --- 6 files changed, 2 insertions(+), 8 deletions(-) diff --git a/fs/erofs/data.c b/fs/erofs/data.c index 0c2c99c58b5e..ceb6c248bf40 100644 --- a/fs/erofs/data.c +++ b/fs/erofs/data.c @@ -5,9 +5,7 @@ * Copyright (C) 2021, Alibaba Cloud */ #include "internal.h" -#include #include -#include #include void erofs_unmap_metabuf(struct erofs_buf *buf) diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c index e75edc8f1753..021be5feb1bc 100644 --- a/fs/erofs/decompressor.c +++ b/fs/erofs/decompressor.c @@ -4,7 +4,6 @@ * https://www.huawei.com/ */ #include "compress.h" -#include #include #ifndef LZ4_DISTANCE_MAX /* history window size */ diff --git a/fs/erofs/decompressor_deflate.c b/fs/erofs/decompressor_deflate.c index 0e1946a6bda5..daf3c1bdeab8 100644 --- a/fs/erofs/decompressor_deflate.c +++ b/fs/erofs/decompressor_deflate.c @@ -1,5 +1,4 @@ // SPDX-License-Identifier: GPL-2.0-or-later -#include #include #include "compress.h" diff --git a/fs/erofs/decompressor_lzma.c b/fs/erofs/decompressor_lzma.c index 852dd8eac5df..2dd14f99c1dc 100644 --- a/fs/erofs/decompressor_lzma.c +++ b/fs/erofs/decompressor_lzma.c @@ -1,6 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-or-later #include -#include #include "compress.h" struct z_erofs_lzma { diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h index 9586674ba55c..00a2b392f445 100644 --- a/fs/erofs/internal.h +++ b/fs/erofs/internal.h @@ -8,8 +8,10 @@ #define __EROFS_INTERNAL_H #include +#include #include #include +#include #include #include #include diff --git a/fs/erofs/super.c b/fs/erofs/super.c index b1518df3af00..93aba02346aa 100644 --- a/fs/erofs/super.c +++ b/fs/erofs/super.c @@ -4,14 +4,11 @@ * https://www.huawei.com/ * Copyright (C) 2021, Alibaba Cloud */ -#include #include -#include #include #include #include #include -#include #include #include "xattr.h" From 0ab8ba71868594acfc717b8c7d1738b9118118ec Mon Sep 17 00:00:00 2001 From: Jan Cincera Date: Mon, 30 Oct 2023 20:53:18 +0900 Subject: [PATCH 59/69] exfat: add ioctls for accessing attributes Add GET and SET attributes ioctls to enable attribute modification. We already do this in FAT and a few userspace utils made for it would benefit from this also working on exFAT, namely fatattr. Signed-off-by: Jan Cincera Signed-off-by: Namjae Jeon --- fs/exfat/dir.c | 8 ++-- fs/exfat/exfat_fs.h | 12 +++--- fs/exfat/exfat_raw.h | 17 ++++---- fs/exfat/file.c | 97 +++++++++++++++++++++++++++++++++++++++++++- fs/exfat/inode.c | 6 +-- fs/exfat/namei.c | 16 ++++---- fs/exfat/super.c | 4 +- 7 files changed, 128 insertions(+), 32 deletions(-) diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c index e1586bba6d86..fdd46aa466e1 100644 --- a/fs/exfat/dir.c +++ b/fs/exfat/dir.c @@ -287,7 +287,7 @@ get_new: mutex_unlock(&EXFAT_SB(sb)->s_lock); if (!dir_emit(ctx, nb->lfn, strlen(nb->lfn), inum, - (de.attr & ATTR_SUBDIR) ? DT_DIR : DT_REG)) + (de.attr & EXFAT_ATTR_SUBDIR) ? DT_DIR : DT_REG)) goto out; ctx->pos = cpos; goto get_new; @@ -359,7 +359,7 @@ unsigned int exfat_get_entry_type(struct exfat_dentry *ep) if (ep->type == EXFAT_VOLUME) return TYPE_VOLUME; if (ep->type == EXFAT_FILE) { - if (le16_to_cpu(ep->dentry.file.attr) & ATTR_SUBDIR) + if (le16_to_cpu(ep->dentry.file.attr) & EXFAT_ATTR_SUBDIR) return TYPE_DIR; return TYPE_FILE; } @@ -410,10 +410,10 @@ static void exfat_set_entry_type(struct exfat_dentry *ep, unsigned int type) ep->type = EXFAT_VOLUME; } else if (type == TYPE_DIR) { ep->type = EXFAT_FILE; - ep->dentry.file.attr = cpu_to_le16(ATTR_SUBDIR); + ep->dentry.file.attr = cpu_to_le16(EXFAT_ATTR_SUBDIR); } else if (type == TYPE_FILE) { ep->type = EXFAT_FILE; - ep->dentry.file.attr = cpu_to_le16(ATTR_ARCHIVE); + ep->dentry.file.attr = cpu_to_le16(EXFAT_ATTR_ARCHIVE); } } diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h index f78b614f44dc..99208902883b 100644 --- a/fs/exfat/exfat_fs.h +++ b/fs/exfat/exfat_fs.h @@ -357,10 +357,10 @@ static inline int exfat_mode_can_hold_ro(struct inode *inode) static inline mode_t exfat_make_mode(struct exfat_sb_info *sbi, unsigned short attr, mode_t mode) { - if ((attr & ATTR_READONLY) && !(attr & ATTR_SUBDIR)) + if ((attr & EXFAT_ATTR_READONLY) && !(attr & EXFAT_ATTR_SUBDIR)) mode &= ~0222; - if (attr & ATTR_SUBDIR) + if (attr & EXFAT_ATTR_SUBDIR) return (mode & ~sbi->options.fs_dmask) | S_IFDIR; return (mode & ~sbi->options.fs_fmask) | S_IFREG; @@ -372,18 +372,18 @@ static inline unsigned short exfat_make_attr(struct inode *inode) unsigned short attr = EXFAT_I(inode)->attr; if (S_ISDIR(inode->i_mode)) - attr |= ATTR_SUBDIR; + attr |= EXFAT_ATTR_SUBDIR; if (exfat_mode_can_hold_ro(inode) && !(inode->i_mode & 0222)) - attr |= ATTR_READONLY; + attr |= EXFAT_ATTR_READONLY; return attr; } static inline void exfat_save_attr(struct inode *inode, unsigned short attr) { if (exfat_mode_can_hold_ro(inode)) - EXFAT_I(inode)->attr = attr & (ATTR_RWMASK | ATTR_READONLY); + EXFAT_I(inode)->attr = attr & (EXFAT_ATTR_RWMASK | EXFAT_ATTR_READONLY); else - EXFAT_I(inode)->attr = attr & ATTR_RWMASK; + EXFAT_I(inode)->attr = attr & EXFAT_ATTR_RWMASK; } static inline bool exfat_is_last_sector_in_cluster(struct exfat_sb_info *sbi, diff --git a/fs/exfat/exfat_raw.h b/fs/exfat/exfat_raw.h index 0ece2e43cf49..971a1ccd0e89 100644 --- a/fs/exfat/exfat_raw.h +++ b/fs/exfat/exfat_raw.h @@ -64,15 +64,16 @@ #define CS_DEFAULT 2 /* file attributes */ -#define ATTR_READONLY 0x0001 -#define ATTR_HIDDEN 0x0002 -#define ATTR_SYSTEM 0x0004 -#define ATTR_VOLUME 0x0008 -#define ATTR_SUBDIR 0x0010 -#define ATTR_ARCHIVE 0x0020 +#define EXFAT_ATTR_READONLY 0x0001 +#define EXFAT_ATTR_HIDDEN 0x0002 +#define EXFAT_ATTR_SYSTEM 0x0004 +#define EXFAT_ATTR_VOLUME 0x0008 +#define EXFAT_ATTR_SUBDIR 0x0010 +#define EXFAT_ATTR_ARCHIVE 0x0020 -#define ATTR_RWMASK (ATTR_HIDDEN | ATTR_SYSTEM | ATTR_VOLUME | \ - ATTR_SUBDIR | ATTR_ARCHIVE) +#define EXFAT_ATTR_RWMASK (EXFAT_ATTR_HIDDEN | EXFAT_ATTR_SYSTEM | \ + EXFAT_ATTR_VOLUME | EXFAT_ATTR_SUBDIR | \ + EXFAT_ATTR_ARCHIVE) #define BOOTSEC_JUMP_BOOT_LEN 3 #define BOOTSEC_FS_NAME_LEN 8 diff --git a/fs/exfat/file.c b/fs/exfat/file.c index 30ee2c8d36a5..02c4e2937879 100644 --- a/fs/exfat/file.c +++ b/fs/exfat/file.c @@ -8,6 +8,9 @@ #include #include #include +#include +#include +#include #include "exfat_raw.h" #include "exfat_fs.h" @@ -144,7 +147,7 @@ int __exfat_truncate(struct inode *inode) } if (ei->type == TYPE_FILE) - ei->attr |= ATTR_ARCHIVE; + ei->attr |= EXFAT_ATTR_ARCHIVE; /* * update the directory entry @@ -315,6 +318,93 @@ out: return error; } +/* + * modified ioctls from fat/file.c by Welmer Almesberger + */ +static int exfat_ioctl_get_attributes(struct inode *inode, u32 __user *user_attr) +{ + u32 attr; + + inode_lock_shared(inode); + attr = exfat_make_attr(inode); + inode_unlock_shared(inode); + + return put_user(attr, user_attr); +} + +static int exfat_ioctl_set_attributes(struct file *file, u32 __user *user_attr) +{ + struct inode *inode = file_inode(file); + struct exfat_sb_info *sbi = EXFAT_SB(inode->i_sb); + int is_dir = S_ISDIR(inode->i_mode); + u32 attr, oldattr; + struct iattr ia; + int err; + + err = get_user(attr, user_attr); + if (err) + goto out; + + err = mnt_want_write_file(file); + if (err) + goto out; + inode_lock(inode); + + oldattr = exfat_make_attr(inode); + + /* + * Mask attributes so we don't set reserved fields. + */ + attr &= (EXFAT_ATTR_READONLY | EXFAT_ATTR_HIDDEN | EXFAT_ATTR_SYSTEM | + EXFAT_ATTR_ARCHIVE); + attr |= (is_dir ? EXFAT_ATTR_SUBDIR : 0); + + /* Equivalent to a chmod() */ + ia.ia_valid = ATTR_MODE | ATTR_CTIME; + ia.ia_ctime = current_time(inode); + if (is_dir) + ia.ia_mode = exfat_make_mode(sbi, attr, 0777); + else + ia.ia_mode = exfat_make_mode(sbi, attr, 0666 | (inode->i_mode & 0111)); + + /* The root directory has no attributes */ + if (inode->i_ino == EXFAT_ROOT_INO && attr != EXFAT_ATTR_SUBDIR) { + err = -EINVAL; + goto out_unlock_inode; + } + + if (((attr | oldattr) & EXFAT_ATTR_SYSTEM) && + !capable(CAP_LINUX_IMMUTABLE)) { + err = -EPERM; + goto out_unlock_inode; + } + + /* + * The security check is questionable... We single + * out the RO attribute for checking by the security + * module, just because it maps to a file mode. + */ + err = security_inode_setattr(file_mnt_idmap(file), + file->f_path.dentry, &ia); + if (err) + goto out_unlock_inode; + + /* This MUST be done before doing anything irreversible... */ + err = exfat_setattr(file_mnt_idmap(file), file->f_path.dentry, &ia); + if (err) + goto out_unlock_inode; + + fsnotify_change(file->f_path.dentry, ia.ia_valid); + + exfat_save_attr(inode, attr); + mark_inode_dirty(inode); +out_unlock_inode: + inode_unlock(inode); + mnt_drop_write_file(file); +out: + return err; +} + static int exfat_ioctl_fitrim(struct inode *inode, unsigned long arg) { struct fstrim_range range; @@ -345,8 +435,13 @@ static int exfat_ioctl_fitrim(struct inode *inode, unsigned long arg) long exfat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) { struct inode *inode = file_inode(filp); + u32 __user *user_attr = (u32 __user *)arg; switch (cmd) { + case FAT_IOCTL_GET_ATTRIBUTES: + return exfat_ioctl_get_attributes(inode, user_attr); + case FAT_IOCTL_SET_ATTRIBUTES: + return exfat_ioctl_set_attributes(filp, user_attr); case FITRIM: return exfat_ioctl_fitrim(inode, arg); default: diff --git a/fs/exfat/inode.c b/fs/exfat/inode.c index a2185e6f0548..875234179d1f 100644 --- a/fs/exfat/inode.c +++ b/fs/exfat/inode.c @@ -400,9 +400,9 @@ static int exfat_write_end(struct file *file, struct address_space *mapping, if (err < len) exfat_write_failed(mapping, pos+len); - if (!(err < 0) && !(ei->attr & ATTR_ARCHIVE)) { + if (!(err < 0) && !(ei->attr & EXFAT_ATTR_ARCHIVE)) { inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode)); - ei->attr |= ATTR_ARCHIVE; + ei->attr |= EXFAT_ATTR_ARCHIVE; mark_inode_dirty(inode); } @@ -550,7 +550,7 @@ static int exfat_fill_inode(struct inode *inode, struct exfat_dir_entry *info) inode_inc_iversion(inode); inode->i_generation = get_random_u32(); - if (info->attr & ATTR_SUBDIR) { /* directory */ + if (info->attr & EXFAT_ATTR_SUBDIR) { /* directory */ inode->i_generation &= ~1; inode->i_mode = exfat_make_mode(sbi, info->attr, 0777); inode->i_op = &exfat_dir_inode_operations; diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c index b92e46916dea..d1ab95ded156 100644 --- a/fs/exfat/namei.c +++ b/fs/exfat/namei.c @@ -534,12 +534,12 @@ static int exfat_add_entry(struct inode *inode, const char *path, info->type = type; if (type == TYPE_FILE) { - info->attr = ATTR_ARCHIVE; + info->attr = EXFAT_ATTR_ARCHIVE; info->start_clu = EXFAT_EOF_CLUSTER; info->size = 0; info->num_subdirs = 0; } else { - info->attr = ATTR_SUBDIR; + info->attr = EXFAT_ATTR_SUBDIR; info->start_clu = start_clu; info->size = clu_size; info->num_subdirs = EXFAT_MIN_SUBDIR; @@ -1033,8 +1033,8 @@ static int exfat_rename_file(struct inode *inode, struct exfat_chain *p_dir, *epnew = *epold; if (exfat_get_entry_type(epnew) == TYPE_FILE) { - epnew->dentry.file.attr |= cpu_to_le16(ATTR_ARCHIVE); - ei->attr |= ATTR_ARCHIVE; + epnew->dentry.file.attr |= cpu_to_le16(EXFAT_ATTR_ARCHIVE); + ei->attr |= EXFAT_ATTR_ARCHIVE; } exfat_update_bh(new_bh, sync); brelse(old_bh); @@ -1065,8 +1065,8 @@ static int exfat_rename_file(struct inode *inode, struct exfat_chain *p_dir, ei->entry = newentry; } else { if (exfat_get_entry_type(epold) == TYPE_FILE) { - epold->dentry.file.attr |= cpu_to_le16(ATTR_ARCHIVE); - ei->attr |= ATTR_ARCHIVE; + epold->dentry.file.attr |= cpu_to_le16(EXFAT_ATTR_ARCHIVE); + ei->attr |= EXFAT_ATTR_ARCHIVE; } exfat_update_bh(old_bh, sync); brelse(old_bh); @@ -1114,8 +1114,8 @@ static int exfat_move_file(struct inode *inode, struct exfat_chain *p_olddir, *epnew = *epmov; if (exfat_get_entry_type(epnew) == TYPE_FILE) { - epnew->dentry.file.attr |= cpu_to_le16(ATTR_ARCHIVE); - ei->attr |= ATTR_ARCHIVE; + epnew->dentry.file.attr |= cpu_to_le16(EXFAT_ATTR_ARCHIVE); + ei->attr |= EXFAT_ATTR_ARCHIVE; } exfat_update_bh(new_bh, IS_DIRSYNC(inode)); brelse(mov_bh); diff --git a/fs/exfat/super.c b/fs/exfat/super.c index e919a68bf4a1..ecee1664ab7f 100644 --- a/fs/exfat/super.c +++ b/fs/exfat/super.c @@ -360,7 +360,7 @@ static int exfat_read_root(struct inode *inode) inode->i_gid = sbi->options.fs_gid; inode_inc_iversion(inode); inode->i_generation = 0; - inode->i_mode = exfat_make_mode(sbi, ATTR_SUBDIR, 0777); + inode->i_mode = exfat_make_mode(sbi, EXFAT_ATTR_SUBDIR, 0777); inode->i_op = &exfat_dir_inode_operations; inode->i_fop = &exfat_dir_operations; @@ -369,7 +369,7 @@ static int exfat_read_root(struct inode *inode) ei->i_size_aligned = i_size_read(inode); ei->i_size_ondisk = i_size_read(inode); - exfat_save_attr(inode, ATTR_SUBDIR); + exfat_save_attr(inode, EXFAT_ATTR_SUBDIR); ei->i_crtime = simple_inode_init_ts(inode); exfat_truncate_inode_atime(inode); return 0; From dab48b8f2fe7264d51ec9eed0adea0fe3c78830a Mon Sep 17 00:00:00 2001 From: Yuezhang Mo Date: Thu, 20 Jul 2023 14:23:08 +0800 Subject: [PATCH 60/69] exfat: support handle zero-size directory After repairing a corrupted file system with exfatprogs' fsck.exfat, zero-size directories may result. It is also possible to create zero-size directories in other exFAT implementation, such as Paragon ufsd dirver. As described in the specification, the lower directory size limits is 0 bytes. Without this commit, sub-directories and files cannot be created under a zero-size directory, and it cannot be removed. Signed-off-by: Yuezhang Mo Reviewed-by: Andy Wu Reviewed-by: Aoyama Wataru Signed-off-by: Namjae Jeon --- fs/exfat/namei.c | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c index d1ab95ded156..1e9c945130f4 100644 --- a/fs/exfat/namei.c +++ b/fs/exfat/namei.c @@ -351,14 +351,20 @@ static int exfat_find_empty_entry(struct inode *inode, if (exfat_check_max_dentries(inode)) return -ENOSPC; - /* we trust p_dir->size regardless of FAT type */ - if (exfat_find_last_cluster(sb, p_dir, &last_clu)) - return -EIO; - /* * Allocate new cluster to this directory */ - exfat_chain_set(&clu, last_clu + 1, 0, p_dir->flags); + if (ei->start_clu != EXFAT_EOF_CLUSTER) { + /* we trust p_dir->size regardless of FAT type */ + if (exfat_find_last_cluster(sb, p_dir, &last_clu)) + return -EIO; + + exfat_chain_set(&clu, last_clu + 1, 0, p_dir->flags); + } else { + /* This directory is empty */ + exfat_chain_set(&clu, EXFAT_EOF_CLUSTER, 0, + ALLOC_NO_FAT_CHAIN); + } /* allocate a cluster */ ret = exfat_alloc_cluster(inode, 1, &clu, IS_DIRSYNC(inode)); @@ -368,6 +374,11 @@ static int exfat_find_empty_entry(struct inode *inode, if (exfat_zeroed_cluster(inode, clu.dir)) return -EIO; + if (ei->start_clu == EXFAT_EOF_CLUSTER) { + ei->start_clu = clu.dir; + p_dir->dir = clu.dir; + } + /* append to the FAT chain */ if (clu.flags != p_dir->flags) { /* no-fat-chain bit is disabled, @@ -646,7 +657,7 @@ static int exfat_find(struct inode *dir, struct qstr *qname, info->type = exfat_get_entry_type(ep); info->attr = le16_to_cpu(ep->dentry.file.attr); info->size = le64_to_cpu(ep2->dentry.stream.valid_size); - if ((info->type == TYPE_FILE) && (info->size == 0)) { + if (info->size == 0) { info->flags = ALLOC_NO_FAT_CHAIN; info->start_clu = EXFAT_EOF_CLUSTER; } else { @@ -889,6 +900,9 @@ static int exfat_check_dir_empty(struct super_block *sb, dentries_per_clu = sbi->dentries_per_clu; + if (p_dir->dir == EXFAT_EOF_CLUSTER) + return 0; + exfat_chain_dup(&clu, p_dir); while (clu.dir != EXFAT_EOF_CLUSTER) { @@ -1256,7 +1270,8 @@ static int __exfat_rename(struct inode *old_parent_inode, } /* Free the clusters if new_inode is a dir(as if exfat_rmdir) */ - if (new_entry_type == TYPE_DIR) { + if (new_entry_type == TYPE_DIR && + new_ei->start_clu != EXFAT_EOF_CLUSTER) { /* new_ei, new_clu_to_free */ struct exfat_chain new_clu_to_free; From ee785c15b5906a69d4007b4754e8ae40fb41e0b4 Mon Sep 17 00:00:00 2001 From: Yuezhang Mo Date: Thu, 20 Jul 2023 14:40:08 +0800 Subject: [PATCH 61/69] exfat: support create zero-size directory This commit adds mount option 'zero_size_dir'. If this option enabled, don't allocate a cluster to directory when creating it, and set the directory size to 0. On Windows, a cluster is allocated for a directory when it is created, so the mount option is disabled by default. Signed-off-by: Yuezhang Mo Reviewed-by: Andy Wu Reviewed-by: Aoyama Wataru Signed-off-by: Namjae Jeon --- fs/exfat/dir.c | 12 ++++++------ fs/exfat/exfat_fs.h | 2 ++ fs/exfat/namei.c | 7 +++++-- fs/exfat/super.c | 7 +++++++ 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c index fdd46aa466e1..9f9295847a4e 100644 --- a/fs/exfat/dir.c +++ b/fs/exfat/dir.c @@ -418,11 +418,13 @@ static void exfat_set_entry_type(struct exfat_dentry *ep, unsigned int type) } static void exfat_init_stream_entry(struct exfat_dentry *ep, - unsigned char flags, unsigned int start_clu, - unsigned long long size) + unsigned int start_clu, unsigned long long size) { exfat_set_entry_type(ep, TYPE_STREAM); - ep->dentry.stream.flags = flags; + if (size == 0) + ep->dentry.stream.flags = ALLOC_FAT_CHAIN; + else + ep->dentry.stream.flags = ALLOC_NO_FAT_CHAIN; ep->dentry.stream.start_clu = cpu_to_le32(start_clu); ep->dentry.stream.valid_size = cpu_to_le64(size); ep->dentry.stream.size = cpu_to_le64(size); @@ -488,9 +490,7 @@ int exfat_init_dir_entry(struct inode *inode, struct exfat_chain *p_dir, if (!ep) return -EIO; - exfat_init_stream_entry(ep, - (type == TYPE_FILE) ? ALLOC_FAT_CHAIN : ALLOC_NO_FAT_CHAIN, - start_clu, size); + exfat_init_stream_entry(ep, start_clu, size); exfat_update_bh(bh, IS_DIRSYNC(inode)); brelse(bh); diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h index 99208902883b..a7a2c35d74fb 100644 --- a/fs/exfat/exfat_fs.h +++ b/fs/exfat/exfat_fs.h @@ -234,6 +234,8 @@ struct exfat_mount_options { discard:1, /* Issue discard requests on deletions */ keep_last_dots:1; /* Keep trailing periods in paths */ int time_offset; /* Offset of timestamps from UTC (in minutes) */ + /* Support creating zero-size directory, default: false */ + bool zero_size_dir; }; /* diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c index 1e9c945130f4..5d737e0b639a 100644 --- a/fs/exfat/namei.c +++ b/fs/exfat/namei.c @@ -518,7 +518,7 @@ static int exfat_add_entry(struct inode *inode, const char *path, goto out; } - if (type == TYPE_DIR) { + if (type == TYPE_DIR && !sbi->options.zero_size_dir) { ret = exfat_alloc_new_dir(inode, &clu); if (ret) goto out; @@ -551,7 +551,10 @@ static int exfat_add_entry(struct inode *inode, const char *path, info->num_subdirs = 0; } else { info->attr = EXFAT_ATTR_SUBDIR; - info->start_clu = start_clu; + if (sbi->options.zero_size_dir) + info->start_clu = EXFAT_EOF_CLUSTER; + else + info->start_clu = start_clu; info->size = clu_size; info->num_subdirs = EXFAT_MIN_SUBDIR; } diff --git a/fs/exfat/super.c b/fs/exfat/super.c index ecee1664ab7f..d9d4fa91010b 100644 --- a/fs/exfat/super.c +++ b/fs/exfat/super.c @@ -165,6 +165,8 @@ static int exfat_show_options(struct seq_file *m, struct dentry *root) seq_puts(m, ",sys_tz"); else if (opts->time_offset) seq_printf(m, ",time_offset=%d", opts->time_offset); + if (opts->zero_size_dir) + seq_puts(m, ",zero_size_dir"); return 0; } @@ -209,6 +211,7 @@ enum { Opt_keep_last_dots, Opt_sys_tz, Opt_time_offset, + Opt_zero_size_dir, /* Deprecated options */ Opt_utf8, @@ -237,6 +240,7 @@ static const struct fs_parameter_spec exfat_parameters[] = { fsparam_flag("keep_last_dots", Opt_keep_last_dots), fsparam_flag("sys_tz", Opt_sys_tz), fsparam_s32("time_offset", Opt_time_offset), + fsparam_flag("zero_size_dir", Opt_zero_size_dir), __fsparam(NULL, "utf8", Opt_utf8, fs_param_deprecated, NULL), __fsparam(NULL, "debug", Opt_debug, fs_param_deprecated, @@ -305,6 +309,9 @@ static int exfat_parse_param(struct fs_context *fc, struct fs_parameter *param) return -EINVAL; opts->time_offset = result.int_32; break; + case Opt_zero_size_dir: + opts->zero_size_dir = true; + break; case Opt_utf8: case Opt_debug: case Opt_namecase: From 1a0ac8bd7a4fa5b2f4ef14c3b1e9d6e5a5faae06 Mon Sep 17 00:00:00 2001 From: Gao Xiang Date: Tue, 31 Oct 2023 14:05:24 +0800 Subject: [PATCH 62/69] erofs: fix erofs_insert_workgroup() lockref usage As Linus pointed out [1], lockref_put_return() is fundamentally designed to be something that can fail. It behaves as a fastpath-only thing, and the failure case needs to be handled anyway. Actually, since the new pcluster was just allocated without being populated, it won't be accessed by others until it is inserted into XArray, so lockref helpers are actually unneeded here. Let's just set the proper reference count on initializing. [1] https://lore.kernel.org/r/CAHk-=whCga8BeQnJ3ZBh_Hfm9ctba_wpF444LpwRybVNMzO6Dw@mail.gmail.com Fixes: 7674a42f35ea ("erofs: use struct lockref to replace handcrafted approach") Reviewed-by: Chao Yu Link: https://lore.kernel.org/r/20231031060524.1103921-1-hsiangkao@linux.alibaba.com Signed-off-by: Gao Xiang --- fs/erofs/utils.c | 8 +------- fs/erofs/zdata.c | 1 + 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/fs/erofs/utils.c b/fs/erofs/utils.c index cc6fb9e98899..4256a85719a1 100644 --- a/fs/erofs/utils.c +++ b/fs/erofs/utils.c @@ -77,12 +77,7 @@ struct erofs_workgroup *erofs_insert_workgroup(struct super_block *sb, struct erofs_sb_info *const sbi = EROFS_SB(sb); struct erofs_workgroup *pre; - /* - * Bump up before making this visible to others for the XArray in order - * to avoid potential UAF without serialized by xa_lock. - */ - lockref_get(&grp->lockref); - + DBG_BUGON(grp->lockref.count < 1); repeat: xa_lock(&sbi->managed_pslots); pre = __xa_cmpxchg(&sbi->managed_pslots, grp->index, @@ -96,7 +91,6 @@ repeat: cond_resched(); goto repeat; } - lockref_put_return(&grp->lockref); grp = pre; } xa_unlock(&sbi->managed_pslots); diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c index 036f610e044b..a7e6847f6f8f 100644 --- a/fs/erofs/zdata.c +++ b/fs/erofs/zdata.c @@ -796,6 +796,7 @@ static int z_erofs_register_pcluster(struct z_erofs_decompress_frontend *fe) return PTR_ERR(pcl); spin_lock_init(&pcl->obj.lockref.lock); + pcl->obj.lockref.count = 1; /* one ref for this request */ pcl->algorithmformat = map->m_algorithmformat; pcl->length = 0; pcl->partial = true; From e044374a8a0a99e46f4e6d6751d3042b6d9cc12e Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Thu, 5 Oct 2023 14:15:58 +0300 Subject: [PATCH 63/69] ima: annotate iint mutex to avoid lockdep false positive warnings It is not clear that IMA should be nested at all, but as long is it measures files both on overlayfs and on underlying fs, we need to annotate the iint mutex to avoid lockdep false positives related to IMA + overlayfs, same as overlayfs annotates the inode mutex. Reported-and-tested-by: syzbot+b42fe626038981fb7bfa@syzkaller.appspotmail.com Signed-off-by: Amir Goldstein Cc: stable@vger.kernel.org Signed-off-by: Mimi Zohar --- security/integrity/iint.c | 48 ++++++++++++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/security/integrity/iint.c b/security/integrity/iint.c index a462df827de2..27ea19fb1f54 100644 --- a/security/integrity/iint.c +++ b/security/integrity/iint.c @@ -66,9 +66,32 @@ struct integrity_iint_cache *integrity_iint_find(struct inode *inode) return iint; } -static void iint_free(struct integrity_iint_cache *iint) +#define IMA_MAX_NESTING (FILESYSTEM_MAX_STACK_DEPTH+1) + +/* + * It is not clear that IMA should be nested at all, but as long is it measures + * files both on overlayfs and on underlying fs, we need to annotate the iint + * mutex to avoid lockdep false positives related to IMA + overlayfs. + * See ovl_lockdep_annotate_inode_mutex_key() for more details. + */ +static inline void iint_lockdep_annotate(struct integrity_iint_cache *iint, + struct inode *inode) +{ +#ifdef CONFIG_LOCKDEP + static struct lock_class_key iint_mutex_key[IMA_MAX_NESTING]; + + int depth = inode->i_sb->s_stack_depth; + + if (WARN_ON_ONCE(depth < 0 || depth >= IMA_MAX_NESTING)) + depth = 0; + + lockdep_set_class(&iint->mutex, &iint_mutex_key[depth]); +#endif +} + +static void iint_init_always(struct integrity_iint_cache *iint, + struct inode *inode) { - kfree(iint->ima_hash); iint->ima_hash = NULL; iint->version = 0; iint->flags = 0UL; @@ -80,6 +103,14 @@ static void iint_free(struct integrity_iint_cache *iint) iint->ima_creds_status = INTEGRITY_UNKNOWN; iint->evm_status = INTEGRITY_UNKNOWN; iint->measured_pcrs = 0; + mutex_init(&iint->mutex); + iint_lockdep_annotate(iint, inode); +} + +static void iint_free(struct integrity_iint_cache *iint) +{ + kfree(iint->ima_hash); + mutex_destroy(&iint->mutex); kmem_cache_free(iint_cache, iint); } @@ -104,6 +135,8 @@ struct integrity_iint_cache *integrity_inode_get(struct inode *inode) if (!iint) return NULL; + iint_init_always(iint, inode); + write_lock(&integrity_iint_lock); p = &integrity_iint_tree.rb_node; @@ -153,25 +186,18 @@ void integrity_inode_free(struct inode *inode) iint_free(iint); } -static void init_once(void *foo) +static void iint_init_once(void *foo) { struct integrity_iint_cache *iint = (struct integrity_iint_cache *) foo; memset(iint, 0, sizeof(*iint)); - iint->ima_file_status = INTEGRITY_UNKNOWN; - iint->ima_mmap_status = INTEGRITY_UNKNOWN; - iint->ima_bprm_status = INTEGRITY_UNKNOWN; - iint->ima_read_status = INTEGRITY_UNKNOWN; - iint->ima_creds_status = INTEGRITY_UNKNOWN; - iint->evm_status = INTEGRITY_UNKNOWN; - mutex_init(&iint->mutex); } static int __init integrity_iintcache_init(void) { iint_cache = kmem_cache_create("iint_cache", sizeof(struct integrity_iint_cache), - 0, SLAB_PANIC, init_once); + 0, SLAB_PANIC, iint_init_once); return 0; } DEFINE_LSM(integrity) = { From 7b5c3086d1f85448a2a81947b685119c6c9894c8 Mon Sep 17 00:00:00 2001 From: Prasad Pandit Date: Sun, 22 Oct 2023 12:17:23 +0530 Subject: [PATCH 64/69] integrity: fix indentation of config attributes Fix indentation of config attributes. Attributes are generally indented with a leading tab(\t) character. Signed-off-by: Prasad Pandit Signed-off-by: Mimi Zohar --- security/integrity/Kconfig | 44 +++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig index 232191ee09e3..1e151e6a5d3f 100644 --- a/security/integrity/Kconfig +++ b/security/integrity/Kconfig @@ -34,10 +34,10 @@ config INTEGRITY_ASYMMETRIC_KEYS bool "Enable asymmetric keys support" depends on INTEGRITY_SIGNATURE default n - select ASYMMETRIC_KEY_TYPE - select ASYMMETRIC_PUBLIC_KEY_SUBTYPE - select CRYPTO_RSA - select X509_CERTIFICATE_PARSER + select ASYMMETRIC_KEY_TYPE + select ASYMMETRIC_PUBLIC_KEY_SUBTYPE + select CRYPTO_RSA + select X509_CERTIFICATE_PARSER help This option enables digital signature verification using asymmetric keys. @@ -53,14 +53,14 @@ config INTEGRITY_TRUSTED_KEYRING keyring. config INTEGRITY_PLATFORM_KEYRING - bool "Provide keyring for platform/firmware trusted keys" - depends on INTEGRITY_ASYMMETRIC_KEYS - depends on SYSTEM_BLACKLIST_KEYRING - help - Provide a separate, distinct keyring for platform trusted keys, which - the kernel automatically populates during initialization from values - provided by the platform for verifying the kexec'ed kerned image - and, possibly, the initramfs signature. + bool "Provide keyring for platform/firmware trusted keys" + depends on INTEGRITY_ASYMMETRIC_KEYS + depends on SYSTEM_BLACKLIST_KEYRING + help + Provide a separate, distinct keyring for platform trusted keys, which + the kernel automatically populates during initialization from values + provided by the platform for verifying the kexec'ed kerned image + and, possibly, the initramfs signature. config INTEGRITY_MACHINE_KEYRING bool "Provide a keyring to which Machine Owner Keys may be added" @@ -71,10 +71,10 @@ config INTEGRITY_MACHINE_KEYRING select INTEGRITY_CA_MACHINE_KEYRING if LOAD_PPC_KEYS select INTEGRITY_CA_MACHINE_KEYRING_MAX if LOAD_PPC_KEYS help - If set, provide a keyring to which Machine Owner Keys (MOK) may - be added. This keyring shall contain just MOK keys. Unlike keys - in the platform keyring, keys contained in the .machine keyring will - be trusted within the kernel. + If set, provide a keyring to which Machine Owner Keys (MOK) may + be added. This keyring shall contain just MOK keys. Unlike keys + in the platform keyring, keys contained in the .machine keyring will + be trusted within the kernel. config INTEGRITY_CA_MACHINE_KEYRING bool "Enforce Machine Keyring CA Restrictions" @@ -99,14 +99,14 @@ config INTEGRITY_CA_MACHINE_KEYRING_MAX .platform keyring. config LOAD_UEFI_KEYS - depends on INTEGRITY_PLATFORM_KEYRING - depends on EFI - def_bool y + depends on INTEGRITY_PLATFORM_KEYRING + depends on EFI + def_bool y config LOAD_IPL_KEYS - depends on INTEGRITY_PLATFORM_KEYRING - depends on S390 - def_bool y + depends on INTEGRITY_PLATFORM_KEYRING + depends on S390 + def_bool y config LOAD_PPC_KEYS bool "Enable loading of platform and blacklisted keys for POWER" From b46503068cb9ed63ff1d8250f143354ead0b16eb Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Sun, 15 Oct 2023 20:18:03 -0400 Subject: [PATCH 65/69] certs: Only allow certs signed by keys on the builtin keyring Originally the secondary trusted keyring provided a keyring to which extra keys may be added, provided those keys were not blacklisted and were vouched for by a key built into the kernel or already in the secondary trusted keyring. On systems with the machine keyring configured, additional keys may also be vouched for by a key on the machine keyring. Prevent loading additional certificates directly onto the secondary keyring, vouched for by keys on the machine keyring, yet allow these certificates to be loaded onto other trusted keyrings. Reviewed-by: Jarkko Sakkinen Signed-off-by: Mimi Zohar --- certs/Kconfig | 16 +++++++++++++++- crypto/asymmetric_keys/restrict.c | 4 ++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/certs/Kconfig b/certs/Kconfig index 1f109b070877..62036974367c 100644 --- a/certs/Kconfig +++ b/certs/Kconfig @@ -88,7 +88,21 @@ config SECONDARY_TRUSTED_KEYRING help If set, provide a keyring to which extra keys may be added, provided those keys are not blacklisted and are vouched for by a key built - into the kernel or already in the secondary trusted keyring. + into the kernel, machine keyring (if configured), or already in the + secondary trusted keyring. + +config SECONDARY_TRUSTED_KEYRING_SIGNED_BY_BUILTIN + bool "Only allow additional certs signed by keys on the builtin trusted keyring" + depends on SECONDARY_TRUSTED_KEYRING + help + If set, only certificates signed by keys on the builtin trusted + keyring may be loaded onto the secondary trusted keyring. + + Note: The machine keyring, if configured, will be linked to the + secondary keyring. When enabling this option, it is recommended + to also configure INTEGRITY_CA_MACHINE_KEYRING_MAX to prevent + linking code signing keys with imputed trust to the secondary + trusted keyring. config SYSTEM_BLACKLIST_KEYRING bool "Provide system-wide ring of blacklisted keys" diff --git a/crypto/asymmetric_keys/restrict.c b/crypto/asymmetric_keys/restrict.c index 6b69ea40da23..afcd4d101ac5 100644 --- a/crypto/asymmetric_keys/restrict.c +++ b/crypto/asymmetric_keys/restrict.c @@ -102,6 +102,10 @@ int restrict_link_by_signature(struct key *dest_keyring, if (use_builtin_keys && !test_bit(KEY_FLAG_BUILTIN, &key->flags)) ret = -ENOKEY; + else if (IS_BUILTIN(CONFIG_SECONDARY_TRUSTED_KEYRING_SIGNED_BY_BUILTIN) && + !strcmp(dest_keyring->description, ".secondary_trusted_keys") && + !test_bit(KEY_FLAG_BUILTIN, &key->flags)) + ret = -ENOKEY; else ret = verify_signature(key, sig); key_put(key); From b836c4d29f2744200b2af41e14bf50758dddc818 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Wed, 18 Oct 2023 14:47:02 -0400 Subject: [PATCH 66/69] ima: detect changes to the backing overlay file Commit 18b44bc5a672 ("ovl: Always reevaluate the file signature for IMA") forced signature re-evaulation on every file access. Instead of always re-evaluating the file's integrity, detect a change to the backing file, by comparing the cached file metadata with the backing file's metadata. Verifying just the i_version has not changed is insufficient. In addition save and compare the i_ino and s_dev as well. Reviewed-by: Amir Goldstein Tested-by: Eric Snowberg Tested-by: Raul E Rangel Cc: stable@vger.kernel.org Signed-off-by: Mimi Zohar --- fs/overlayfs/super.c | 2 +- security/integrity/ima/ima_api.c | 5 +++++ security/integrity/ima/ima_main.c | 16 +++++++++++++++- security/integrity/integrity.h | 2 ++ 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 3fa2416264a4..c71d185980c0 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -1489,7 +1489,7 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc) ovl_trusted_xattr_handlers; sb->s_fs_info = ofs; sb->s_flags |= SB_POSIXACL; - sb->s_iflags |= SB_I_SKIP_SYNC | SB_I_IMA_UNVERIFIABLE_SIGNATURE; + sb->s_iflags |= SB_I_SKIP_SYNC; err = -ENOMEM; root_dentry = ovl_get_root(sb, ctx->upper.dentry, oe); diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index 452e80b541e5..597ea0c4d72f 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -243,6 +243,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint, { const char *audit_cause = "failed"; struct inode *inode = file_inode(file); + struct inode *real_inode = d_real_inode(file_dentry(file)); const char *filename = file->f_path.dentry->d_name.name; struct ima_max_digest_data hash; struct kstat stat; @@ -302,6 +303,10 @@ int ima_collect_measurement(struct integrity_iint_cache *iint, iint->ima_hash = tmpbuf; memcpy(iint->ima_hash, &hash, length); iint->version = i_version; + if (real_inode != inode) { + iint->real_ino = real_inode->i_ino; + iint->real_dev = real_inode->i_sb->s_dev; + } /* Possibly temporary failure due to type of read (eg. O_DIRECT) */ if (!result) diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 365db0e43d7c..cc1217ac2c6f 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -25,6 +25,7 @@ #include #include #include +#include #include "ima.h" @@ -207,7 +208,7 @@ static int process_measurement(struct file *file, const struct cred *cred, u32 secid, char *buf, loff_t size, int mask, enum ima_hooks func) { - struct inode *inode = file_inode(file); + struct inode *backing_inode, *inode = file_inode(file); struct integrity_iint_cache *iint = NULL; struct ima_template_desc *template_desc = NULL; char *pathbuf = NULL; @@ -284,6 +285,19 @@ static int process_measurement(struct file *file, const struct cred *cred, iint->measured_pcrs = 0; } + /* Detect and re-evaluate changes made to the backing file. */ + backing_inode = d_real_inode(file_dentry(file)); + if (backing_inode != inode && + (action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) { + if (!IS_I_VERSION(backing_inode) || + backing_inode->i_sb->s_dev != iint->real_dev || + backing_inode->i_ino != iint->real_ino || + !inode_eq_iversion(backing_inode, iint->version)) { + iint->flags &= ~IMA_DONE_MASK; + iint->measured_pcrs = 0; + } + } + /* Determine if already appraised/measured based on bitmask * (IMA_MEASURE, IMA_MEASURED, IMA_XXXX_APPRAISE, IMA_XXXX_APPRAISED, * IMA_AUDIT, IMA_AUDITED) diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index d7553c93f5c0..9561db7cf6b4 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -164,6 +164,8 @@ struct integrity_iint_cache { unsigned long flags; unsigned long measured_pcrs; unsigned long atomic_flags; + unsigned long real_ino; + dev_t real_dev; enum integrity_status ima_file_status:4; enum integrity_status ima_mmap_status:4; enum integrity_status ima_bprm_status:4; From ce56d21355cd6f6937aca32f1f44ca749d1e4808 Mon Sep 17 00:00:00 2001 From: Brian Foster Date: Mon, 2 Oct 2023 14:50:20 -0400 Subject: [PATCH 67/69] ext4: fix racy may inline data check in dio write syzbot reports that the following warning from ext4_iomap_begin() triggers as of the commit referenced below: if (WARN_ON_ONCE(ext4_has_inline_data(inode))) return -ERANGE; This occurs during a dio write, which is never expected to encounter an inode with inline data. To enforce this behavior, ext4_dio_write_iter() checks the current inline state of the inode and clears the MAY_INLINE_DATA state flag to either fall back to buffered writes, or enforce that any other writers in progress on the inode are not allowed to create inline data. The problem is that the check for existing inline data and the state flag can span a lock cycle. For example, if the ilock is originally locked shared and subsequently upgraded to exclusive, another writer may have reacquired the lock and created inline data before the dio write task acquires the lock and proceeds. The commit referenced below loosens the lock requirements to allow some forms of unaligned dio writes to occur under shared lock, but AFAICT the inline data check was technically already racy for any dio write that would have involved a lock cycle. Regardless, lift clearing of the state bit to the same lock critical section that checks for preexisting inline data on the inode to close the race. Cc: stable@kernel.org Reported-by: syzbot+307da6ca5cb0d01d581a@syzkaller.appspotmail.com Fixes: 310ee0902b8d ("ext4: allow concurrent unaligned dio overwrites") Signed-off-by: Brian Foster Link: https://lore.kernel.org/r/20231002185020.531537-1-bfoster@redhat.com Signed-off-by: Theodore Ts'o --- fs/ext4/file.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/fs/ext4/file.c b/fs/ext4/file.c index 6830ea3a6c59..747c0378122d 100644 --- a/fs/ext4/file.c +++ b/fs/ext4/file.c @@ -569,18 +569,20 @@ static ssize_t ext4_dio_write_iter(struct kiocb *iocb, struct iov_iter *from) return ext4_buffered_write_iter(iocb, from); } + /* + * Prevent inline data from being created since we are going to allocate + * blocks for DIO. We know the inode does not currently have inline data + * because ext4_should_use_dio() checked for it, but we have to clear + * the state flag before the write checks because a lock cycle could + * introduce races with other writers. + */ + ext4_clear_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA); + ret = ext4_dio_write_checks(iocb, from, &ilock_shared, &extend, &unwritten, &dio_flags); if (ret <= 0) return ret; - /* - * Make sure inline data cannot be created anymore since we are going - * to allocate blocks for DIO. We know the inode does not have any - * inline data now because ext4_dio_supported() checked for that. - */ - ext4_clear_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA); - offset = iocb->ki_pos; count = ret; From 91562895f8030cb9a0470b1db49de79346a69f91 Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Fri, 13 Oct 2023 14:13:50 +0200 Subject: [PATCH 68/69] ext4: properly sync file size update after O_SYNC direct IO Gao Xiang has reported that on ext4 O_SYNC direct IO does not properly sync file size update and thus if we crash at unfortunate moment, the file can have smaller size although O_SYNC IO has reported successful completion. The problem happens because update of on-disk inode size is handled in ext4_dio_write_iter() *after* iomap_dio_rw() (and thus dio_complete() in particular) has returned and generic_file_sync() gets called by dio_complete(). Fix the problem by handling on-disk inode size update directly in our ->end_io completion handler. References: https://lore.kernel.org/all/02d18236-26ef-09b0-90ad-030c4fe3ee20@linux.alibaba.com Reported-by: Gao Xiang CC: stable@vger.kernel.org Fixes: 378f32bab371 ("ext4: introduce direct I/O write using iomap infrastructure") Signed-off-by: Jan Kara Tested-by: Joseph Qi Reviewed-by: "Ritesh Harjani (IBM)" Link: https://lore.kernel.org/r/20231013121350.26872-1-jack@suse.cz Signed-off-by: Theodore Ts'o --- fs/ext4/file.c | 153 +++++++++++++++++++++---------------------------- 1 file changed, 65 insertions(+), 88 deletions(-) diff --git a/fs/ext4/file.c b/fs/ext4/file.c index 747c0378122d..0166bb9ca160 100644 --- a/fs/ext4/file.c +++ b/fs/ext4/file.c @@ -306,80 +306,38 @@ out: } static ssize_t ext4_handle_inode_extension(struct inode *inode, loff_t offset, - ssize_t written, size_t count) + ssize_t count) { handle_t *handle; - bool truncate = false; - u8 blkbits = inode->i_blkbits; - ext4_lblk_t written_blk, end_blk; - int ret; - - /* - * Note that EXT4_I(inode)->i_disksize can get extended up to - * inode->i_size while the I/O was running due to writeback of delalloc - * blocks. But, the code in ext4_iomap_alloc() is careful to use - * zeroed/unwritten extents if this is possible; thus we won't leave - * uninitialized blocks in a file even if we didn't succeed in writing - * as much as we intended. - */ - WARN_ON_ONCE(i_size_read(inode) < EXT4_I(inode)->i_disksize); - if (offset + count <= EXT4_I(inode)->i_disksize) { - /* - * We need to ensure that the inode is removed from the orphan - * list if it has been added prematurely, due to writeback of - * delalloc blocks. - */ - if (!list_empty(&EXT4_I(inode)->i_orphan) && inode->i_nlink) { - handle = ext4_journal_start(inode, EXT4_HT_INODE, 2); - - if (IS_ERR(handle)) { - ext4_orphan_del(NULL, inode); - return PTR_ERR(handle); - } - - ext4_orphan_del(handle, inode); - ext4_journal_stop(handle); - } - - return written; - } - - if (written < 0) - goto truncate; + lockdep_assert_held_write(&inode->i_rwsem); handle = ext4_journal_start(inode, EXT4_HT_INODE, 2); - if (IS_ERR(handle)) { - written = PTR_ERR(handle); - goto truncate; - } + if (IS_ERR(handle)) + return PTR_ERR(handle); - if (ext4_update_inode_size(inode, offset + written)) { - ret = ext4_mark_inode_dirty(handle, inode); + if (ext4_update_inode_size(inode, offset + count)) { + int ret = ext4_mark_inode_dirty(handle, inode); if (unlikely(ret)) { - written = ret; ext4_journal_stop(handle); - goto truncate; + return ret; } } - /* - * We may need to truncate allocated but not written blocks beyond EOF. - */ - written_blk = ALIGN(offset + written, 1 << blkbits); - end_blk = ALIGN(offset + count, 1 << blkbits); - if (written_blk < end_blk && ext4_can_truncate(inode)) - truncate = true; - - /* - * Remove the inode from the orphan list if it has been extended and - * everything went OK. - */ - if (!truncate && inode->i_nlink) + if (inode->i_nlink) ext4_orphan_del(handle, inode); ext4_journal_stop(handle); - if (truncate) { -truncate: + return count; +} + +/* + * Clean up the inode after DIO or DAX extending write has completed and the + * inode size has been updated using ext4_handle_inode_extension(). + */ +static void ext4_inode_extension_cleanup(struct inode *inode, ssize_t count) +{ + lockdep_assert_held_write(&inode->i_rwsem); + if (count < 0) { ext4_truncate_failed_write(inode); /* * If the truncate operation failed early, then the inode may @@ -388,9 +346,28 @@ truncate: */ if (inode->i_nlink) ext4_orphan_del(NULL, inode); + return; } + /* + * If i_disksize got extended due to writeback of delalloc blocks while + * the DIO was running we could fail to cleanup the orphan list in + * ext4_handle_inode_extension(). Do it now. + */ + if (!list_empty(&EXT4_I(inode)->i_orphan) && inode->i_nlink) { + handle_t *handle = ext4_journal_start(inode, EXT4_HT_INODE, 2); - return written; + if (IS_ERR(handle)) { + /* + * The write has successfully completed. Not much to + * do with the error here so just cleanup the orphan + * list and hope for the best. + */ + ext4_orphan_del(NULL, inode); + return; + } + ext4_orphan_del(handle, inode); + ext4_journal_stop(handle); + } } static int ext4_dio_write_end_io(struct kiocb *iocb, ssize_t size, @@ -399,31 +376,22 @@ static int ext4_dio_write_end_io(struct kiocb *iocb, ssize_t size, loff_t pos = iocb->ki_pos; struct inode *inode = file_inode(iocb->ki_filp); + if (!error && size && flags & IOMAP_DIO_UNWRITTEN) + error = ext4_convert_unwritten_extents(NULL, inode, pos, size); if (error) return error; - - if (size && flags & IOMAP_DIO_UNWRITTEN) { - error = ext4_convert_unwritten_extents(NULL, inode, pos, size); - if (error < 0) - return error; - } /* - * If we are extending the file, we have to update i_size here before - * page cache gets invalidated in iomap_dio_rw(). Otherwise racing - * buffered reads could zero out too much from page cache pages. Update - * of on-disk size will happen later in ext4_dio_write_iter() where - * we have enough information to also perform orphan list handling etc. - * Note that we perform all extending writes synchronously under - * i_rwsem held exclusively so i_size update is safe here in that case. - * If the write was not extending, we cannot see pos > i_size here - * because operations reducing i_size like truncate wait for all - * outstanding DIO before updating i_size. + * Note that EXT4_I(inode)->i_disksize can get extended up to + * inode->i_size while the I/O was running due to writeback of delalloc + * blocks. But the code in ext4_iomap_alloc() is careful to use + * zeroed/unwritten extents if this is possible; thus we won't leave + * uninitialized blocks in a file even if we didn't succeed in writing + * as much as we intended. */ - pos += size; - if (pos > i_size_read(inode)) - i_size_write(inode, pos); - - return 0; + WARN_ON_ONCE(i_size_read(inode) < READ_ONCE(EXT4_I(inode)->i_disksize)); + if (pos + size <= READ_ONCE(EXT4_I(inode)->i_disksize)) + return size; + return ext4_handle_inode_extension(inode, pos, size); } static const struct iomap_dio_ops ext4_dio_write_ops = { @@ -608,9 +576,16 @@ static ssize_t ext4_dio_write_iter(struct kiocb *iocb, struct iov_iter *from) dio_flags, NULL, 0); if (ret == -ENOTBLK) ret = 0; - - if (extend) - ret = ext4_handle_inode_extension(inode, offset, ret, count); + if (extend) { + /* + * We always perform extending DIO write synchronously so by + * now the IO is completed and ext4_handle_inode_extension() + * was called. Cleanup the inode in case of error or race with + * writeback of delalloc blocks. + */ + WARN_ON_ONCE(ret == -EIOCBQUEUED); + ext4_inode_extension_cleanup(inode, ret); + } out: if (ilock_shared) @@ -691,8 +666,10 @@ ext4_dax_write_iter(struct kiocb *iocb, struct iov_iter *from) ret = dax_iomap_rw(iocb, from, &ext4_iomap_ops); - if (extend) - ret = ext4_handle_inode_extension(inode, offset, ret, count); + if (extend) { + ret = ext4_handle_inode_extension(inode, offset, ret); + ext4_inode_extension_cleanup(inode, ret); + } out: inode_unlock(inode); if (ret > 0) From 17fc8084aa8f9d5235f252fc3978db657dd77e92 Mon Sep 17 00:00:00 2001 From: Andrea Righi Date: Thu, 2 Nov 2023 09:19:14 +0100 Subject: [PATCH 69/69] module/decompress: use kvmalloc() consistently We consistently switched from kmalloc() to vmalloc() in module decompression to prevent potential memory allocation failures with large modules, however vmalloc() is not as memory-efficient and fast as kmalloc(). Since we don't know in general the size of the workspace required by the decompression algorithm, it is more reasonable to use kvmalloc() consistently, also considering that we don't have special memory requirements here. Suggested-by: Linus Torvalds Tested-by: Andrea Righi Signed-off-by: Andrea Righi Signed-off-by: Linus Torvalds --- kernel/module/decompress.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/kernel/module/decompress.c b/kernel/module/decompress.c index 4156d59be440..474e68f0f063 100644 --- a/kernel/module/decompress.c +++ b/kernel/module/decompress.c @@ -100,7 +100,7 @@ static ssize_t module_gzip_decompress(struct load_info *info, s.next_in = buf + gzip_hdr_len; s.avail_in = size - gzip_hdr_len; - s.workspace = vmalloc(zlib_inflate_workspacesize()); + s.workspace = kvmalloc(zlib_inflate_workspacesize(), GFP_KERNEL); if (!s.workspace) return -ENOMEM; @@ -138,7 +138,7 @@ static ssize_t module_gzip_decompress(struct load_info *info, out_inflate_end: zlib_inflateEnd(&s); out: - vfree(s.workspace); + kvfree(s.workspace); return retval; } #elif defined(CONFIG_MODULE_COMPRESS_XZ) @@ -241,7 +241,7 @@ static ssize_t module_zstd_decompress(struct load_info *info, } wksp_size = zstd_dstream_workspace_bound(header.windowSize); - wksp = vmalloc(wksp_size); + wksp = kvmalloc(wksp_size, GFP_KERNEL); if (!wksp) { retval = -ENOMEM; goto out; @@ -284,7 +284,7 @@ static ssize_t module_zstd_decompress(struct load_info *info, retval = new_size; out: - vfree(wksp); + kvfree(wksp); return retval; } #else