From 4bdba812279bb1fd64450cf575f58f3546aa7e9e Mon Sep 17 00:00:00 2001 From: Ryota Sakamoto Date: Tue, 27 Jan 2026 23:23:23 +0900 Subject: [PATCH 01/20] ext4: replace KUnit tests for memcmp() with KUNIT_ASSERT_MEMEQ() Replace KUnit tests for memcmp() with KUNIT_ASSERT_MEMEQ() to improve debugging that prints the hex dump of the buffers when the assertion fails, whereas memcmp() only returns an integer difference. Signed-off-by: Ryota Sakamoto Link: https://patch.msgid.link/20260127-fix-fs_ext4-memcmp-v1-1-5c269ae906b6@gmail.com Signed-off-by: Theodore Ts'o --- fs/ext4/mballoc-test.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/fs/ext4/mballoc-test.c b/fs/ext4/mballoc-test.c index 90ed505fa4b1..3bd8ef087b28 100644 --- a/fs/ext4/mballoc-test.c +++ b/fs/ext4/mballoc-test.c @@ -714,8 +714,7 @@ do_test_generate_buddy(struct kunit *test, struct super_block *sb, void *bitmap, ext4_mb_generate_buddy_test(sb, ext4_buddy, bitmap, TEST_GOAL_GROUP, ext4_grp); - KUNIT_ASSERT_EQ(test, memcmp(mbt_buddy, ext4_buddy, sb->s_blocksize), - 0); + KUNIT_ASSERT_MEMEQ(test, mbt_buddy, ext4_buddy, sb->s_blocksize); mbt_validate_group_info(test, mbt_grp, ext4_grp); } @@ -776,8 +775,7 @@ test_mb_mark_used_range(struct kunit *test, struct ext4_buddy *e4b, grp->bb_counters[i] = 0; ext4_mb_generate_buddy_test(sb, buddy, bitmap, 0, grp); - KUNIT_ASSERT_EQ(test, memcmp(buddy, e4b->bd_buddy, sb->s_blocksize), - 0); + KUNIT_ASSERT_MEMEQ(test, buddy, e4b->bd_buddy, sb->s_blocksize); mbt_validate_group_info(test, grp, e4b->bd_info); } @@ -841,8 +839,7 @@ test_mb_free_blocks_range(struct kunit *test, struct ext4_buddy *e4b, grp->bb_counters[i] = 0; ext4_mb_generate_buddy_test(sb, buddy, bitmap, 0, grp); - KUNIT_ASSERT_EQ(test, memcmp(buddy, e4b->bd_buddy, sb->s_blocksize), - 0); + KUNIT_ASSERT_MEMEQ(test, buddy, e4b->bd_buddy, sb->s_blocksize); mbt_validate_group_info(test, grp, e4b->bd_info); } From d99748ef1695ce17eaf51c64b7a06952fa7cddab Mon Sep 17 00:00:00 2001 From: Zhang Yi Date: Fri, 24 Apr 2026 18:42:01 +0800 Subject: [PATCH 02/20] ext4: fix LOGFLUSH shutdown ordering to allow ordered-mode data writeback In EXT4_GOING_FLAGS_LOGFLUSH mode, the EXT4_FLAGS_SHUTDOWN flag was set before calling ext4_force_commit(). This caused ordered-mode data writeback (triggered by journal commit) to fail with -EIO, since ext4_do_writepages() checks for the shutdown flag. The journal would then be aborted prematurely before the commit could succeed. Fix this by calling ext4_force_commit() first, then setting the shutdown flag, so that pending data can be written back correctly. Note that moving ext4_force_commit() before setting the shutdown flag creates a small window in which new writes may occur and generate new journal transactions. When the journal is subsequently aborted, the new transactions will not be able to write to disk. This is intentional because LOGFLUSH's semantics are to flush pre-existing journal entries before shutdown, not to guarantee atomicity for writes that race with the ioctl. Fixes: 783d94854499 ("ext4: add EXT4_IOC_GOINGDOWN ioctl") Signed-off-by: Zhang Yi Reviewed-by: Baokun Li Reviewed-by: Jan Kara Link: https://patch.msgid.link/20260424104201.1930823-1-yi.zhang@huaweicloud.com Signed-off-by: Theodore Ts'o --- fs/ext4/ioctl.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c index 1d0c3d4bdf47..110e3fb194ec 100644 --- a/fs/ext4/ioctl.c +++ b/fs/ext4/ioctl.c @@ -830,11 +830,17 @@ int ext4_force_shutdown(struct super_block *sb, u32 flags) bdev_thaw(sb->s_bdev); break; case EXT4_GOING_FLAGS_LOGFLUSH: + /* + * Call ext4_force_commit() before setting EXT4_FLAGS_SHUTDOWN. + * This is because in data=ordered mode, journal commit + * triggers data writeback which fails if shutdown is already + * set, causing the journal to be aborted prematurely before + * the commit succeeds. + */ + (void) ext4_force_commit(sb); set_bit(EXT4_FLAGS_SHUTDOWN, &sbi->s_ext4_flags); - if (sbi->s_journal && !is_journal_aborted(sbi->s_journal)) { - (void) ext4_force_commit(sb); + if (sbi->s_journal && !is_journal_aborted(sbi->s_journal)) jbd2_journal_abort(sbi->s_journal, -ESHUTDOWN); - } break; case EXT4_GOING_FLAGS_NOLOGFLUSH: set_bit(EXT4_FLAGS_SHUTDOWN, &sbi->s_ext4_flags); From 3fb803cdfbb1a2c31a77b1c0e6b6e2022f29af97 Mon Sep 17 00:00:00 2001 From: Abdellah Ouhbi Date: Fri, 24 Apr 2026 16:43:07 +0100 Subject: [PATCH 03/20] ext4: Use %pe to print PTR_ERR() Replace %ld with %pe and PTR_ERR(path) with path pointer. The %pe specifier automatically converts error pointers to human-readable error names instead of raw error codes. These changes were found by coccicheck. Signed-off-by: Abdellah Ouhbi Link: https://patch.msgid.link/20260424154307.169881-1-abdououhbi1@gmail.com Link: https://patch.msgid.link/20260424155508.186235-1-abdououhbi1@gmail.com Link: https://patch.msgid.link/20260424152245.142308-1-abdououhbi1@gmail.com Signed-off-by: Theodore Ts'o --- fs/ext4/extents.c | 4 ++-- fs/ext4/namei.c | 4 ++-- fs/ext4/super.c | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index 125f628e738a..91c97af64b31 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -3268,8 +3268,8 @@ static struct ext4_ext_path *ext4_split_extent_at(handle_t *handle, */ path = ext4_find_extent(inode, ee_block, NULL, flags | EXT4_EX_NOFAIL); if (IS_ERR(path)) { - EXT4_ERROR_INODE(inode, "Failed split extent on %u, err %ld", - split, PTR_ERR(path)); + EXT4_ERROR_INODE(inode, "Failed split extent on %u, err %pe", + split, path); goto out_path; } diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c index 4a47fbd8dd30..c0cabf172020 100644 --- a/fs/ext4/namei.c +++ b/fs/ext4/namei.c @@ -145,9 +145,9 @@ static struct buffer_head *__ext4_read_dirblock(struct inode *inode, if (IS_ERR(bh)) { __ext4_warning(inode->i_sb, func, line, "inode #%llu: lblock %lu: comm %s: " - "error %ld reading directory block", + "error %pe reading directory block", inode->i_ino, (unsigned long)block, - current->comm, PTR_ERR(bh)); + current->comm, bh); return bh; } diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 6a77db4d3124..4b69e0879731 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -5977,8 +5977,8 @@ static struct file *ext4_get_journal_blkdev(struct super_block *sb, sb, &fs_holder_ops); if (IS_ERR(bdev_file)) { ext4_msg(sb, KERN_ERR, - "failed to open journal device unknown-block(%u,%u) %ld", - MAJOR(j_dev), MINOR(j_dev), PTR_ERR(bdev_file)); + "failed to open journal device unknown-block(%u,%u) %pe", + MAJOR(j_dev), MINOR(j_dev), bdev_file); return bdev_file; } From 8fc197cf366beaabaeb46575c8cf46fe5076b943 Mon Sep 17 00:00:00 2001 From: Deepanshu Kartikey Date: Thu, 7 May 2026 10:36:05 +0530 Subject: [PATCH 04/20] jbd2: check for aborted handle in jbd2_journal_dirty_metadata() jbd2_journal_dirty_metadata() unconditionally dereferences handle->h_transaction at function entry to obtain the journal pointer: transaction_t *transaction = handle->h_transaction; journal_t *journal = transaction->t_journal; However, h_transaction may legitimately be NULL for an aborted handle. The is_handle_aborted() helper in include/linux/jbd2.h explicitly treats !h_transaction as one of the aborted states: if (handle->h_aborted || !handle->h_transaction) return 1; Every other entry point in fs/jbd2/transaction.c (jbd2_journal_get_{write,undo,create}_access, jbd2_journal_extend, jbd2_journal_restart, jbd2_journal_stop, etc.) guards against this with an is_handle_aborted() check before any dereference of h_transaction. jbd2_journal_dirty_metadata() was missing this guard. This is reachable from ocfs2's xattr code. ocfs2_xa_set() intentionally falls through to ocfs2_xa_journal_dirty() even after ocfs2_xa_prepare_entry() fails, on the assumption that the buffer needs to be journaled to record any partial modifications (see the comment above the out_dirty label in fs/ocfs2/xattr.c). If the failure was caused by the journal being aborted -- e.g. an underlying I/O error during a sub-operation such as __ocfs2_remove_xattr_range() -- the handle's h_transaction has been cleared by the abort path, and the unconditional deref in jbd2_journal_dirty_metadata() becomes a NULL deref. Reproduced by syzbot with a crafted ocfs2 image where I/O against the loop device backing the mount is sabotaged via LOOP_SET_STATUS64 between two setxattr() calls, causing the second setxattr (which truncates an external xattr value) to abort the journal mid-flight: Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000 KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] RIP: jbd2_journal_dirty_metadata+0x4a/0xd30 fs/jbd2/transaction.c:1520 Call Trace: ocfs2_journal_dirty+0x130/0x700 fs/ocfs2/journal.c:831 ocfs2_xa_journal_dirty fs/ocfs2/xattr.c:1483 [inline] ocfs2_xa_set+0x15e3/0x2ec0 fs/ocfs2/xattr.c:2294 ocfs2_xattr_block_set+0x3e0/0x33c0 fs/ocfs2/xattr.c:3016 __ocfs2_xattr_set_handle+0x6b3/0xf50 fs/ocfs2/xattr.c:3418 ocfs2_xattr_set+0xf3f/0x13e0 fs/ocfs2/xattr.c:3681 __vfs_setxattr+0x43c/0x480 fs/xattr.c:218 ... Fix by adding the standard is_handle_aborted() guard at the top of jbd2_journal_dirty_metadata() and returning -EROFS, matching the pattern used by every other entry point in this file. ocfs2_journal_dirty() already handles a non-zero return from jbd2_journal_dirty_metadata() correctly. Reported-by: syzbot+98f651460e558a21baae@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=98f651460e558a21baae Tested-by: syzbot+98f651460e558a21baae@syzkaller.appspotmail.com Signed-off-by: Deepanshu Kartikey Reviewed-by: Zhang Yi Reviewed-by: Jan Kara Reviewed-by: Andreas Dilger Link: https://patch.msgid.link/20260507050605.50081-1-kartikey406@gmail.com Signed-off-by: Theodore Ts'o --- fs/jbd2/transaction.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c index 4885903bbd10..aa0be9e9c876 100644 --- a/fs/jbd2/transaction.c +++ b/fs/jbd2/transaction.c @@ -1516,14 +1516,19 @@ void jbd2_buffer_abort_trigger(struct journal_head *jh, */ int jbd2_journal_dirty_metadata(handle_t *handle, struct buffer_head *bh) { - transaction_t *transaction = handle->h_transaction; - journal_t *journal = transaction->t_journal; + transaction_t *transaction; + journal_t *journal; struct journal_head *jh; int ret = 0; + if (is_handle_aborted(handle)) + return -EROFS; if (!buffer_jbd(bh)) return -EUCLEAN; + transaction = handle->h_transaction; + journal = transaction->t_journal; + /* * We don't grab jh reference here since the buffer must be part * of the running transaction. From 8b3bc93fee6771775243665a0cf31857d6659775 Mon Sep 17 00:00:00 2001 From: Li Chen Date: Wed, 13 May 2026 16:58:17 +0800 Subject: [PATCH 05/20] ext4: fix fast commit wait/wake bit mapping on 64-bit On 64-bit, ext4 dynamic inode states live in the upper half of i_flags, and ext4_test_inode_state() applies the corresponding +32 offset. The fast-commit wait and wake paths open-coded the wait key with the raw EXT4_STATE_* value. Add small helpers for the state wait word and bit, and use them for the FC_COMMITTING and FC_FLUSHING_DATA waits so the wait key follows the same mapping as the state helpers. Fixes: 857d32f26181 ("ext4: rework fast commit commit path") Reported-by: Sashiko AI review Signed-off-by: Li Chen Reviewed-by: Baokun Li Reviewed-by: Zhang Yi Reviewed-by: Jan Kara Link: https://patch.msgid.link/20260513085818.552432-1-me@linux.beauty Signed-off-by: Theodore Ts'o --- fs/ext4/ext4.h | 20 +++++++++++++++++ fs/ext4/fast_commit.c | 50 ++++++++++++++++--------------------------- 2 files changed, 38 insertions(+), 32 deletions(-) diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index 94283a991e5c..6569d1d575a0 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -2000,6 +2000,8 @@ EXT4_INODE_BIT_FNS(flag, flags, 0) static inline int ext4_test_inode_state(struct inode *inode, int bit); static inline void ext4_set_inode_state(struct inode *inode, int bit); static inline void ext4_clear_inode_state(struct inode *inode, int bit); +static inline unsigned long *ext4_inode_state_wait_word(struct inode *inode); +static inline int ext4_inode_state_wait_bit(int bit); #if (BITS_PER_LONG < 64) EXT4_INODE_BIT_FNS(state, state_flags, 0) @@ -2015,6 +2017,24 @@ static inline void ext4_clear_state_flags(struct ext4_inode_info *ei) /* We depend on the fact that callers will set i_flags */ } #endif + +static inline unsigned long *ext4_inode_state_wait_word(struct inode *inode) +{ +#if (BITS_PER_LONG < 64) + return &EXT4_I(inode)->i_state_flags; +#else + return &EXT4_I(inode)->i_flags; +#endif +} + +static inline int ext4_inode_state_wait_bit(int bit) +{ +#if (BITS_PER_LONG < 64) + return bit; +#else + return bit + 32; +#endif +} #else /* Assume that user mode programs are passing in an ext4fs superblock, not * a kernel struct super_block. This will allow us to call the feature-test diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c index b3c22636251d..1775bce9649a 100644 --- a/fs/ext4/fast_commit.c +++ b/fs/ext4/fast_commit.c @@ -239,6 +239,8 @@ void ext4_fc_del(struct inode *inode) struct ext4_inode_info *ei = EXT4_I(inode); struct ext4_fc_dentry_update *fc_dentry; wait_queue_head_t *wq; + unsigned long *wait_word = ext4_inode_state_wait_word(inode); + int wait_bit = ext4_inode_state_wait_bit(EXT4_STATE_FC_FLUSHING_DATA); int alloc_ctx; if (ext4_fc_disabled(inode->i_sb)) @@ -268,17 +270,9 @@ void ext4_fc_del(struct inode *inode) WARN_ON(ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING) && !ext4_test_mount_flag(inode->i_sb, EXT4_MF_FC_INELIGIBLE)); while (ext4_test_inode_state(inode, EXT4_STATE_FC_FLUSHING_DATA)) { -#if (BITS_PER_LONG < 64) - DEFINE_WAIT_BIT(wait, &ei->i_state_flags, - EXT4_STATE_FC_FLUSHING_DATA); - wq = bit_waitqueue(&ei->i_state_flags, - EXT4_STATE_FC_FLUSHING_DATA); -#else - DEFINE_WAIT_BIT(wait, &ei->i_flags, - EXT4_STATE_FC_FLUSHING_DATA); - wq = bit_waitqueue(&ei->i_flags, - EXT4_STATE_FC_FLUSHING_DATA); -#endif + DEFINE_WAIT_BIT(wait, wait_word, wait_bit); + + wq = bit_waitqueue(wait_word, wait_bit); prepare_to_wait(wq, &wait.wq_entry, TASK_UNINTERRUPTIBLE); if (ext4_test_inode_state(inode, EXT4_STATE_FC_FLUSHING_DATA)) { ext4_fc_unlock(inode->i_sb, alloc_ctx); @@ -542,6 +536,8 @@ void ext4_fc_track_inode(handle_t *handle, struct inode *inode) { struct ext4_inode_info *ei = EXT4_I(inode); wait_queue_head_t *wq; + unsigned long *wait_word = ext4_inode_state_wait_word(inode); + int wait_bit = ext4_inode_state_wait_bit(EXT4_STATE_FC_COMMITTING); int ret; if (S_ISDIR(inode->i_mode)) @@ -564,17 +560,9 @@ void ext4_fc_track_inode(handle_t *handle, struct inode *inode) lockdep_assert_not_held(&ei->i_data_sem); while (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING)) { -#if (BITS_PER_LONG < 64) - DEFINE_WAIT_BIT(wait, &ei->i_state_flags, - EXT4_STATE_FC_COMMITTING); - wq = bit_waitqueue(&ei->i_state_flags, - EXT4_STATE_FC_COMMITTING); -#else - DEFINE_WAIT_BIT(wait, &ei->i_flags, - EXT4_STATE_FC_COMMITTING); - wq = bit_waitqueue(&ei->i_flags, - EXT4_STATE_FC_COMMITTING); -#endif + DEFINE_WAIT_BIT(wait, wait_word, wait_bit); + + wq = bit_waitqueue(wait_word, wait_bit); prepare_to_wait(wq, &wait.wq_entry, TASK_UNINTERRUPTIBLE); if (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING)) schedule(); @@ -1034,6 +1022,8 @@ static int ext4_fc_perform_commit(journal_t *journal) int ret = 0; u32 crc = 0; int alloc_ctx; + int flushing_wait_bit = + ext4_inode_state_wait_bit(EXT4_STATE_FC_FLUSHING_DATA); /* * Step 1: Mark all inodes on s_fc_q[MAIN] with @@ -1059,11 +1049,8 @@ static int ext4_fc_perform_commit(journal_t *journal) list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) { ext4_clear_inode_state(&iter->vfs_inode, EXT4_STATE_FC_FLUSHING_DATA); -#if (BITS_PER_LONG < 64) - wake_up_bit(&iter->i_state_flags, EXT4_STATE_FC_FLUSHING_DATA); -#else - wake_up_bit(&iter->i_flags, EXT4_STATE_FC_FLUSHING_DATA); -#endif + wake_up_bit(ext4_inode_state_wait_word(&iter->vfs_inode), + flushing_wait_bit); } /* @@ -1279,6 +1266,8 @@ static void ext4_fc_cleanup(journal_t *journal, int full, tid_t tid) struct ext4_inode_info *ei; struct ext4_fc_dentry_update *fc_dentry; int alloc_ctx; + int committing_wait_bit = + ext4_inode_state_wait_bit(EXT4_STATE_FC_COMMITTING); if (full && sbi->s_fc_bh) sbi->s_fc_bh = NULL; @@ -1315,11 +1304,8 @@ static void ext4_fc_cleanup(journal_t *journal, int full, tid_t tid) * barrier in prepare_to_wait() in ext4_fc_track_inode(). */ smp_mb(); -#if (BITS_PER_LONG < 64) - wake_up_bit(&ei->i_state_flags, EXT4_STATE_FC_COMMITTING); -#else - wake_up_bit(&ei->i_flags, EXT4_STATE_FC_COMMITTING); -#endif + wake_up_bit(ext4_inode_state_wait_word(&ei->vfs_inode), + committing_wait_bit); } while (!list_empty(&sbi->s_fc_dentry_q[FC_Q_MAIN])) { From 289a2ca0c9b7eae74f93fc213b0b971669b8683d Mon Sep 17 00:00:00 2001 From: Junrui Luo Date: Wed, 13 May 2026 17:28:40 +0800 Subject: [PATCH 06/20] jbd2: fix integer underflow in jbd2_journal_initialize_fast_commit() jbd2_journal_initialize_fast_commit() validates journal capacity by checking (journal->j_last - num_fc_blks < JBD2_MIN_JOURNAL_BLOCKS). Both j_last and num_fc_blks are unsigned, so when num_fc_blks exceeds j_last the subtraction wraps to a large value, bypassing the bounds check. The resulting underflow corrupts j_last, j_fc_first, and j_free, leading to journal abort. Fix by checking num_fc_blks against j_last before the subtraction, returning -EFSCORRUPTED. Fixes: 6866d7b3f2bb ("ext4 / jbd2: add fast commit initialization") Reported-by: Yuhao Jiang Cc: stable@vger.kernel.org Signed-off-by: Junrui Luo Fixes: e029c5f27987 ("ext4: make num of fast commit blocks configurable") Reviewed-by: Baokun Li Fixes: e029c5f279872 ("ext4: make num of fast commit blocks configurable") Reviewed-by: Zhang Yi Reviewed-by: Jan Kara Link: https://patch.msgid.link/SYBPR01MB7881663C927DE9D7BBF4D1DFAF062@SYBPR01MB7881.ausprd01.prod.outlook.com Signed-off-by: Theodore Ts'o --- fs/jbd2/journal.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c index 4f397fcdb13c..e3b2e38e1a1b 100644 --- a/fs/jbd2/journal.c +++ b/fs/jbd2/journal.c @@ -2263,6 +2263,8 @@ jbd2_journal_initialize_fast_commit(journal_t *journal) unsigned long long num_fc_blks; num_fc_blks = jbd2_journal_get_num_fc_blks(sb); + if (num_fc_blks > journal->j_last) + return -EFSCORRUPTED; if (journal->j_last - num_fc_blks < JBD2_MIN_JOURNAL_BLOCKS) return -ENOSPC; From e9c6e0b8e096255feb71ec996c77bdfbe9c36e91 Mon Sep 17 00:00:00 2001 From: Li Chen Date: Fri, 15 May 2026 17:18:21 +0800 Subject: [PATCH 07/20] ext4: fast commit: snapshot inode state before writing log Fast commit writes inode metadata and data range updates after unlocking journal updates. New handles can start at that point, so the log writing path must not look at live inode state. Add a commit-time per-inode snapshot and populate it while journal updates are locked and existing handles are drained. Store the snapshot behind ext4_inode_info->i_fc_snap so ext4_inode_info only grows by one pointer. The snapshot contains a copy of the on-disk inode plus the data range records needed for fast commit TLVs. Snapshotting runs under jbd2_journal_lock_updates(). Avoid triggering I/O there by using ext4_get_inode_loc_noio() and falling back to full commit if the inode table block is not present or not uptodate. Log writing then only serializes the snapshot, so it no longer needs to call ext4_map_blocks() and take i_data_sem under s_fc_lock. The snapshot is installed and freed under s_fc_lock and is released from fast commit cleanup and inode eviction. Signed-off-by: Li Chen Link: https://patch.msgid.link/20260515091829.194810-2-me@linux.beauty Signed-off-by: Theodore Ts'o --- fs/ext4/ext4.h | 22 ++- fs/ext4/fast_commit.c | 331 +++++++++++++++++++++++++++++++++++------- fs/ext4/inode.c | 51 +++++++ 3 files changed, 352 insertions(+), 52 deletions(-) diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index 6569d1d575a0..e337a37bb6fb 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -1023,6 +1023,7 @@ enum { I_DATA_SEM_EA }; +struct ext4_fc_inode_snap; /* * fourth extended file system inode data in memory @@ -1079,6 +1080,22 @@ struct ext4_inode_info { /* End of lblk range that needs to be committed in this fast commit */ ext4_lblk_t i_fc_lblk_len; + /* + * Commit-time fast commit snapshots. + * + * i_fc_snap is installed and freed under sbi->s_fc_lock. The fast + * commit log writing path reads the snapshot under sbi->s_fc_lock while + * serializing fast commit TLVs. + * + * The snapshot lifetime is bounded by EXT4_STATE_FC_COMMITTING and the + * corresponding cleanup / eviction paths. + * + * i_fc_snap points to per-inode snapshot data for fast commit: + * - a raw inode snapshot for EXT4_FC_TAG_INODE + * - data range records for EXT4_FC_TAG_{ADD,DEL}_RANGE + */ + struct ext4_fc_inode_snap *i_fc_snap; + spinlock_t i_raw_lock; /* protects updates to the raw inode */ /* @@ -3100,8 +3117,9 @@ extern int ext4_file_getattr(struct mnt_idmap *, const struct path *, struct kstat *, u32, unsigned int); extern void ext4_dirty_inode(struct inode *, int); extern int ext4_change_inode_journal_flag(struct inode *, int); -extern int ext4_get_inode_loc(struct inode *, struct ext4_iloc *); -extern int ext4_get_fc_inode_loc(struct super_block *sb, unsigned long ino, +int ext4_get_inode_loc(struct inode *inode, struct ext4_iloc *iloc); +int ext4_get_inode_loc_noio(struct inode *inode, struct ext4_iloc *iloc); +int ext4_get_fc_inode_loc(struct super_block *sb, unsigned long ino, struct ext4_iloc *iloc); extern int ext4_inode_attach_jinode(struct inode *inode); extern int ext4_can_truncate(struct inode *inode); diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c index 1775bce9649a..0c49144e8ca2 100644 --- a/fs/ext4/fast_commit.c +++ b/fs/ext4/fast_commit.c @@ -56,21 +56,23 @@ * deleted while it is being flushed. * [2] Flush data buffers to disk and clear "EXT4_STATE_FC_FLUSHING_DATA" * state. - * [3] Lock the journal by calling jbd2_journal_lock_updates. This ensures that - * all the exsiting handles finish and no new handles can start. - * [4] Mark all the fast commit eligible inodes as undergoing fast commit - * by setting "EXT4_STATE_FC_COMMITTING" state. - * [5] Unlock the journal by calling jbd2_journal_unlock_updates. This allows + * [3] Lock the journal by calling jbd2_journal_lock_updates(). This ensures + * that all the existing handles finish and no new handles can start. + * [4] Mark all the fast commit eligible inodes as undergoing fast commit by + * setting "EXT4_STATE_FC_COMMITTING" state, and snapshot the inode state + * needed for log writing. + * [5] Unlock the journal by calling jbd2_journal_unlock_updates(). This allows * starting of new handles. If new handles try to start an update on * any of the inodes that are being committed, ext4_fc_track_inode() * will block until those inodes have finished the fast commit. * [6] Commit all the directory entry updates in the fast commit space. - * [7] Commit all the changed inodes in the fast commit space and clear - * "EXT4_STATE_FC_COMMITTING" for these inodes. + * [7] Commit all the changed inodes in the fast commit space. * [8] Write tail tag (this tag ensures the atomicity, please read the following * section for more details). + * [9] Clear "EXT4_STATE_FC_COMMITTING" and wake up waiters in + * ext4_fc_cleanup(). * - * All the inode updates must be enclosed within jbd2_jounrnal_start() + * All the inode updates must be enclosed within jbd2_journal_start() * and jbd2_journal_stop() similar to JBD2 journaling. * * Fast Commit Ineligibility @@ -200,6 +202,8 @@ static void ext4_end_buffer_io_sync(struct buffer_head *bh, int uptodate) unlock_buffer(bh); } +static void ext4_fc_free_inode_snap(struct inode *inode); + static inline void ext4_fc_reset_inode(struct inode *inode) { struct ext4_inode_info *ei = EXT4_I(inode); @@ -216,6 +220,7 @@ void ext4_fc_init_inode(struct inode *inode) ext4_clear_inode_state(inode, EXT4_STATE_FC_COMMITTING); INIT_LIST_HEAD(&ei->i_fc_list); INIT_LIST_HEAD(&ei->i_fc_dilist); + ei->i_fc_snap = NULL; } static bool ext4_fc_disabled(struct super_block *sb) @@ -248,6 +253,7 @@ void ext4_fc_del(struct inode *inode) alloc_ctx = ext4_fc_lock(inode->i_sb); if (list_empty(&ei->i_fc_list) && list_empty(&ei->i_fc_dilist)) { + ext4_fc_free_inode_snap(inode); ext4_fc_unlock(inode->i_sb, alloc_ctx); return; } @@ -281,6 +287,7 @@ void ext4_fc_del(struct inode *inode) } finish_wait(wq, &wait.wq_entry); } + ext4_fc_free_inode_snap(inode); list_del_init(&ei->i_fc_list); /* @@ -817,6 +824,21 @@ static bool ext4_fc_add_dentry_tlv(struct super_block *sb, u32 *crc, return true; } +struct ext4_fc_range { + struct list_head list; + u16 tag; + ext4_lblk_t lblk; + ext4_lblk_t len; + ext4_fsblk_t pblk; + bool unwritten; +}; + +struct ext4_fc_inode_snap { + struct list_head data_list; + unsigned int inode_len; + u8 inode_buf[]; +}; + /* * Writes inode in the fast commit space under TLV with tag @tag. * Returns 0 on success, error on failure. @@ -824,21 +846,21 @@ static bool ext4_fc_add_dentry_tlv(struct super_block *sb, u32 *crc, static int ext4_fc_write_inode(struct inode *inode, u32 *crc) { struct ext4_inode_info *ei = EXT4_I(inode); - int inode_len = EXT4_GOOD_OLD_INODE_SIZE; - int ret; - struct ext4_iloc iloc; + struct ext4_fc_inode_snap *snap = ei->i_fc_snap; struct ext4_fc_inode fc_inode; struct ext4_fc_tl tl; u8 *dst; + u8 *src; + int inode_len; + int ret; - ret = ext4_get_inode_loc(inode, &iloc); - if (ret) - return ret; + if (!snap) + return -ECANCELED; - if (ext4_test_inode_flag(inode, EXT4_INODE_INLINE_DATA)) - inode_len = EXT4_INODE_SIZE(inode->i_sb); - else if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE) - inode_len += ei->i_extra_isize; + src = snap->inode_buf; + inode_len = snap->inode_len; + if (!src || inode_len == 0) + return -ECANCELED; fc_inode.fc_ino = cpu_to_le32(inode->i_ino); tl.fc_tag = cpu_to_le16(EXT4_FC_TAG_INODE); @@ -854,10 +876,9 @@ static int ext4_fc_write_inode(struct inode *inode, u32 *crc) dst += EXT4_FC_TAG_BASE_LEN; memcpy(dst, &fc_inode, sizeof(fc_inode)); dst += sizeof(fc_inode); - memcpy(dst, (u8 *)ext4_raw_inode(&iloc), inode_len); + memcpy(dst, src, inode_len); ret = 0; err: - brelse(iloc.bh); return ret; } @@ -867,12 +888,74 @@ static int ext4_fc_write_inode(struct inode *inode, u32 *crc) */ static int ext4_fc_write_inode_data(struct inode *inode, u32 *crc) { - ext4_lblk_t old_blk_size, cur_lblk_off, new_blk_size; struct ext4_inode_info *ei = EXT4_I(inode); - struct ext4_map_blocks map; + struct ext4_fc_inode_snap *snap = ei->i_fc_snap; struct ext4_fc_add_range fc_ext; struct ext4_fc_del_range lrange; struct ext4_extent *ex; + struct ext4_fc_range *range; + + if (!snap) + return -ECANCELED; + + list_for_each_entry(range, &snap->data_list, list) { + if (range->tag == EXT4_FC_TAG_DEL_RANGE) { + lrange.fc_ino = cpu_to_le32(inode->i_ino); + lrange.fc_lblk = cpu_to_le32(range->lblk); + lrange.fc_len = cpu_to_le32(range->len); + if (!ext4_fc_add_tlv(inode->i_sb, EXT4_FC_TAG_DEL_RANGE, + sizeof(lrange), (u8 *)&lrange, crc)) + return -ENOSPC; + continue; + } + + fc_ext.fc_ino = cpu_to_le32(inode->i_ino); + ex = (struct ext4_extent *)&fc_ext.fc_ex; + ex->ee_block = cpu_to_le32(range->lblk); + ex->ee_len = cpu_to_le16(range->len); + ext4_ext_store_pblock(ex, range->pblk); + if (range->unwritten) + ext4_ext_mark_unwritten(ex); + else + ext4_ext_mark_initialized(ex); + + if (!ext4_fc_add_tlv(inode->i_sb, EXT4_FC_TAG_ADD_RANGE, + sizeof(fc_ext), (u8 *)&fc_ext, crc)) + return -ENOSPC; + } + + return 0; +} + +static void ext4_fc_free_ranges(struct list_head *head) +{ + struct ext4_fc_range *range, *range_n; + + list_for_each_entry_safe(range, range_n, head, list) { + list_del(&range->list); + kfree(range); + } +} + +static void ext4_fc_free_inode_snap(struct inode *inode) +{ + struct ext4_inode_info *ei = EXT4_I(inode); + struct ext4_fc_inode_snap *snap = ei->i_fc_snap; + + if (!snap) + return; + + ext4_fc_free_ranges(&snap->data_list); + kfree(snap); + ei->i_fc_snap = NULL; +} + +static int ext4_fc_snapshot_inode_data(struct inode *inode, + struct list_head *ranges) +{ + struct ext4_inode_info *ei = EXT4_I(inode); + ext4_lblk_t start_lblk, end_lblk, cur_lblk; + struct ext4_map_blocks map; int ret; spin_lock(&ei->i_fc_lock); @@ -880,18 +963,21 @@ static int ext4_fc_write_inode_data(struct inode *inode, u32 *crc) spin_unlock(&ei->i_fc_lock); return 0; } - old_blk_size = ei->i_fc_lblk_start; - new_blk_size = ei->i_fc_lblk_start + ei->i_fc_lblk_len - 1; + start_lblk = ei->i_fc_lblk_start; + end_lblk = ei->i_fc_lblk_start + ei->i_fc_lblk_len - 1; ei->i_fc_lblk_len = 0; spin_unlock(&ei->i_fc_lock); - cur_lblk_off = old_blk_size; - ext4_debug("will try writing %d to %d for inode %llu\n", - cur_lblk_off, new_blk_size, inode->i_ino); + cur_lblk = start_lblk; + ext4_debug("snapshot data ranges %u-%u for inode %llu\n", + start_lblk, end_lblk, + (unsigned long long)inode->i_ino); - while (cur_lblk_off <= new_blk_size) { - map.m_lblk = cur_lblk_off; - map.m_len = new_blk_size - cur_lblk_off + 1; + while (cur_lblk <= end_lblk) { + struct ext4_fc_range *range; + + map.m_lblk = cur_lblk; + map.m_len = end_lblk - cur_lblk + 1; ret = ext4_map_blocks(NULL, inode, &map, EXT4_GET_BLOCKS_IO_SUBMIT | EXT4_EX_NOCACHE); @@ -899,17 +985,21 @@ static int ext4_fc_write_inode_data(struct inode *inode, u32 *crc) return -ECANCELED; if (map.m_len == 0) { - cur_lblk_off++; + cur_lblk++; continue; } + range = kmalloc(sizeof(*range), GFP_NOFS); + if (!range) + return -ENOMEM; + + range->lblk = map.m_lblk; + range->len = map.m_len; + range->pblk = 0; + range->unwritten = false; + if (ret == 0) { - lrange.fc_ino = cpu_to_le32(inode->i_ino); - lrange.fc_lblk = cpu_to_le32(map.m_lblk); - lrange.fc_len = cpu_to_le32(map.m_len); - if (!ext4_fc_add_tlv(inode->i_sb, EXT4_FC_TAG_DEL_RANGE, - sizeof(lrange), (u8 *)&lrange, crc)) - return -ENOSPC; + range->tag = EXT4_FC_TAG_DEL_RANGE; } else { unsigned int max = (map.m_flags & EXT4_MAP_UNWRITTEN) ? EXT_UNWRITTEN_MAX_LEN : EXT_INIT_MAX_LEN; @@ -917,26 +1007,67 @@ static int ext4_fc_write_inode_data(struct inode *inode, u32 *crc) /* Limit the number of blocks in one extent */ map.m_len = min(max, map.m_len); - fc_ext.fc_ino = cpu_to_le32(inode->i_ino); - ex = (struct ext4_extent *)&fc_ext.fc_ex; - ex->ee_block = cpu_to_le32(map.m_lblk); - ex->ee_len = cpu_to_le16(map.m_len); - ext4_ext_store_pblock(ex, map.m_pblk); - if (map.m_flags & EXT4_MAP_UNWRITTEN) - ext4_ext_mark_unwritten(ex); - else - ext4_ext_mark_initialized(ex); - if (!ext4_fc_add_tlv(inode->i_sb, EXT4_FC_TAG_ADD_RANGE, - sizeof(fc_ext), (u8 *)&fc_ext, crc)) - return -ENOSPC; + range->tag = EXT4_FC_TAG_ADD_RANGE; + range->len = map.m_len; + range->pblk = map.m_pblk; + range->unwritten = !!(map.m_flags & EXT4_MAP_UNWRITTEN); } - cur_lblk_off += map.m_len; + INIT_LIST_HEAD(&range->list); + list_add_tail(&range->list, ranges); + + cur_lblk += map.m_len; } return 0; } +static int ext4_fc_snapshot_inode(struct inode *inode) +{ + struct ext4_inode_info *ei = EXT4_I(inode); + struct ext4_fc_inode_snap *snap; + int inode_len = EXT4_GOOD_OLD_INODE_SIZE; + struct ext4_iloc iloc; + LIST_HEAD(ranges); + int ret; + int alloc_ctx; + + ret = ext4_get_inode_loc_noio(inode, &iloc); + if (ret) + return ret; + + if (ext4_test_inode_flag(inode, EXT4_INODE_INLINE_DATA)) + inode_len = EXT4_INODE_SIZE(inode->i_sb); + else if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE) + inode_len += ei->i_extra_isize; + + snap = kmalloc(struct_size(snap, inode_buf, inode_len), GFP_NOFS); + if (!snap) { + brelse(iloc.bh); + return -ENOMEM; + } + INIT_LIST_HEAD(&snap->data_list); + snap->inode_len = inode_len; + + memcpy(snap->inode_buf, (u8 *)ext4_raw_inode(&iloc), inode_len); + brelse(iloc.bh); + + ret = ext4_fc_snapshot_inode_data(inode, &ranges); + if (ret) { + kfree(snap); + ext4_fc_free_ranges(&ranges); + return ret; + } + + alloc_ctx = ext4_fc_lock(inode->i_sb); + ext4_fc_free_inode_snap(inode); + ei->i_fc_snap = snap; + list_splice_tail_init(&ranges, &snap->data_list); + ext4_fc_unlock(inode->i_sb, alloc_ctx); + + return 0; +} + /* Flushes data of all the inodes in the commit queue. */ static int ext4_fc_flush_data(journal_t *journal) @@ -987,6 +1118,11 @@ static int ext4_fc_commit_dentry_updates(journal_t *journal, u32 *crc) */ if (list_empty(&fc_dentry->fcd_dilist)) continue; + /* + * For EXT4_FC_TAG_CREAT, fcd_dilist is linked on the created + * inode's i_fc_dilist list (kept singular), so we can recover the + * inode through it. + */ ei = list_first_entry(&fc_dentry->fcd_dilist, struct ext4_inode_info, i_fc_dilist); inode = &ei->vfs_inode; @@ -1011,6 +1147,88 @@ static int ext4_fc_commit_dentry_updates(journal_t *journal, u32 *crc) return 0; } +static int ext4_fc_snapshot_inodes(journal_t *journal) +{ + struct super_block *sb = journal->j_private; + struct ext4_sb_info *sbi = EXT4_SB(sb); + struct ext4_inode_info *iter; + struct ext4_fc_dentry_update *fc_dentry; + struct inode **inodes; + unsigned int nr_inodes = 0; + unsigned int i = 0; + int ret = 0; + int alloc_ctx; + + alloc_ctx = ext4_fc_lock(sb); + list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) + nr_inodes++; + + list_for_each_entry(fc_dentry, &sbi->s_fc_dentry_q[FC_Q_MAIN], fcd_list) { + struct ext4_inode_info *ei; + + if (fc_dentry->fcd_op != EXT4_FC_TAG_CREAT) + continue; + if (list_empty(&fc_dentry->fcd_dilist)) + continue; + + /* See the comment in ext4_fc_commit_dentry_updates(). */ + ei = list_first_entry(&fc_dentry->fcd_dilist, + struct ext4_inode_info, i_fc_dilist); + if (!list_empty(&ei->i_fc_list)) + continue; + + nr_inodes++; + } + ext4_fc_unlock(sb, alloc_ctx); + + if (!nr_inodes) + return 0; + + inodes = kvcalloc(nr_inodes, sizeof(*inodes), GFP_NOFS); + if (!inodes) + return -ENOMEM; + + alloc_ctx = ext4_fc_lock(sb); + list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) { + inodes[i] = igrab(&iter->vfs_inode); + if (inodes[i]) + i++; + } + + list_for_each_entry(fc_dentry, &sbi->s_fc_dentry_q[FC_Q_MAIN], fcd_list) { + struct ext4_inode_info *ei; + + if (fc_dentry->fcd_op != EXT4_FC_TAG_CREAT) + continue; + if (list_empty(&fc_dentry->fcd_dilist)) + continue; + + /* See the comment in ext4_fc_commit_dentry_updates(). */ + ei = list_first_entry(&fc_dentry->fcd_dilist, + struct ext4_inode_info, i_fc_dilist); + if (!list_empty(&ei->i_fc_list)) + continue; + + inodes[i] = igrab(&ei->vfs_inode); + if (inodes[i]) + i++; + } + ext4_fc_unlock(sb, alloc_ctx); + + for (nr_inodes = 0; nr_inodes < i; nr_inodes++) { + ret = ext4_fc_snapshot_inode(inodes[nr_inodes]); + if (ret) + break; + } + + for (nr_inodes = 0; nr_inodes < i; nr_inodes++) { + if (inodes[nr_inodes]) + iput(inodes[nr_inodes]); + } + kvfree(inodes); + return ret; +} + static int ext4_fc_perform_commit(journal_t *journal) { struct super_block *sb = journal->j_private; @@ -1082,7 +1300,11 @@ static int ext4_fc_perform_commit(journal_t *journal) EXT4_STATE_FC_COMMITTING); } ext4_fc_unlock(sb, alloc_ctx); + + ret = ext4_fc_snapshot_inodes(journal); jbd2_journal_unlock_updates(journal); + if (ret) + return ret; /* * Step 5: If file system device is different from journal device, @@ -1281,6 +1503,7 @@ static void ext4_fc_cleanup(journal_t *journal, int full, tid_t tid) struct ext4_inode_info, i_fc_list); list_del_init(&ei->i_fc_list); + ext4_fc_free_inode_snap(&ei->vfs_inode); ext4_clear_inode_state(&ei->vfs_inode, EXT4_STATE_FC_COMMITTING); if (tid_geq(tid, ei->i_sync_tid)) { @@ -1313,6 +1536,14 @@ static void ext4_fc_cleanup(journal_t *journal, int full, tid_t tid) struct ext4_fc_dentry_update, fcd_list); list_del_init(&fc_dentry->fcd_list); + if (fc_dentry->fcd_op == EXT4_FC_TAG_CREAT && + !list_empty(&fc_dentry->fcd_dilist)) { + /* See the comment in ext4_fc_commit_dentry_updates(). */ + ei = list_first_entry(&fc_dentry->fcd_dilist, + struct ext4_inode_info, + i_fc_dilist); + ext4_fc_free_inode_snap(&ei->vfs_inode); + } list_del_init(&fc_dentry->fcd_dilist); release_dentry_name_snapshot(&fc_dentry->fcd_name); diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index c2c2d6ac7f3d..4678612f82e8 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -5025,6 +5025,57 @@ int ext4_get_inode_loc(struct inode *inode, struct ext4_iloc *iloc) return ret; } +/* + * ext4_get_inode_loc_noio() is a best-effort variant of ext4_get_inode_loc(). + * It looks up the inode table block in the buffer cache and returns -EAGAIN if + * the block is not present or not uptodate, without starting any I/O. + */ +int ext4_get_inode_loc_noio(struct inode *inode, struct ext4_iloc *iloc) +{ + struct super_block *sb = inode->i_sb; + struct ext4_group_desc *gdp; + struct buffer_head *bh; + ext4_fsblk_t block; + int inodes_per_block, inode_offset; + unsigned long ino = inode->i_ino; + + iloc->bh = NULL; + if (ino < EXT4_ROOT_INO || + ino > le32_to_cpu(EXT4_SB(sb)->s_es->s_inodes_count)) + return -EFSCORRUPTED; + + iloc->block_group = (ino - 1) / EXT4_INODES_PER_GROUP(sb); + gdp = ext4_get_group_desc(sb, iloc->block_group, NULL); + if (!gdp) + return -EIO; + + /* Figure out the offset within the block group inode table. */ + inodes_per_block = EXT4_SB(sb)->s_inodes_per_block; + inode_offset = ((ino - 1) % EXT4_INODES_PER_GROUP(sb)); + iloc->offset = (inode_offset % inodes_per_block) * EXT4_INODE_SIZE(sb); + + block = ext4_inode_table(sb, gdp); + if (block <= le32_to_cpu(EXT4_SB(sb)->s_es->s_first_data_block) || + block >= ext4_blocks_count(EXT4_SB(sb)->s_es)) { + ext4_error(sb, + "Invalid inode table block %llu in block_group %u", + block, iloc->block_group); + return -EFSCORRUPTED; + } + block += inode_offset / inodes_per_block; + + bh = sb_find_get_block(sb, block); + if (!bh) + return -EAGAIN; + if (!ext4_buffer_uptodate(bh)) { + brelse(bh); + return -EAGAIN; + } + + iloc->bh = bh; + return 0; +} + int ext4_get_fc_inode_loc(struct super_block *sb, unsigned long ino, struct ext4_iloc *iloc) From 7f473f971382d73a58e386afa7efdaac294b89f0 Mon Sep 17 00:00:00 2001 From: Li Chen Date: Fri, 15 May 2026 17:18:22 +0800 Subject: [PATCH 08/20] ext4: lockdep: handle i_data_sem subclassing for special inodes Fast commit can hold s_fc_lock while writing journal blocks. Mapping the journal inode can take its i_data_sem. Normal inode update paths can take a data inode i_data_sem and then s_fc_lock, which makes lockdep report a circular dependency. lockdep treats all i_data_sem instances as one lock class and cannot distinguish the journal inode i_data_sem from a regular inode i_data_sem. The journal inode is not tracked by fast commit and no FC waiters ever depend on it, so this is not a real ABBA deadlock. Assign the journal inode a dedicated i_data_sem lockdep subclass to avoid the false positive. Inode cache objects can be recycled, so also reset i_data_sem to I_DATA_SEM_NORMAL when allocating an ext4 inode. Otherwise a new inode may inherit an old subclass (journal/quota/ea) and trigger lockdep warnings. Signed-off-by: Li Chen Link: https://patch.msgid.link/20260515091829.194810-3-me@linux.beauty Signed-off-by: Theodore Ts'o --- fs/ext4/ext4.h | 4 +++- fs/ext4/super.c | 8 ++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index e337a37bb6fb..115a3c94db16 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -1015,12 +1015,14 @@ do { \ * than the first * I_DATA_SEM_QUOTA - Used for quota inodes only * I_DATA_SEM_EA - Used for ea_inodes only + * I_DATA_SEM_JOURNAL - Used for journal inode only */ enum { I_DATA_SEM_NORMAL = 0, I_DATA_SEM_OTHER, I_DATA_SEM_QUOTA, - I_DATA_SEM_EA + I_DATA_SEM_EA, + I_DATA_SEM_JOURNAL }; struct ext4_fc_inode_snap; diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 4b69e0879731..7c484a53c62e 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -1431,6 +1431,9 @@ static struct inode *ext4_alloc_inode(struct super_block *sb) ext4_fc_init_inode(&ei->vfs_inode); spin_lock_init(&ei->i_fc_lock); mmb_init(&ei->i_metadata_bhs, &ei->vfs_inode.i_data); +#ifdef CONFIG_LOCKDEP + lockdep_set_subclass(&ei->i_data_sem, I_DATA_SEM_NORMAL); +#endif return &ei->vfs_inode; } @@ -5910,6 +5913,11 @@ static struct inode *ext4_get_journal_inode(struct super_block *sb, return ERR_PTR(-EFSCORRUPTED); } +#ifdef CONFIG_LOCKDEP + lockdep_set_subclass(&EXT4_I(journal_inode)->i_data_sem, + I_DATA_SEM_JOURNAL); +#endif + ext4_debug("Journal inode found at %p: %lld bytes\n", journal_inode, journal_inode->i_size); return journal_inode; From b3060e96533dc3157fc6d3d45dc19927c566977b Mon Sep 17 00:00:00 2001 From: Li Chen Date: Fri, 15 May 2026 17:18:23 +0800 Subject: [PATCH 09/20] ext4: fast commit: avoid waiting for FC_COMMITTING ext4_fc_track_inode() can be called while holding i_data_sem (e.g. fallocate). Waiting for EXT4_STATE_FC_COMMITTING in that case risks an ABBA deadlock: i_data_sem -> wait(FC_COMMITTING) vs FC_COMMITTING -> wait(i_data_sem) in the commit task. Now that fast commit snapshots inode state at commit time, updates during log writing do not need to block. Drop the wait and lockdep assertion in ext4_fc_track_inode(), and make ext4_fc_del() wait for FC_COMMITTING so an inode cannot be removed while the commit thread is still using it. When an inode is modified during a fast commit, mark it with EXT4_STATE_FC_REQUEUE so cleanup keeps it queued for the next fast commit. This is needed because jbd2_fc_end_commit() invokes the cleanup callback with tid == 0, so tid-based requeue logic would requeue every inode. Testing: tracepoint ext4:ext4_fc_commit_stop with two fsyncs in the same transaction. nblks is the number of journal blocks written for that fast commit. Before this change, the second fsync still wrote almost the same fast commit log (nblks 10->9), because tid == 0 in jbd2_fc_end_commit() caused the tid-based requeue logic to keep all inodes queued. After this change, only inodes modified during the commit are requeued, and the second fsync wrote a nearly empty fast commit (nblks 10->1). Signed-off-by: Li Chen Link: https://patch.msgid.link/20260515091829.194810-4-me@linux.beauty Signed-off-by: Theodore Ts'o --- fs/ext4/ext4.h | 1 + fs/ext4/fast_commit.c | 108 ++++++++++++++++++++---------------------- 2 files changed, 52 insertions(+), 57 deletions(-) diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index 115a3c94db16..927173bc8381 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -1991,6 +1991,7 @@ enum { EXT4_STATE_FC_COMMITTING, /* Fast commit ongoing */ EXT4_STATE_FC_FLUSHING_DATA, /* Fast commit flushing data */ EXT4_STATE_ORPHAN_FILE, /* Inode orphaned in orphan file */ + EXT4_STATE_FC_REQUEUE, /* Inode modified during fast commit */ }; #define EXT4_INODE_BIT_FNS(name, field, offset) \ diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c index 0c49144e8ca2..673668860e2d 100644 --- a/fs/ext4/fast_commit.c +++ b/fs/ext4/fast_commit.c @@ -62,9 +62,8 @@ * setting "EXT4_STATE_FC_COMMITTING" state, and snapshot the inode state * needed for log writing. * [5] Unlock the journal by calling jbd2_journal_unlock_updates(). This allows - * starting of new handles. If new handles try to start an update on - * any of the inodes that are being committed, ext4_fc_track_inode() - * will block until those inodes have finished the fast commit. + * starting of new handles. Updates to inodes being fast committed are + * tracked for requeue rather than blocking. * [6] Commit all the directory entry updates in the fast commit space. * [7] Commit all the changed inodes in the fast commit space. * [8] Write tail tag (this tag ensures the atomicity, please read the following @@ -218,6 +217,7 @@ void ext4_fc_init_inode(struct inode *inode) ext4_fc_reset_inode(inode); ext4_clear_inode_state(inode, EXT4_STATE_FC_COMMITTING); + ext4_clear_inode_state(inode, EXT4_STATE_FC_REQUEUE); INIT_LIST_HEAD(&ei->i_fc_list); INIT_LIST_HEAD(&ei->i_fc_dilist); ei->i_fc_snap = NULL; @@ -245,7 +245,10 @@ void ext4_fc_del(struct inode *inode) struct ext4_fc_dentry_update *fc_dentry; wait_queue_head_t *wq; unsigned long *wait_word = ext4_inode_state_wait_word(inode); - int wait_bit = ext4_inode_state_wait_bit(EXT4_STATE_FC_FLUSHING_DATA); + int committing_wait_bit = + ext4_inode_state_wait_bit(EXT4_STATE_FC_COMMITTING); + int flushing_wait_bit = + ext4_inode_state_wait_bit(EXT4_STATE_FC_FLUSHING_DATA); int alloc_ctx; if (ext4_fc_disabled(inode->i_sb)) @@ -259,26 +262,26 @@ void ext4_fc_del(struct inode *inode) } /* - * Since ext4_fc_del is called from ext4_evict_inode while having a - * handle open, there is no need for us to wait here even if a fast - * commit is going on. That is because, if this inode is being - * committed, ext4_mark_inode_dirty would have waited for inode commit - * operation to finish before we come here. So, by the time we come - * here, inode's EXT4_STATE_FC_COMMITTING would have been cleared. So, - * we shouldn't see EXT4_STATE_FC_COMMITTING to be set on this inode - * here. - * - * We may come here without any handles open in the "no_delete" case of - * ext4_evict_inode as well. However, if that happens, we first mark the - * file system as fast commit ineligible anyway. So, even in that case, - * it is okay to remove the inode from the fc list. + * Wait for ongoing fast commit to finish. We cannot remove the inode + * from fast commit lists while it is being committed. */ - WARN_ON(ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING) - && !ext4_test_mount_flag(inode->i_sb, EXT4_MF_FC_INELIGIBLE)); - while (ext4_test_inode_state(inode, EXT4_STATE_FC_FLUSHING_DATA)) { - DEFINE_WAIT_BIT(wait, wait_word, wait_bit); + while (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING)) { + DEFINE_WAIT_BIT(wait, wait_word, committing_wait_bit); - wq = bit_waitqueue(wait_word, wait_bit); + wq = bit_waitqueue(wait_word, committing_wait_bit); + prepare_to_wait(wq, &wait.wq_entry, TASK_UNINTERRUPTIBLE); + if (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING)) { + ext4_fc_unlock(inode->i_sb, alloc_ctx); + schedule(); + alloc_ctx = ext4_fc_lock(inode->i_sb); + } + finish_wait(wq, &wait.wq_entry); + } + + while (ext4_test_inode_state(inode, EXT4_STATE_FC_FLUSHING_DATA)) { + DEFINE_WAIT_BIT(wait, wait_word, flushing_wait_bit); + + wq = bit_waitqueue(wait_word, flushing_wait_bit); prepare_to_wait(wq, &wait.wq_entry, TASK_UNINTERRUPTIBLE); if (ext4_test_inode_state(inode, EXT4_STATE_FC_FLUSHING_DATA)) { ext4_fc_unlock(inode->i_sb, alloc_ctx); @@ -287,19 +290,22 @@ void ext4_fc_del(struct inode *inode) } finish_wait(wq, &wait.wq_entry); } + ext4_fc_free_inode_snap(inode); list_del_init(&ei->i_fc_list); /* - * Since this inode is getting removed, let's also remove all FC - * dentry create references, since it is not needed to log it anyways. + * Since this inode is getting removed, let's also remove all FC dentry + * create references, since it is not needed to log it anyways. */ if (list_empty(&ei->i_fc_dilist)) { ext4_fc_unlock(inode->i_sb, alloc_ctx); return; } - fc_dentry = list_first_entry(&ei->i_fc_dilist, struct ext4_fc_dentry_update, fcd_dilist); + fc_dentry = list_first_entry(&ei->i_fc_dilist, + struct ext4_fc_dentry_update, + fcd_dilist); WARN_ON(fc_dentry->fcd_op != EXT4_FC_TAG_CREAT); list_del_init(&fc_dentry->fcd_list); list_del_init(&fc_dentry->fcd_dilist); @@ -371,6 +377,8 @@ static int ext4_fc_track_template( tid = handle->h_transaction->t_tid; spin_lock(&ei->i_fc_lock); + if (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING)) + ext4_set_inode_state(inode, EXT4_STATE_FC_REQUEUE); if (tid == ei->i_sync_tid) { update = true; } else { @@ -541,10 +549,6 @@ static int __track_inode(handle_t *handle, struct inode *inode, void *arg, void ext4_fc_track_inode(handle_t *handle, struct inode *inode) { - struct ext4_inode_info *ei = EXT4_I(inode); - wait_queue_head_t *wq; - unsigned long *wait_word = ext4_inode_state_wait_word(inode); - int wait_bit = ext4_inode_state_wait_bit(EXT4_STATE_FC_COMMITTING); int ret; if (S_ISDIR(inode->i_mode)) @@ -560,21 +564,11 @@ void ext4_fc_track_inode(handle_t *handle, struct inode *inode) return; /* - * If we come here, we may sleep while waiting for the inode to - * commit. We shouldn't be holding i_data_sem when we go to sleep since - * the commit path needs to grab the lock while committing the inode. + * Fast commit snapshots inode state at commit time, so there's no need + * to wait for EXT4_STATE_FC_COMMITTING here. If the inode is already + * on the commit queue, ext4_fc_cleanup() will requeue it for the new + * transaction once the current commit finishes. */ - lockdep_assert_not_held(&ei->i_data_sem); - - while (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING)) { - DEFINE_WAIT_BIT(wait, wait_word, wait_bit); - - wq = bit_waitqueue(wait_word, wait_bit); - prepare_to_wait(wq, &wait.wq_entry, TASK_UNINTERRUPTIBLE); - if (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING)) - schedule(); - finish_wait(wq, &wait.wq_entry); - } /* * From this point on, this inode will not be committed either @@ -1499,32 +1493,32 @@ static void ext4_fc_cleanup(journal_t *journal, int full, tid_t tid) alloc_ctx = ext4_fc_lock(sb); while (!list_empty(&sbi->s_fc_q[FC_Q_MAIN])) { + bool requeue; + ei = list_first_entry(&sbi->s_fc_q[FC_Q_MAIN], struct ext4_inode_info, i_fc_list); list_del_init(&ei->i_fc_list); ext4_fc_free_inode_snap(&ei->vfs_inode); + spin_lock(&ei->i_fc_lock); + if (full) + requeue = !tid_geq(tid, ei->i_sync_tid); + else + requeue = ext4_test_inode_state(&ei->vfs_inode, + EXT4_STATE_FC_REQUEUE); + if (!requeue) + ext4_fc_reset_inode(&ei->vfs_inode); + ext4_clear_inode_state(&ei->vfs_inode, EXT4_STATE_FC_REQUEUE); ext4_clear_inode_state(&ei->vfs_inode, EXT4_STATE_FC_COMMITTING); - if (tid_geq(tid, ei->i_sync_tid)) { - ext4_fc_reset_inode(&ei->vfs_inode); - } else if (full) { - /* - * We are called after a full commit, inode has been - * modified while the commit was running. Re-enqueue - * the inode into STAGING, which will then be splice - * back into MAIN. This cannot happen during - * fastcommit because the journal is locked all the - * time in that case (and tid doesn't increase so - * tid check above isn't reliable). - */ + spin_unlock(&ei->i_fc_lock); + if (requeue) list_add_tail(&ei->i_fc_list, &sbi->s_fc_q[FC_Q_STAGING]); - } /* * Make sure clearing of EXT4_STATE_FC_COMMITTING is * visible before we send the wakeup. Pairs with implicit - * barrier in prepare_to_wait() in ext4_fc_track_inode(). + * barrier in prepare_to_wait() in ext4_fc_del(). */ smp_mb(); wake_up_bit(ext4_inode_state_wait_word(&ei->vfs_inode), From 2b9b216628fd9352f9c791701c8990d05736aa90 Mon Sep 17 00:00:00 2001 From: Li Chen Date: Fri, 15 May 2026 17:18:24 +0800 Subject: [PATCH 10/20] ext4: fast commit: avoid self-deadlock in inode snapshotting ext4_fc_snapshot_inodes() used igrab()/iput() to pin inodes while building commit-time snapshots. With ext4_fc_del() waiting for EXT4_STATE_FC_COMMITTING, iput() can trigger ext4_clear_inode()->ext4_fc_del() in the commit thread and deadlock waiting for the fast commit to finish. ext4_fc_del() also has to re-check EXT4_STATE_FC_COMMITTING after waiting on EXT4_STATE_FC_FLUSHING_DATA. The commit thread clears FLUSHING_DATA before it sets COMMITTING, so a waiter woken from the flush wait must not delete the inode based on an old COMMITTING check. Avoid taking extra references. Collect inode pointers under s_fc_lock and rely on EXT4_STATE_FC_COMMITTING to pin inodes until ext4_fc_cleanup() clears the bit. Also set EXT4_STATE_FC_COMMITTING for create-only inodes referenced from the dentry update queue, and wake up waiters when ext4_fc_cleanup() clears the bit. Signed-off-by: Li Chen Link: https://patch.msgid.link/20260515091829.194810-5-me@linux.beauty Signed-off-by: Theodore Ts'o --- fs/ext4/fast_commit.c | 124 +++++++++++++++++++++++++----------------- 1 file changed, 75 insertions(+), 49 deletions(-) diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c index 673668860e2d..8a6981e50ffe 100644 --- a/fs/ext4/fast_commit.c +++ b/fs/ext4/fast_commit.c @@ -235,6 +235,37 @@ static bool ext4_fc_eligible(struct super_block *sb) !(ext4_test_mount_flag(sb, EXT4_MF_FC_INELIGIBLE)); } +/* + * Wait for an inode fast-commit state bit to clear while dropping the + * fast-commit lock around schedule(). + */ +static void ext4_fc_wait_inode_state(struct inode *inode, int bit, + int *alloc_ctx) +{ + wait_queue_head_t *wq; + unsigned long *wait_word = ext4_inode_state_wait_word(inode); + int wait_bit = ext4_inode_state_wait_bit(bit); + + while (ext4_test_inode_state(inode, bit)) { + DEFINE_WAIT_BIT(wait, wait_word, wait_bit); + + wq = bit_waitqueue(wait_word, wait_bit); + prepare_to_wait(wq, &wait.wq_entry, TASK_UNINTERRUPTIBLE); + if (ext4_test_inode_state(inode, bit)) { + ext4_fc_unlock(inode->i_sb, *alloc_ctx); + schedule(); + *alloc_ctx = ext4_fc_lock(inode->i_sb); + } + finish_wait(wq, &wait.wq_entry); + } +} + +static inline void ext4_fc_wake_inode_state(struct inode *inode, int bit) +{ + wake_up_bit(ext4_inode_state_wait_word(inode), + ext4_inode_state_wait_bit(bit)); +} + /* * Remove inode from fast commit list. If the inode is being committed * we wait until inode commit is done. @@ -243,12 +274,6 @@ void ext4_fc_del(struct inode *inode) { struct ext4_inode_info *ei = EXT4_I(inode); struct ext4_fc_dentry_update *fc_dentry; - wait_queue_head_t *wq; - unsigned long *wait_word = ext4_inode_state_wait_word(inode); - int committing_wait_bit = - ext4_inode_state_wait_bit(EXT4_STATE_FC_COMMITTING); - int flushing_wait_bit = - ext4_inode_state_wait_bit(EXT4_STATE_FC_FLUSHING_DATA); int alloc_ctx; if (ext4_fc_disabled(inode->i_sb)) @@ -263,32 +288,19 @@ void ext4_fc_del(struct inode *inode) /* * Wait for ongoing fast commit to finish. We cannot remove the inode - * from fast commit lists while it is being committed. + * from fast commit lists while it is being committed. If we wake from + * FC_FLUSHING_DATA, re-check FC_COMMITTING before deleting because the + * commit thread sets FC_COMMITTING only after clearing FLUSHING_DATA. */ - while (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING)) { - DEFINE_WAIT_BIT(wait, wait_word, committing_wait_bit); + for (;;) { + ext4_fc_wait_inode_state(inode, EXT4_STATE_FC_COMMITTING, + &alloc_ctx); - wq = bit_waitqueue(wait_word, committing_wait_bit); - prepare_to_wait(wq, &wait.wq_entry, TASK_UNINTERRUPTIBLE); - if (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING)) { - ext4_fc_unlock(inode->i_sb, alloc_ctx); - schedule(); - alloc_ctx = ext4_fc_lock(inode->i_sb); - } - finish_wait(wq, &wait.wq_entry); - } + if (!ext4_test_inode_state(inode, EXT4_STATE_FC_FLUSHING_DATA)) + break; - while (ext4_test_inode_state(inode, EXT4_STATE_FC_FLUSHING_DATA)) { - DEFINE_WAIT_BIT(wait, wait_word, flushing_wait_bit); - - wq = bit_waitqueue(wait_word, flushing_wait_bit); - prepare_to_wait(wq, &wait.wq_entry, TASK_UNINTERRUPTIBLE); - if (ext4_test_inode_state(inode, EXT4_STATE_FC_FLUSHING_DATA)) { - ext4_fc_unlock(inode->i_sb, alloc_ctx); - schedule(); - alloc_ctx = ext4_fc_lock(inode->i_sb); - } - finish_wait(wq, &wait.wq_entry); + ext4_fc_wait_inode_state(inode, EXT4_STATE_FC_FLUSHING_DATA, + &alloc_ctx); } ext4_fc_free_inode_snap(inode); @@ -1184,13 +1196,12 @@ static int ext4_fc_snapshot_inodes(journal_t *journal) alloc_ctx = ext4_fc_lock(sb); list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) { - inodes[i] = igrab(&iter->vfs_inode); - if (inodes[i]) - i++; + inodes[i++] = &iter->vfs_inode; } list_for_each_entry(fc_dentry, &sbi->s_fc_dentry_q[FC_Q_MAIN], fcd_list) { struct ext4_inode_info *ei; + struct inode *inode; if (fc_dentry->fcd_op != EXT4_FC_TAG_CREAT) continue; @@ -1200,12 +1211,20 @@ static int ext4_fc_snapshot_inodes(journal_t *journal) /* See the comment in ext4_fc_commit_dentry_updates(). */ ei = list_first_entry(&fc_dentry->fcd_dilist, struct ext4_inode_info, i_fc_dilist); + inode = &ei->vfs_inode; if (!list_empty(&ei->i_fc_list)) continue; - inodes[i] = igrab(&ei->vfs_inode); - if (inodes[i]) - i++; + /* + * Create-only inodes may only be referenced via fcd_dilist and + * not appear on s_fc_q[MAIN]. They may hit the last iput while + * we are snapshotting, but inode eviction calls ext4_fc_del(), + * which waits for FC_COMMITTING to clear. Mark them FC_COMMITTING + * so the inode stays pinned and the snapshot stays valid until + * ext4_fc_cleanup(). + */ + ext4_set_inode_state(inode, EXT4_STATE_FC_COMMITTING); + inodes[i++] = inode; } ext4_fc_unlock(sb, alloc_ctx); @@ -1215,10 +1234,6 @@ static int ext4_fc_snapshot_inodes(journal_t *journal) break; } - for (nr_inodes = 0; nr_inodes < i; nr_inodes++) { - if (inodes[nr_inodes]) - iput(inodes[nr_inodes]); - } kvfree(inodes); return ret; } @@ -1234,8 +1249,6 @@ static int ext4_fc_perform_commit(journal_t *journal) int ret = 0; u32 crc = 0; int alloc_ctx; - int flushing_wait_bit = - ext4_inode_state_wait_bit(EXT4_STATE_FC_FLUSHING_DATA); /* * Step 1: Mark all inodes on s_fc_q[MAIN] with @@ -1261,8 +1274,8 @@ static int ext4_fc_perform_commit(journal_t *journal) list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) { ext4_clear_inode_state(&iter->vfs_inode, EXT4_STATE_FC_FLUSHING_DATA); - wake_up_bit(ext4_inode_state_wait_word(&iter->vfs_inode), - flushing_wait_bit); + ext4_fc_wake_inode_state(&iter->vfs_inode, + EXT4_STATE_FC_FLUSHING_DATA); } /* @@ -1285,8 +1298,9 @@ static int ext4_fc_perform_commit(journal_t *journal) jbd2_journal_lock_updates(journal); /* * The journal is now locked. No more handles can start and all the - * previous handles are now drained. We now mark the inodes on the - * commit queue as being committed. + * previous handles are now drained. Snapshotting happens in this + * window so log writing can consume only stable snapshots without + * doing logical-to-physical mapping. */ alloc_ctx = ext4_fc_lock(sb); list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) { @@ -1482,8 +1496,6 @@ static void ext4_fc_cleanup(journal_t *journal, int full, tid_t tid) struct ext4_inode_info *ei; struct ext4_fc_dentry_update *fc_dentry; int alloc_ctx; - int committing_wait_bit = - ext4_inode_state_wait_bit(EXT4_STATE_FC_COMMITTING); if (full && sbi->s_fc_bh) sbi->s_fc_bh = NULL; @@ -1521,8 +1533,8 @@ static void ext4_fc_cleanup(journal_t *journal, int full, tid_t tid) * barrier in prepare_to_wait() in ext4_fc_del(). */ smp_mb(); - wake_up_bit(ext4_inode_state_wait_word(&ei->vfs_inode), - committing_wait_bit); + ext4_fc_wake_inode_state(&ei->vfs_inode, + EXT4_STATE_FC_COMMITTING); } while (!list_empty(&sbi->s_fc_dentry_q[FC_Q_MAIN])) { @@ -1537,6 +1549,20 @@ static void ext4_fc_cleanup(journal_t *journal, int full, tid_t tid) struct ext4_inode_info, i_fc_dilist); ext4_fc_free_inode_snap(&ei->vfs_inode); + spin_lock(&ei->i_fc_lock); + ext4_clear_inode_state(&ei->vfs_inode, + EXT4_STATE_FC_REQUEUE); + ext4_clear_inode_state(&ei->vfs_inode, + EXT4_STATE_FC_COMMITTING); + spin_unlock(&ei->i_fc_lock); + /* + * Make sure clearing of EXT4_STATE_FC_COMMITTING is + * visible before we send the wakeup. Pairs with implicit + * barrier in prepare_to_wait() in ext4_fc_del(). + */ + smp_mb(); + ext4_fc_wake_inode_state(&ei->vfs_inode, + EXT4_STATE_FC_COMMITTING); } list_del_init(&fc_dentry->fcd_dilist); From 22d887e06a57261df58404c8dce50c4ef37549ed Mon Sep 17 00:00:00 2001 From: Li Chen Date: Fri, 15 May 2026 17:18:25 +0800 Subject: [PATCH 11/20] ext4: fast commit: avoid i_data_sem by dropping ext4_map_blocks() in snapshots Commit-time snapshots run under jbd2_journal_lock_updates(), so the work done there must stay bounded. The snapshot path still used ext4_map_blocks() to build data ranges. This can take i_data_sem and pulls the mapping code into the snapshot logic. Build inode data range snapshots from the extent status tree instead. The extent status tree is a cache, not an authoritative source. If the needed information is missing or unstable (e.g. delayed allocation), treat the transaction as fast commit ineligible and fall back to full commit. Also cap the number of inodes and ranges snapshotted per fast commit and allocate range records from a dedicated slab cache. The inode pointer array is allocated outside the updates-locked window. Testing: QEMU/KVM guest, virtio-pmem + dax, ext4 -O fast_commit, mounted dax,noatime. Ran python3 500x {4K write + fsync}, fallocate 256M, and python3 500x {creat + fsync(dir)} without lockdep splats or errors. Signed-off-by: Li Chen Link: https://patch.msgid.link/20260515091829.194810-6-me@linux.beauty Signed-off-by: Theodore Ts'o --- fs/ext4/fast_commit.c | 253 ++++++++++++++++++++++++++++++------------ 1 file changed, 179 insertions(+), 74 deletions(-) diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c index 8a6981e50ffe..9e73c83b0e25 100644 --- a/fs/ext4/fast_commit.c +++ b/fs/ext4/fast_commit.c @@ -184,6 +184,15 @@ #include static struct kmem_cache *ext4_fc_dentry_cachep; +static struct kmem_cache *ext4_fc_range_cachep; + +/* + * Avoid spending unbounded time/memory snapshotting highly fragmented files + * under jbd2_journal_lock_updates(). If we exceed this limit, fall back to + * full commit. + */ +#define EXT4_FC_SNAPSHOT_MAX_INODES 1024 +#define EXT4_FC_SNAPSHOT_MAX_RANGES 2048 static void ext4_end_buffer_io_sync(struct buffer_head *bh, int uptodate) { @@ -939,7 +948,7 @@ static void ext4_fc_free_ranges(struct list_head *head) list_for_each_entry_safe(range, range_n, head, list) { list_del(&range->list); - kfree(range); + kmem_cache_free(ext4_fc_range_cachep, range); } } @@ -957,16 +966,19 @@ static void ext4_fc_free_inode_snap(struct inode *inode) } static int ext4_fc_snapshot_inode_data(struct inode *inode, - struct list_head *ranges) + struct list_head *ranges, + unsigned int nr_ranges_total, + unsigned int *nr_rangesp) { struct ext4_inode_info *ei = EXT4_I(inode); + unsigned int nr_ranges = 0; ext4_lblk_t start_lblk, end_lblk, cur_lblk; - struct ext4_map_blocks map; - int ret; spin_lock(&ei->i_fc_lock); if (ei->i_fc_lblk_len == 0) { spin_unlock(&ei->i_fc_lock); + if (nr_rangesp) + *nr_rangesp = 0; return 0; } start_lblk = ei->i_fc_lblk_start; @@ -980,61 +992,82 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode, (unsigned long long)inode->i_ino); while (cur_lblk <= end_lblk) { + struct extent_status es; struct ext4_fc_range *range; + ext4_lblk_t len; + u64 remaining = (u64)end_lblk - cur_lblk + 1; - map.m_lblk = cur_lblk; - map.m_len = end_lblk - cur_lblk + 1; - ret = ext4_map_blocks(NULL, inode, &map, - EXT4_GET_BLOCKS_IO_SUBMIT | - EXT4_EX_NOCACHE); - if (ret < 0) - return -ECANCELED; + if (!ext4_es_lookup_extent(inode, cur_lblk, NULL, &es, NULL)) + return -EAGAIN; - if (map.m_len == 0) { + if (ext4_es_is_delayed(&es)) + return -EAGAIN; + + len = es.es_len - (cur_lblk - es.es_lblk); + if (len > remaining) + len = remaining; + if (len == 0) { cur_lblk++; continue; } - range = kmalloc(sizeof(*range), GFP_NOFS); + if (nr_ranges_total + nr_ranges >= EXT4_FC_SNAPSHOT_MAX_RANGES) + return -E2BIG; + + range = kmem_cache_alloc(ext4_fc_range_cachep, GFP_NOFS); if (!range) return -ENOMEM; + nr_ranges++; - range->lblk = map.m_lblk; - range->len = map.m_len; + range->lblk = cur_lblk; + range->len = len; range->pblk = 0; range->unwritten = false; - if (ret == 0) { + if (ext4_es_is_hole(&es)) { range->tag = EXT4_FC_TAG_DEL_RANGE; - } else { - unsigned int max = (map.m_flags & EXT4_MAP_UNWRITTEN) ? - EXT_UNWRITTEN_MAX_LEN : EXT_INIT_MAX_LEN; - - /* Limit the number of blocks in one extent */ - map.m_len = min(max, map.m_len); + } else if (ext4_es_is_written(&es) || + ext4_es_is_unwritten(&es)) { + unsigned int max; range->tag = EXT4_FC_TAG_ADD_RANGE; - range->len = map.m_len; - range->pblk = map.m_pblk; - range->unwritten = !!(map.m_flags & EXT4_MAP_UNWRITTEN); + range->pblk = ext4_es_pblock(&es) + + (cur_lblk - es.es_lblk); + range->unwritten = ext4_es_is_unwritten(&es); + + max = range->unwritten ? EXT_UNWRITTEN_MAX_LEN : + EXT_INIT_MAX_LEN; + if (range->len > max) + range->len = max; + } else { + kmem_cache_free(ext4_fc_range_cachep, range); + return -EAGAIN; } INIT_LIST_HEAD(&range->list); list_add_tail(&range->list, ranges); - cur_lblk += map.m_len; + if ((u64)range->len > (u64)end_lblk - cur_lblk) + break; + + cur_lblk += range->len; } + if (nr_rangesp) + *nr_rangesp = nr_ranges; return 0; } -static int ext4_fc_snapshot_inode(struct inode *inode) +static int ext4_fc_snapshot_inode(struct inode *inode, + unsigned int nr_ranges_total, + unsigned int *nr_rangesp) { struct ext4_inode_info *ei = EXT4_I(inode); struct ext4_fc_inode_snap *snap; int inode_len = EXT4_GOOD_OLD_INODE_SIZE; struct ext4_iloc iloc; LIST_HEAD(ranges); + unsigned int nr_ranges = 0; int ret; int alloc_ctx; @@ -1058,7 +1091,8 @@ static int ext4_fc_snapshot_inode(struct inode *inode) memcpy(snap->inode_buf, (u8 *)ext4_raw_inode(&iloc), inode_len); brelse(iloc.bh); - ret = ext4_fc_snapshot_inode_data(inode, &ranges); + ret = ext4_fc_snapshot_inode_data(inode, &ranges, nr_ranges_total, + &nr_ranges); if (ret) { kfree(snap); ext4_fc_free_ranges(&ranges); @@ -1071,10 +1105,11 @@ static int ext4_fc_snapshot_inode(struct inode *inode) list_splice_tail_init(&ranges, &snap->data_list); ext4_fc_unlock(inode->i_sb, alloc_ctx); + if (nr_rangesp) + *nr_rangesp = nr_ranges; return 0; } - /* Flushes data of all the inodes in the commit queue. */ static int ext4_fc_flush_data(journal_t *journal) { @@ -1153,49 +1188,32 @@ static int ext4_fc_commit_dentry_updates(journal_t *journal, u32 *crc) return 0; } -static int ext4_fc_snapshot_inodes(journal_t *journal) +static int ext4_fc_alloc_snapshot_inodes(struct super_block *sb, + struct inode ***inodesp, + unsigned int *nr_inodesp); + +static int ext4_fc_snapshot_inodes(journal_t *journal, struct inode **inodes, + unsigned int inodes_size) { struct super_block *sb = journal->j_private; struct ext4_sb_info *sbi = EXT4_SB(sb); struct ext4_inode_info *iter; struct ext4_fc_dentry_update *fc_dentry; - struct inode **inodes; - unsigned int nr_inodes = 0; unsigned int i = 0; + unsigned int idx; + unsigned int nr_ranges = 0; int ret = 0; int alloc_ctx; - alloc_ctx = ext4_fc_lock(sb); - list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) - nr_inodes++; - - list_for_each_entry(fc_dentry, &sbi->s_fc_dentry_q[FC_Q_MAIN], fcd_list) { - struct ext4_inode_info *ei; - - if (fc_dentry->fcd_op != EXT4_FC_TAG_CREAT) - continue; - if (list_empty(&fc_dentry->fcd_dilist)) - continue; - - /* See the comment in ext4_fc_commit_dentry_updates(). */ - ei = list_first_entry(&fc_dentry->fcd_dilist, - struct ext4_inode_info, i_fc_dilist); - if (!list_empty(&ei->i_fc_list)) - continue; - - nr_inodes++; - } - ext4_fc_unlock(sb, alloc_ctx); - - if (!nr_inodes) + if (!inodes_size) return 0; - inodes = kvcalloc(nr_inodes, sizeof(*inodes), GFP_NOFS); - if (!inodes) - return -ENOMEM; - alloc_ctx = ext4_fc_lock(sb); list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) { + if (i >= inodes_size) { + ret = -E2BIG; + goto unlock; + } inodes[i++] = &iter->vfs_inode; } @@ -1215,6 +1233,10 @@ static int ext4_fc_snapshot_inodes(journal_t *journal) if (!list_empty(&ei->i_fc_list)) continue; + if (i >= inodes_size) { + ret = -E2BIG; + goto unlock; + } /* * Create-only inodes may only be referenced via fcd_dilist and * not appear on s_fc_q[MAIN]. They may hit the last iput while @@ -1226,15 +1248,22 @@ static int ext4_fc_snapshot_inodes(journal_t *journal) ext4_set_inode_state(inode, EXT4_STATE_FC_COMMITTING); inodes[i++] = inode; } +unlock: ext4_fc_unlock(sb, alloc_ctx); - for (nr_inodes = 0; nr_inodes < i; nr_inodes++) { - ret = ext4_fc_snapshot_inode(inodes[nr_inodes]); + if (ret) + return ret; + + for (idx = 0; idx < i; idx++) { + unsigned int inode_ranges = 0; + + ret = ext4_fc_snapshot_inode(inodes[idx], nr_ranges, + &inode_ranges); if (ret) break; + nr_ranges += inode_ranges; } - kvfree(inodes); return ret; } @@ -1245,6 +1274,8 @@ static int ext4_fc_perform_commit(journal_t *journal) struct ext4_inode_info *iter; struct ext4_fc_head head; struct inode *inode; + struct inode **inodes; + unsigned int inodes_size; struct blk_plug plug; int ret = 0; u32 crc = 0; @@ -1294,6 +1325,10 @@ static int ext4_fc_perform_commit(journal_t *journal) return ret; + ret = ext4_fc_alloc_snapshot_inodes(sb, &inodes, &inodes_size); + if (ret) + return ret; + /* Step 4: Mark all inodes as being committed. */ jbd2_journal_lock_updates(journal); /* @@ -1309,8 +1344,9 @@ static int ext4_fc_perform_commit(journal_t *journal) } ext4_fc_unlock(sb, alloc_ctx); - ret = ext4_fc_snapshot_inodes(journal); + ret = ext4_fc_snapshot_inodes(journal, inodes, inodes_size); jbd2_journal_unlock_updates(journal); + kvfree(inodes); if (ret) return ret; @@ -1366,6 +1402,64 @@ static int ext4_fc_perform_commit(journal_t *journal) return ret; } +static unsigned int ext4_fc_count_snapshot_inodes(struct super_block *sb) +{ + struct ext4_sb_info *sbi = EXT4_SB(sb); + struct ext4_inode_info *iter; + struct ext4_fc_dentry_update *fc_dentry; + unsigned int nr_inodes = 0; + int alloc_ctx; + + alloc_ctx = ext4_fc_lock(sb); + list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) + nr_inodes++; + + list_for_each_entry(fc_dentry, &sbi->s_fc_dentry_q[FC_Q_MAIN], fcd_list) { + struct ext4_inode_info *ei; + + if (fc_dentry->fcd_op != EXT4_FC_TAG_CREAT) + continue; + if (list_empty(&fc_dentry->fcd_dilist)) + continue; + + /* See the comment in ext4_fc_commit_dentry_updates(). */ + ei = list_first_entry(&fc_dentry->fcd_dilist, + struct ext4_inode_info, i_fc_dilist); + if (!list_empty(&ei->i_fc_list)) + continue; + + nr_inodes++; + } + ext4_fc_unlock(sb, alloc_ctx); + + return nr_inodes; +} + +static int ext4_fc_alloc_snapshot_inodes(struct super_block *sb, + struct inode ***inodesp, + unsigned int *nr_inodesp) +{ + unsigned int nr_inodes = ext4_fc_count_snapshot_inodes(sb); + struct inode **inodes; + + *inodesp = NULL; + *nr_inodesp = 0; + + if (!nr_inodes) + return 0; + + if (nr_inodes > EXT4_FC_SNAPSHOT_MAX_INODES) + return -E2BIG; + + inodes = kvcalloc(nr_inodes, sizeof(*inodes), GFP_NOFS); + if (!inodes) + return -ENOMEM; + + *inodesp = inodes; + *nr_inodesp = nr_inodes; + return 0; +} + static void ext4_fc_update_stats(struct super_block *sb, int status, u64 commit_time, int nblks, tid_t commit_tid) { @@ -1458,7 +1552,10 @@ int ext4_fc_commit(journal_t *journal, tid_t commit_tid) fc_bufs_before = (sbi->s_fc_bytes + bsize - 1) / bsize; ret = ext4_fc_perform_commit(journal); if (ret < 0) { - status = EXT4_FC_STATUS_FAILED; + if (ret == -EAGAIN || ret == -E2BIG || ret == -ECANCELED) + status = EXT4_FC_STATUS_INELIGIBLE; + else + status = EXT4_FC_STATUS_FAILED; goto fallback; } nblks = (sbi->s_fc_bytes + bsize - 1) / bsize - fc_bufs_before; @@ -1539,26 +1636,27 @@ static void ext4_fc_cleanup(journal_t *journal, int full, tid_t tid) while (!list_empty(&sbi->s_fc_dentry_q[FC_Q_MAIN])) { fc_dentry = list_first_entry(&sbi->s_fc_dentry_q[FC_Q_MAIN], - struct ext4_fc_dentry_update, - fcd_list); + struct ext4_fc_dentry_update, + fcd_list); list_del_init(&fc_dentry->fcd_list); if (fc_dentry->fcd_op == EXT4_FC_TAG_CREAT && - !list_empty(&fc_dentry->fcd_dilist)) { + !list_empty(&fc_dentry->fcd_dilist)) { /* See the comment in ext4_fc_commit_dentry_updates(). */ ei = list_first_entry(&fc_dentry->fcd_dilist, - struct ext4_inode_info, - i_fc_dilist); + struct ext4_inode_info, + i_fc_dilist); ext4_fc_free_inode_snap(&ei->vfs_inode); spin_lock(&ei->i_fc_lock); ext4_clear_inode_state(&ei->vfs_inode, - EXT4_STATE_FC_REQUEUE); + EXT4_STATE_FC_REQUEUE); ext4_clear_inode_state(&ei->vfs_inode, - EXT4_STATE_FC_COMMITTING); + EXT4_STATE_FC_COMMITTING); spin_unlock(&ei->i_fc_lock); /* * Make sure clearing of EXT4_STATE_FC_COMMITTING is - * visible before we send the wakeup. Pairs with implicit - * barrier in prepare_to_wait() in ext4_fc_del(). + * visible before we send the wakeup. Pairs with + * implicit barrier in prepare_to_wait() in + * ext4_fc_del(). */ smp_mb(); ext4_fc_wake_inode_state(&ei->vfs_inode, @@ -2538,13 +2636,20 @@ int __init ext4_fc_init_dentry_cache(void) ext4_fc_dentry_cachep = KMEM_CACHE(ext4_fc_dentry_update, SLAB_RECLAIM_ACCOUNT); - if (ext4_fc_dentry_cachep == NULL) + if (!ext4_fc_dentry_cachep) return -ENOMEM; + ext4_fc_range_cachep = KMEM_CACHE(ext4_fc_range, SLAB_RECLAIM_ACCOUNT); + if (!ext4_fc_range_cachep) { + kmem_cache_destroy(ext4_fc_dentry_cachep); + return -ENOMEM; + } + return 0; } void ext4_fc_destroy_dentry_cache(void) { + kmem_cache_destroy(ext4_fc_range_cachep); kmem_cache_destroy(ext4_fc_dentry_cachep); } From d2f6e83bbbef31169ea363af4277f5c09c914eda Mon Sep 17 00:00:00 2001 From: Li Chen Date: Fri, 15 May 2026 17:18:26 +0800 Subject: [PATCH 12/20] ext4: fast commit: add lock_updates tracepoint Commit-time fast commit snapshots run under jbd2_journal_lock_updates(), so it is useful to quantify the time spent with updates locked and to understand why snapshotting can fail. Add a new tracepoint, ext4_fc_lock_updates, reporting the time spent in the updates-locked window along with the number of snapshotted inodes and ranges. Record the first snapshot failure reason in a stable snap_err field for tooling. Signed-off-by: Li Chen Reviewed-by: Steven Rostedt (Google) Link: https://patch.msgid.link/20260515091829.194810-7-me@linux.beauty Signed-off-by: Theodore Ts'o --- fs/ext4/ext4.h | 15 ++++++++ fs/ext4/fast_commit.c | 74 +++++++++++++++++++++++++++++-------- include/trace/events/ext4.h | 61 ++++++++++++++++++++++++++++++ 3 files changed, 135 insertions(+), 15 deletions(-) diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index 927173bc8381..dd09d00a73af 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -1027,6 +1027,21 @@ enum { struct ext4_fc_inode_snap; +/* + * Snapshot failure reasons for ext4_fc_lock_updates tracepoint. + * Keep these stable for tooling. + */ +enum ext4_fc_snap_err { + EXT4_FC_SNAP_ERR_NONE = 0, + EXT4_FC_SNAP_ERR_ES_MISS, + EXT4_FC_SNAP_ERR_ES_DELAYED, + EXT4_FC_SNAP_ERR_ES_OTHER, + EXT4_FC_SNAP_ERR_INODES_CAP, + EXT4_FC_SNAP_ERR_RANGES_CAP, + EXT4_FC_SNAP_ERR_NOMEM, + EXT4_FC_SNAP_ERR_INODE_LOC, +}; + /* * fourth extended file system inode data in memory */ diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c index 9e73c83b0e25..dc08f8ff43d9 100644 --- a/fs/ext4/fast_commit.c +++ b/fs/ext4/fast_commit.c @@ -194,6 +194,12 @@ static struct kmem_cache *ext4_fc_range_cachep; #define EXT4_FC_SNAPSHOT_MAX_INODES 1024 #define EXT4_FC_SNAPSHOT_MAX_RANGES 2048 +static inline void ext4_fc_set_snap_err(int *snap_err, int err) +{ + if (snap_err && *snap_err == EXT4_FC_SNAP_ERR_NONE) + *snap_err = err; +} + static void ext4_end_buffer_io_sync(struct buffer_head *bh, int uptodate) { BUFFER_TRACE(bh, ""); @@ -968,11 +974,12 @@ static void ext4_fc_free_inode_snap(struct inode *inode) static int ext4_fc_snapshot_inode_data(struct inode *inode, struct list_head *ranges, unsigned int nr_ranges_total, - unsigned int *nr_rangesp) + unsigned int *nr_rangesp, + int *snap_err) { struct ext4_inode_info *ei = EXT4_I(inode); - unsigned int nr_ranges = 0; ext4_lblk_t start_lblk, end_lblk, cur_lblk; + unsigned int nr_ranges = 0; spin_lock(&ei->i_fc_lock); if (ei->i_fc_lblk_len == 0) { @@ -997,11 +1004,16 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode, ext4_lblk_t len; u64 remaining = (u64)end_lblk - cur_lblk + 1; - if (!ext4_es_lookup_extent(inode, cur_lblk, NULL, &es, NULL)) + if (!ext4_es_lookup_extent(inode, cur_lblk, NULL, &es, NULL)) { + ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_ES_MISS); return -EAGAIN; + } - if (ext4_es_is_delayed(&es)) + if (ext4_es_is_delayed(&es)) { + ext4_fc_set_snap_err(snap_err, + EXT4_FC_SNAP_ERR_ES_DELAYED); return -EAGAIN; + } len = es.es_len - (cur_lblk - es.es_lblk); if (len > remaining) @@ -1011,12 +1023,17 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode, continue; } - if (nr_ranges_total + nr_ranges >= EXT4_FC_SNAPSHOT_MAX_RANGES) + if (nr_ranges_total + nr_ranges >= EXT4_FC_SNAPSHOT_MAX_RANGES) { + ext4_fc_set_snap_err(snap_err, + EXT4_FC_SNAP_ERR_RANGES_CAP); return -E2BIG; + } range = kmem_cache_alloc(ext4_fc_range_cachep, GFP_NOFS); - if (!range) + if (!range) { + ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_NOMEM); return -ENOMEM; + } nr_ranges++; range->lblk = cur_lblk; @@ -1041,6 +1058,7 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode, range->len = max; } else { kmem_cache_free(ext4_fc_range_cachep, range); + ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_ES_OTHER); return -EAGAIN; } @@ -1060,7 +1078,7 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode, static int ext4_fc_snapshot_inode(struct inode *inode, unsigned int nr_ranges_total, - unsigned int *nr_rangesp) + unsigned int *nr_rangesp, int *snap_err) { struct ext4_inode_info *ei = EXT4_I(inode); struct ext4_fc_inode_snap *snap; @@ -1072,8 +1090,10 @@ static int ext4_fc_snapshot_inode(struct inode *inode, int alloc_ctx; ret = ext4_get_inode_loc_noio(inode, &iloc); - if (ret) + if (ret) { + ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_INODE_LOC); return ret; + } if (ext4_test_inode_flag(inode, EXT4_INODE_INLINE_DATA)) inode_len = EXT4_INODE_SIZE(inode->i_sb); @@ -1082,6 +1102,7 @@ static int ext4_fc_snapshot_inode(struct inode *inode, snap = kmalloc(struct_size(snap, inode_buf, inode_len), GFP_NOFS); if (!snap) { + ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_NOMEM); brelse(iloc.bh); return -ENOMEM; } @@ -1092,7 +1113,7 @@ static int ext4_fc_snapshot_inode(struct inode *inode, brelse(iloc.bh); ret = ext4_fc_snapshot_inode_data(inode, &ranges, nr_ranges_total, - &nr_ranges); + &nr_ranges, snap_err); if (ret) { kfree(snap); ext4_fc_free_ranges(&ranges); @@ -1193,7 +1214,10 @@ static int ext4_fc_alloc_snapshot_inodes(struct super_block *sb, unsigned int *nr_inodesp); static int ext4_fc_snapshot_inodes(journal_t *journal, struct inode **inodes, - unsigned int inodes_size) + unsigned int inodes_size, + unsigned int *nr_inodesp, + unsigned int *nr_rangesp, + int *snap_err) { struct super_block *sb = journal->j_private; struct ext4_sb_info *sbi = EXT4_SB(sb); @@ -1211,6 +1235,8 @@ static int ext4_fc_snapshot_inodes(journal_t *journal, struct inode **inodes, alloc_ctx = ext4_fc_lock(sb); list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) { if (i >= inodes_size) { + ext4_fc_set_snap_err(snap_err, + EXT4_FC_SNAP_ERR_INODES_CAP); ret = -E2BIG; goto unlock; } @@ -1234,6 +1260,8 @@ static int ext4_fc_snapshot_inodes(journal_t *journal, struct inode **inodes, continue; if (i >= inodes_size) { + ext4_fc_set_snap_err(snap_err, + EXT4_FC_SNAP_ERR_INODES_CAP); ret = -E2BIG; goto unlock; } @@ -1258,16 +1286,20 @@ static int ext4_fc_snapshot_inodes(journal_t *journal, struct inode **inodes, unsigned int inode_ranges = 0; ret = ext4_fc_snapshot_inode(inodes[idx], nr_ranges, - &inode_ranges); + &inode_ranges, snap_err); if (ret) break; nr_ranges += inode_ranges; } + if (nr_inodesp) + *nr_inodesp = idx; + if (nr_rangesp) + *nr_rangesp = nr_ranges; return ret; } -static int ext4_fc_perform_commit(journal_t *journal) +static int ext4_fc_perform_commit(journal_t *journal, tid_t commit_tid) { struct super_block *sb = journal->j_private; struct ext4_sb_info *sbi = EXT4_SB(sb); @@ -1276,10 +1308,15 @@ static int ext4_fc_perform_commit(journal_t *journal) struct inode *inode; struct inode **inodes; unsigned int inodes_size; + unsigned int snap_inodes = 0; + unsigned int snap_ranges = 0; + int snap_err = EXT4_FC_SNAP_ERR_NONE; struct blk_plug plug; int ret = 0; u32 crc = 0; int alloc_ctx; + ktime_t lock_start; + u64 locked_ns; /* * Step 1: Mark all inodes on s_fc_q[MAIN] with @@ -1324,13 +1361,13 @@ static int ext4_fc_perform_commit(journal_t *journal) if (ret) return ret; - ret = ext4_fc_alloc_snapshot_inodes(sb, &inodes, &inodes_size); if (ret) return ret; /* Step 4: Mark all inodes as being committed. */ jbd2_journal_lock_updates(journal); + lock_start = ktime_get(); /* * The journal is now locked. No more handles can start and all the * previous handles are now drained. Snapshotting happens in this @@ -1344,8 +1381,15 @@ static int ext4_fc_perform_commit(journal_t *journal) } ext4_fc_unlock(sb, alloc_ctx); - ret = ext4_fc_snapshot_inodes(journal, inodes, inodes_size); + ret = ext4_fc_snapshot_inodes(journal, inodes, inodes_size, + &snap_inodes, &snap_ranges, &snap_err); jbd2_journal_unlock_updates(journal); + if (trace_ext4_fc_lock_updates_enabled()) { + locked_ns = ktime_to_ns(ktime_sub(ktime_get(), lock_start)); + trace_call__ext4_fc_lock_updates(sb, commit_tid, locked_ns, + snap_inodes, snap_ranges, + ret, snap_err); + } kvfree(inodes); if (ret) return ret; @@ -1550,7 +1594,7 @@ int ext4_fc_commit(journal_t *journal, tid_t commit_tid) journal_ioprio = EXT4_DEF_JOURNAL_IOPRIO; set_task_ioprio(current, journal_ioprio); fc_bufs_before = (sbi->s_fc_bytes + bsize - 1) / bsize; - ret = ext4_fc_perform_commit(journal); + ret = ext4_fc_perform_commit(journal, commit_tid); if (ret < 0) { if (ret == -EAGAIN || ret == -E2BIG || ret == -ECANCELED) status = EXT4_FC_STATUS_INELIGIBLE; diff --git a/include/trace/events/ext4.h b/include/trace/events/ext4.h index f493642cf121..7028a28316fa 100644 --- a/include/trace/events/ext4.h +++ b/include/trace/events/ext4.h @@ -107,6 +107,26 @@ TRACE_DEFINE_ENUM(EXT4_FC_REASON_VERITY); TRACE_DEFINE_ENUM(EXT4_FC_REASON_MOVE_EXT); TRACE_DEFINE_ENUM(EXT4_FC_REASON_MAX); +#undef EM +#undef EMe +#define EM(a) TRACE_DEFINE_ENUM(EXT4_FC_SNAP_ERR_##a); +#define EMe(a) TRACE_DEFINE_ENUM(EXT4_FC_SNAP_ERR_##a); + +#define TRACE_SNAP_ERR \ + EM(NONE) \ + EM(ES_MISS) \ + EM(ES_DELAYED) \ + EM(ES_OTHER) \ + EM(INODES_CAP) \ + EM(RANGES_CAP) \ + EM(NOMEM) \ + EMe(INODE_LOC) + +TRACE_SNAP_ERR + +#undef EM +#undef EMe + #define show_fc_reason(reason) \ __print_symbolic(reason, \ { EXT4_FC_REASON_XATTR, "XATTR"}, \ @@ -2818,6 +2838,47 @@ TRACE_EVENT(ext4_fc_commit_stop, __entry->num_fc_ineligible, __entry->nblks_agg, __entry->tid) ); +#define EM(a) { EXT4_FC_SNAP_ERR_##a, #a }, +#define EMe(a) { EXT4_FC_SNAP_ERR_##a, #a } + +TRACE_EVENT(ext4_fc_lock_updates, + TP_PROTO(struct super_block *sb, tid_t commit_tid, u64 locked_ns, + unsigned int nr_inodes, unsigned int nr_ranges, int err, + int snap_err), + + TP_ARGS(sb, commit_tid, locked_ns, nr_inodes, nr_ranges, err, snap_err), + + TP_STRUCT__entry(/* entry */ + __field(dev_t, dev) + __field(tid_t, tid) + __field(u64, locked_ns) + __field(unsigned int, nr_inodes) + __field(unsigned int, nr_ranges) + __field(int, err) + __field(int, snap_err) + ), + + TP_fast_assign(/* assign */ + __entry->dev = sb->s_dev; + __entry->tid = commit_tid; + __entry->locked_ns = locked_ns; + __entry->nr_inodes = nr_inodes; + __entry->nr_ranges = nr_ranges; + __entry->err = err; + __entry->snap_err = snap_err; + ), + + TP_printk("dev %d,%d tid %u locked_ns %llu nr_inodes %u nr_ranges %u err %d snap_err %s", + MAJOR(__entry->dev), MINOR(__entry->dev), __entry->tid, + __entry->locked_ns, __entry->nr_inodes, __entry->nr_ranges, + __entry->err, __print_symbolic(__entry->snap_err, + TRACE_SNAP_ERR)) +); + +#undef EM +#undef EMe +#undef TRACE_SNAP_ERR + #define FC_REASON_NAME_STAT(reason) \ show_fc_reason(reason), \ __entry->fc_ineligible_rc[reason] From 56bb0b64f4b198bad5ce674509c10793d471148f Mon Sep 17 00:00:00 2001 From: Li Chen Date: Fri, 15 May 2026 17:18:27 +0800 Subject: [PATCH 13/20] ext4: fast commit: export snapshot stats in fc_info Snapshot-based fast commit can fall back when the commit-time snapshot cannot be built (e.g. extent status cache misses). It is useful to quantify the updates-locked window and to see why snapshotting failed. Add best-effort snapshot counters to the ext4 superblock and extend /proc/fs/ext4//fc_info to report the number of snapshotted inodes and ranges, snapshot failure reasons, and the average/max time spent with journal updates locked. Signed-off-by: Li Chen Link: https://patch.msgid.link/20260515091829.194810-8-me@linux.beauty Signed-off-by: Theodore Ts'o --- fs/ext4/ext4.h | 31 ++++++++++++++ fs/ext4/fast_commit.c | 96 ++++++++++++++++++++++++++++++++++++++----- fs/ext4/super.c | 1 + 3 files changed, 118 insertions(+), 10 deletions(-) diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index dd09d00a73af..ddc903738c6b 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -1550,6 +1550,36 @@ struct ext4_orphan_info { * file blocks */ }; +/* + * Ext4 fast commit snapshot statistics. + * + * These are best-effort counters intended for debugging / performance + * introspection; they are not exact under concurrent updates. + */ +struct ext4_fc_snap_stats { + atomic64_t lock_updates_ns_total; + atomic64_t lock_updates_ns_max; + atomic64_t lock_updates_samples; + + atomic64_t snap_inodes; + atomic64_t snap_ranges; + + atomic64_t snap_fail_es_miss; + atomic64_t snap_fail_es_delayed; + atomic64_t snap_fail_es_other; + + atomic64_t snap_fail_inodes_cap; + atomic64_t snap_fail_ranges_cap; + atomic64_t snap_fail_nomem; + atomic64_t snap_fail_inode_loc; + + /* + * Missing inode snapshots during log writing should never happen. + * Keep this counter to help catch unexpected regressions. + */ + atomic64_t snap_fail_no_snap; +}; + /* * fourth extended-fs super-block data in memory */ @@ -1824,6 +1854,7 @@ struct ext4_sb_info { struct mutex s_fc_lock; struct buffer_head *s_fc_bh; struct ext4_fc_stats s_fc_stats; + struct ext4_fc_snap_stats s_fc_snap_stats; tid_t s_fc_ineligible_tid; #ifdef CONFIG_EXT4_DEBUG int s_fc_debug_max_replay; diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c index dc08f8ff43d9..4ef796b9b6cb 100644 --- a/fs/ext4/fast_commit.c +++ b/fs/ext4/fast_commit.c @@ -281,6 +281,19 @@ static inline void ext4_fc_wake_inode_state(struct inode *inode, int bit) ext4_inode_state_wait_bit(bit)); } +static void ext4_fc_snap_stats_update_max(atomic64_t *stat, u64 value) +{ + u64 old = atomic64_read(stat); + + while (value > old) { + u64 prev = atomic64_cmpxchg(stat, old, value); + + if (prev == old) + break; + old = prev; + } +} + /* * Remove inode from fast commit list. If the inode is being committed * we wait until inode commit is done. @@ -868,6 +881,8 @@ static int ext4_fc_write_inode(struct inode *inode, u32 *crc) { struct ext4_inode_info *ei = EXT4_I(inode); struct ext4_fc_inode_snap *snap = ei->i_fc_snap; + struct ext4_fc_snap_stats *stats = + &EXT4_SB(inode->i_sb)->s_fc_snap_stats; struct ext4_fc_inode fc_inode; struct ext4_fc_tl tl; u8 *dst; @@ -875,13 +890,17 @@ static int ext4_fc_write_inode(struct inode *inode, u32 *crc) int inode_len; int ret; - if (!snap) + if (!snap) { + atomic64_inc(&stats->snap_fail_no_snap); return -ECANCELED; + } src = snap->inode_buf; inode_len = snap->inode_len; - if (!src || inode_len == 0) + if (!src || inode_len == 0) { + atomic64_inc(&stats->snap_fail_no_snap); return -ECANCELED; + } fc_inode.fc_ino = cpu_to_le32(inode->i_ino); tl.fc_tag = cpu_to_le16(EXT4_FC_TAG_INODE); @@ -911,13 +930,17 @@ static int ext4_fc_write_inode_data(struct inode *inode, u32 *crc) { struct ext4_inode_info *ei = EXT4_I(inode); struct ext4_fc_inode_snap *snap = ei->i_fc_snap; + struct ext4_fc_snap_stats *stats = + &EXT4_SB(inode->i_sb)->s_fc_snap_stats; struct ext4_fc_add_range fc_ext; struct ext4_fc_del_range lrange; struct ext4_extent *ex; struct ext4_fc_range *range; - if (!snap) + if (!snap) { + atomic64_inc(&stats->snap_fail_no_snap); return -ECANCELED; + } list_for_each_entry(range, &snap->data_list, list) { if (range->tag == EXT4_FC_TAG_DEL_RANGE) { @@ -978,6 +1001,8 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode, int *snap_err) { struct ext4_inode_info *ei = EXT4_I(inode); + struct ext4_fc_snap_stats *stats = + &EXT4_SB(inode->i_sb)->s_fc_snap_stats; ext4_lblk_t start_lblk, end_lblk, cur_lblk; unsigned int nr_ranges = 0; @@ -1005,11 +1030,13 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode, u64 remaining = (u64)end_lblk - cur_lblk + 1; if (!ext4_es_lookup_extent(inode, cur_lblk, NULL, &es, NULL)) { + atomic64_inc(&stats->snap_fail_es_miss); ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_ES_MISS); return -EAGAIN; } if (ext4_es_is_delayed(&es)) { + atomic64_inc(&stats->snap_fail_es_delayed); ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_ES_DELAYED); return -EAGAIN; @@ -1024,6 +1051,7 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode, } if (nr_ranges_total + nr_ranges >= EXT4_FC_SNAPSHOT_MAX_RANGES) { + atomic64_inc(&stats->snap_fail_ranges_cap); ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_RANGES_CAP); return -E2BIG; @@ -1031,6 +1059,7 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode, range = kmem_cache_alloc(ext4_fc_range_cachep, GFP_NOFS); if (!range) { + atomic64_inc(&stats->snap_fail_nomem); ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_NOMEM); return -ENOMEM; } @@ -1058,6 +1087,7 @@ static int ext4_fc_snapshot_inode_data(struct inode *inode, range->len = max; } else { kmem_cache_free(ext4_fc_range_cachep, range); + atomic64_inc(&stats->snap_fail_es_other); ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_ES_OTHER); return -EAGAIN; } @@ -1081,6 +1111,8 @@ static int ext4_fc_snapshot_inode(struct inode *inode, unsigned int *nr_rangesp, int *snap_err) { struct ext4_inode_info *ei = EXT4_I(inode); + struct ext4_fc_snap_stats *stats = + &EXT4_SB(inode->i_sb)->s_fc_snap_stats; struct ext4_fc_inode_snap *snap; int inode_len = EXT4_GOOD_OLD_INODE_SIZE; struct ext4_iloc iloc; @@ -1091,6 +1123,7 @@ static int ext4_fc_snapshot_inode(struct inode *inode, ret = ext4_get_inode_loc_noio(inode, &iloc); if (ret) { + atomic64_inc(&stats->snap_fail_inode_loc); ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_INODE_LOC); return ret; } @@ -1102,6 +1135,7 @@ static int ext4_fc_snapshot_inode(struct inode *inode, snap = kmalloc(struct_size(snap, inode_buf, inode_len), GFP_NOFS); if (!snap) { + atomic64_inc(&stats->snap_fail_nomem); ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_NOMEM); brelse(iloc.bh); return -ENOMEM; @@ -1126,6 +1160,8 @@ static int ext4_fc_snapshot_inode(struct inode *inode, list_splice_tail_init(&ranges, &snap->data_list); ext4_fc_unlock(inode->i_sb, alloc_ctx); + atomic64_inc(&stats->snap_inodes); + atomic64_add(nr_ranges, &stats->snap_ranges); if (nr_rangesp) *nr_rangesp = nr_ranges; return 0; @@ -1229,12 +1265,10 @@ static int ext4_fc_snapshot_inodes(journal_t *journal, struct inode **inodes, int ret = 0; int alloc_ctx; - if (!inodes_size) - return 0; - alloc_ctx = ext4_fc_lock(sb); list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) { if (i >= inodes_size) { + atomic64_inc(&sbi->s_fc_snap_stats.snap_fail_inodes_cap); ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_INODES_CAP); ret = -E2BIG; @@ -1260,6 +1294,7 @@ static int ext4_fc_snapshot_inodes(journal_t *journal, struct inode **inodes, continue; if (i >= inodes_size) { + atomic64_inc(&sbi->s_fc_snap_stats.snap_fail_inodes_cap); ext4_fc_set_snap_err(snap_err, EXT4_FC_SNAP_ERR_INODES_CAP); ret = -E2BIG; @@ -1303,6 +1338,7 @@ static int ext4_fc_perform_commit(journal_t *journal, tid_t commit_tid) { struct super_block *sb = journal->j_private; struct ext4_sb_info *sbi = EXT4_SB(sb); + struct ext4_fc_snap_stats *snap_stats = &sbi->s_fc_snap_stats; struct ext4_inode_info *iter; struct ext4_fc_head head; struct inode *inode; @@ -1362,8 +1398,13 @@ static int ext4_fc_perform_commit(journal_t *journal, tid_t commit_tid) return ret; ret = ext4_fc_alloc_snapshot_inodes(sb, &inodes, &inodes_size); - if (ret) + if (ret) { + if (ret == -E2BIG) + atomic64_inc(&snap_stats->snap_fail_inodes_cap); + else if (ret == -ENOMEM) + atomic64_inc(&snap_stats->snap_fail_nomem); return ret; + } /* Step 4: Mark all inodes as being committed. */ jbd2_journal_lock_updates(journal); @@ -1384,12 +1425,15 @@ static int ext4_fc_perform_commit(journal_t *journal, tid_t commit_tid) ret = ext4_fc_snapshot_inodes(journal, inodes, inodes_size, &snap_inodes, &snap_ranges, &snap_err); jbd2_journal_unlock_updates(journal); - if (trace_ext4_fc_lock_updates_enabled()) { - locked_ns = ktime_to_ns(ktime_sub(ktime_get(), lock_start)); + locked_ns = ktime_to_ns(ktime_sub(ktime_get(), lock_start)); + atomic64_add(locked_ns, &snap_stats->lock_updates_ns_total); + atomic64_inc(&snap_stats->lock_updates_samples); + ext4_fc_snap_stats_update_max(&snap_stats->lock_updates_ns_max, + locked_ns); + if (trace_ext4_fc_lock_updates_enabled()) trace_call__ext4_fc_lock_updates(sb, commit_tid, locked_ns, snap_inodes, snap_ranges, ret, snap_err); - } kvfree(inodes); if (ret) return ret; @@ -2657,11 +2701,26 @@ int ext4_fc_info_show(struct seq_file *seq, void *v) { struct ext4_sb_info *sbi = EXT4_SB((struct super_block *)seq->private); struct ext4_fc_stats *stats = &sbi->s_fc_stats; + struct ext4_fc_snap_stats *snap_stats = &sbi->s_fc_snap_stats; + u64 lock_avg_ns = 0; + u64 lock_updates_samples; + u64 lock_updates_ns_total; + u64 lock_updates_ns_max; int i; if (v != SEQ_START_TOKEN) return 0; + lock_updates_samples = + atomic64_read(&snap_stats->lock_updates_samples); + lock_updates_ns_total = + atomic64_read(&snap_stats->lock_updates_ns_total); + lock_updates_ns_max = + atomic64_read(&snap_stats->lock_updates_ns_max); + if (lock_updates_samples) + lock_avg_ns = div64_u64(lock_updates_ns_total, + lock_updates_samples); + seq_printf(seq, "fc stats:\n%ld commits\n%ld ineligible\n%ld numblks\n%lluus avg_commit_time\n", stats->fc_num_commits, stats->fc_ineligible_commits, @@ -2672,6 +2731,23 @@ int ext4_fc_info_show(struct seq_file *seq, void *v) seq_printf(seq, "\"%s\":\t%d\n", fc_ineligible_reasons[i], stats->fc_ineligible_reason_count[i]); + seq_printf(seq, + "Snapshot stats:\n%llu inodes\n%llu ranges\n%lluus lock_updates_avg\n%lluus lock_updates_max\n", + atomic64_read(&snap_stats->snap_inodes), + atomic64_read(&snap_stats->snap_ranges), + div_u64(lock_avg_ns, 1000), + div_u64(lock_updates_ns_max, 1000)); + seq_printf(seq, + "Snapshot failures:\n%llu es_miss\n%llu es_delayed\n%llu es_other\n%llu inodes_cap\n%llu ranges_cap\n%llu nomem\n%llu inode_loc\n%llu no_snap\n", + atomic64_read(&snap_stats->snap_fail_es_miss), + atomic64_read(&snap_stats->snap_fail_es_delayed), + atomic64_read(&snap_stats->snap_fail_es_other), + atomic64_read(&snap_stats->snap_fail_inodes_cap), + atomic64_read(&snap_stats->snap_fail_ranges_cap), + atomic64_read(&snap_stats->snap_fail_nomem), + atomic64_read(&snap_stats->snap_fail_inode_loc), + atomic64_read(&snap_stats->snap_fail_no_snap)); + return 0; } diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 7c484a53c62e..f2c52cc74676 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -4544,6 +4544,7 @@ static void ext4_fast_commit_init(struct super_block *sb) sbi->s_fc_ineligible_tid = 0; mutex_init(&sbi->s_fc_lock); memset(&sbi->s_fc_stats, 0, sizeof(sbi->s_fc_stats)); + memset(&sbi->s_fc_snap_stats, 0, sizeof(sbi->s_fc_snap_stats)); sbi->s_fc_replay_state.fc_regions = NULL; sbi->s_fc_replay_state.fc_regions_size = 0; sbi->s_fc_replay_state.fc_regions_used = 0; From 3147cac6c1929f26b4687993b8c7af5b7b34496d Mon Sep 17 00:00:00 2001 From: Guan-Chun Wu <409411716@gms.tku.edu.tw> Date: Sun, 31 May 2026 16:00:18 +0800 Subject: [PATCH 14/20] ext4: add Kunit coverage for directory hash computation Introduce Kunit tests for fs/ext4/hash.c to verify ext4fs_dirhash() across the legacy, half-MD4, and TEA hash variants. The tests cover empty, seeded hashing, and non-ASCII name handling. They also verify error paths, including invalid hash versions and SipHash without a configured key, and check that the signed and unsigned hash variants differ on non-ASCII input as expected. When CONFIG_UNICODE is enabled, the tests further verify casefolded-name hashing and the fallback behavior for invalid input. Co-developed-by: Chen Hao Yu Signed-off-by: Chen Hao Yu Signed-off-by: Guan-Chun Wu <409411716@gms.tku.edu.tw> Link: https://patch.msgid.link/20260531080019.3794809-2-409411716@gms.tku.edu.tw Signed-off-by: Theodore Ts'o --- fs/ext4/Makefile | 2 +- fs/ext4/hash-test.c | 567 ++++++++++++++++++++++++++++++++++++++++++++ fs/ext4/hash.c | 4 + 3 files changed, 572 insertions(+), 1 deletion(-) create mode 100644 fs/ext4/hash-test.c diff --git a/fs/ext4/Makefile b/fs/ext4/Makefile index 3baee4e7c1cf..3f9fc0eb8eca 100644 --- a/fs/ext4/Makefile +++ b/fs/ext4/Makefile @@ -15,7 +15,7 @@ ext4-y := balloc.o bitmap.o block_validity.o dir.o ext4_jbd2.o extents.o \ ext4-$(CONFIG_EXT4_FS_POSIX_ACL) += acl.o ext4-$(CONFIG_EXT4_FS_SECURITY) += xattr_security.o ext4-test-objs += inode-test.o mballoc-test.o \ - extents-test.o + extents-test.o hash-test.o obj-$(CONFIG_EXT4_KUNIT_TESTS) += ext4-test.o ext4-$(CONFIG_FS_VERITY) += verity.o ext4-$(CONFIG_FS_ENCRYPTION) += crypto.o diff --git a/fs/ext4/hash-test.c b/fs/ext4/hash-test.c new file mode 100644 index 000000000000..49b0d874c833 --- /dev/null +++ b/fs/ext4/hash-test.c @@ -0,0 +1,567 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * KUnit tests for ext4 directory hash computation. + */ + +#include +#include +#include +#include +#include +#include +#include "ext4.h" + +static void ext4_hash_init_fake_dir(struct inode *dir, struct super_block *sb) +{ + memset(sb, 0, sizeof(*sb)); + memset(dir, 0, sizeof(*dir)); + dir->i_sb = sb; + strscpy(sb->s_id, "kunit-ext4", sizeof(sb->s_id)); +} + +static void ext4_hash_init_fake_dir_with_sbi(struct inode *dir, + struct super_block *sb, + struct ext4_sb_info *sbi) +{ + ext4_hash_init_fake_dir(dir, sb); + memset(sbi, 0, sizeof(*sbi)); + sb->s_fs_info = sbi; + sbi->s_sb = sb; +} + +#ifdef CONFIG_FS_ENCRYPTION +static const struct fscrypt_operations ext4_hash_test_cryptops = { + .inode_info_offs = + (int)offsetof(struct ext4_inode_info, i_crypt_info) - + (int)offsetof(struct ext4_inode_info, vfs_inode), +}; +#endif + +static void ext4_hash_init_fake_ext4_dir(struct ext4_inode_info *ei, + struct super_block *sb, + struct ext4_sb_info *sbi) +{ + struct inode *dir = &ei->vfs_inode; + + memset(sb, 0, sizeof(*sb)); + memset(ei, 0, sizeof(*ei)); + memset(sbi, 0, sizeof(*sbi)); + + strscpy(sb->s_id, "kunit-ext4", sizeof(sb->s_id)); + sb->s_fs_info = sbi; + sbi->s_sb = sb; + + dir->i_sb = sb; + dir->i_mode = S_IFDIR; + +#ifdef CONFIG_FS_ENCRYPTION + fscrypt_set_ops(sb, &ext4_hash_test_cryptops); +#endif +} + +struct ext4_dirhash_test_case { + const char *name; + u32 hash_version; + const char *input; + int len; + u32 seed[4]; + bool use_seed; + u32 expected_hash; + u32 expected_minor_hash; +}; + +static const struct ext4_dirhash_test_case ext4_dirhash_test_cases[] = { + { + .name = "legacy_abc", + .hash_version = DX_HASH_LEGACY, + .input = "abc", + .len = 3, + .use_seed = false, + .expected_hash = 0x75afd992, + .expected_minor_hash = 0x00000000, + }, + { + .name = "legacy_unsigned_abc", + .hash_version = DX_HASH_LEGACY_UNSIGNED, + .input = "abc", + .len = 3, + .use_seed = false, + .expected_hash = 0x75afd992, + .expected_minor_hash = 0x00000000, + }, + { + .name = "half_md4_abc", + .hash_version = DX_HASH_HALF_MD4, + .input = "abc", + .len = 3, + .use_seed = false, + .expected_hash = 0xd196a868, + .expected_minor_hash = 0xc420eb28, + }, + { + .name = "half_md4_unsigned_abc", + .hash_version = DX_HASH_HALF_MD4_UNSIGNED, + .input = "abc", + .len = 3, + .use_seed = false, + .expected_hash = 0xd196a868, + .expected_minor_hash = 0xc420eb28, + }, + { + .name = "tea_abc", + .hash_version = DX_HASH_TEA, + .input = "abc", + .len = 3, + .use_seed = false, + .expected_hash = 0xb1435ec4, + .expected_minor_hash = 0x3f7eaa0e, + }, + { + .name = "tea_unsigned_abc", + .hash_version = DX_HASH_TEA_UNSIGNED, + .input = "abc", + .len = 3, + .use_seed = false, + .expected_hash = 0xb1435ec4, + .expected_minor_hash = 0x3f7eaa0e, + }, + { + .name = "empty_half_md4", + .hash_version = DX_HASH_HALF_MD4, + .input = "", + .len = 0, + .use_seed = false, + .expected_hash = 0xefcdab88, + .expected_minor_hash = 0x98badcfe, + }, + { + .name = "half_md4_31bytes", + .hash_version = DX_HASH_HALF_MD4, + .input = "1234567890123456789012345678901", + .len = 31, + .use_seed = false, + .expected_hash = 0xc4db1f78, + .expected_minor_hash = 0xea23921b, + }, + { + .name = "half_md4_32bytes", + .hash_version = DX_HASH_HALF_MD4, + .input = "12345678901234567890123456789012", + .len = 32, + .use_seed = false, + .expected_hash = 0xfa6cc63e, + .expected_minor_hash = 0x2f77bd1c, + }, + { + .name = "half_md4_33bytes", + .hash_version = DX_HASH_HALF_MD4, + .input = "123456789012345678901234567890123", + .len = 33, + .use_seed = false, + .expected_hash = 0xdc0c2dec, + .expected_minor_hash = 0x5ca23365, + }, + { + .name = "half_md4_unsigned_31bytes", + .hash_version = DX_HASH_HALF_MD4_UNSIGNED, + .input = "1234567890123456789012345678901", + .len = 31, + .use_seed = false, + .expected_hash = 0xc4db1f78, + .expected_minor_hash = 0xea23921b, + }, + { + .name = "half_md4_unsigned_32bytes", + .hash_version = DX_HASH_HALF_MD4_UNSIGNED, + .input = "12345678901234567890123456789012", + .len = 32, + .use_seed = false, + .expected_hash = 0xfa6cc63e, + .expected_minor_hash = 0x2f77bd1c, + }, + { + .name = "half_md4_unsigned_33bytes", + .hash_version = DX_HASH_HALF_MD4_UNSIGNED, + .input = "123456789012345678901234567890123", + .len = 33, + .use_seed = false, + .expected_hash = 0xdc0c2dec, + .expected_minor_hash = 0x5ca23365, + }, + { + .name = "tea_15bytes", + .hash_version = DX_HASH_TEA, + .input = "123456789abcdef", + .len = 15, + .use_seed = false, + .expected_hash = 0xa562903a, + .expected_minor_hash = 0x6174a00f, + }, + { + .name = "tea_16bytes", + .hash_version = DX_HASH_TEA, + .input = "1234567890abcdef", + .len = 16, + .use_seed = false, + .expected_hash = 0x8449f258, + .expected_minor_hash = 0x49a16d46, + }, + { + .name = "tea_17bytes", + .hash_version = DX_HASH_TEA, + .input = "123456789abcdefgh", + .len = 17, + .use_seed = false, + .expected_hash = 0xf32ec10c, + .expected_minor_hash = 0x58ceae61, + }, + { + .name = "half_md4_seeded", + .hash_version = DX_HASH_HALF_MD4, + .input = "same-name", + .len = 9, + .seed = { 0x11111111, 0x22222222, 0x33333333, 0x44444444 }, + .use_seed = true, + .expected_hash = 0x8aebf604, + .expected_minor_hash = 0x66ce48fe, + }, + { + .name = "half_md4_non_ascii_signed", + .hash_version = DX_HASH_HALF_MD4, + .input = "\x80\x81\x82\x83\x84", + .len = 5, + .use_seed = false, + .expected_hash = 0x8bab0498, + .expected_minor_hash = 0xc326632d, + }, + { + .name = "half_md4_non_ascii_unsigned", + .hash_version = DX_HASH_HALF_MD4_UNSIGNED, + .input = "\x80\x81\x82\x83\x84", + .len = 5, + .use_seed = false, + .expected_hash = 0xbc48596e, + .expected_minor_hash = 0xde0fad41, + }, + { + .name = "tea_non_ascii_signed", + .hash_version = DX_HASH_TEA, + .input = "\x80\x81\x82\x83\x84", + .len = 5, + .use_seed = false, + .expected_hash = 0x21e3a154, + .expected_minor_hash = 0x90112c3d, + }, + { + .name = "tea_non_ascii_unsigned", + .hash_version = DX_HASH_TEA_UNSIGNED, + .input = "\x80\x81\x82\x83\x84", + .len = 5, + .use_seed = false, + .expected_hash = 0x9b648616, + .expected_minor_hash = 0x011dd507, + }, +}; + +static void test_ext4fs_dirhash_vectors(struct kunit *test) +{ + struct super_block *sb; + struct inode *dir; + int i; + + sb = kunit_kzalloc(test, sizeof(*sb), GFP_KERNEL); + dir = kunit_kzalloc(test, sizeof(*dir), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, sb); + KUNIT_ASSERT_NOT_NULL(test, dir); + + ext4_hash_init_fake_dir(dir, sb); + + for (i = 0; i < ARRAY_SIZE(ext4_dirhash_test_cases); i++) { + const struct ext4_dirhash_test_case *tc = + &ext4_dirhash_test_cases[i]; + struct dx_hash_info hinfo; + int ret; + + memset(&hinfo, 0, sizeof(hinfo)); + hinfo.hash_version = tc->hash_version; + hinfo.seed = tc->use_seed ? (u32 *)tc->seed : NULL; + + ret = ext4fs_dirhash(dir, tc->input, tc->len, &hinfo); + + KUNIT_ASSERT_EQ_MSG(test, ret, 0, "case=%s", tc->name); + KUNIT_EXPECT_EQ_MSG(test, hinfo.hash, tc->expected_hash, + "case=%s", tc->name); + KUNIT_EXPECT_EQ_MSG(test, hinfo.minor_hash, + tc->expected_minor_hash, + "case=%s", tc->name); + } +} + +static void test_ext4fs_dirhash_seed_changes_result(struct kunit *test) +{ + struct super_block *sb; + struct inode *dir; + u32 seed[4] = { 0x11111111, 0x22222222, 0x33333333, 0x44444444 }; + struct dx_hash_info plain = { + .hash_version = DX_HASH_HALF_MD4, + }; + struct dx_hash_info seeded = { + .hash_version = DX_HASH_HALF_MD4, + .seed = seed, + }; + int ret_plain, ret_seeded; + + sb = kunit_kzalloc(test, sizeof(*sb), GFP_KERNEL); + dir = kunit_kzalloc(test, sizeof(*dir), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, sb); + KUNIT_ASSERT_NOT_NULL(test, dir); + + ext4_hash_init_fake_dir(dir, sb); + + ret_plain = ext4fs_dirhash(dir, "same-name", 9, &plain); + ret_seeded = ext4fs_dirhash(dir, "same-name", 9, &seeded); + + KUNIT_ASSERT_EQ(test, ret_plain, 0); + KUNIT_ASSERT_EQ(test, ret_seeded, 0); + + KUNIT_EXPECT_TRUE(test, + plain.hash != seeded.hash || + plain.minor_hash != seeded.minor_hash); +} + +static void test_ext4fs_dirhash_invalid_version_returns_einval(struct kunit *test) +{ + struct super_block *sb; + struct inode *dir; + struct ext4_sb_info *sbi; + struct dx_hash_info hinfo = { + .hash = 0xdeadbeef, + .minor_hash = 0xcafebabe, + .hash_version = DX_HASH_LAST + 1, + }; + int ret; + + sb = kunit_kzalloc(test, sizeof(*sb), GFP_KERNEL); + dir = kunit_kzalloc(test, sizeof(*dir), GFP_KERNEL); + sbi = kunit_kzalloc(test, sizeof(*sbi), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, sb); + KUNIT_ASSERT_NOT_NULL(test, dir); + KUNIT_ASSERT_NOT_NULL(test, sbi); + + ext4_hash_init_fake_dir_with_sbi(dir, sb, sbi); + + ret = ext4fs_dirhash(dir, "abc", 3, &hinfo); + + KUNIT_EXPECT_EQ(test, ret, -EINVAL); + KUNIT_EXPECT_EQ(test, hinfo.hash, 0); + KUNIT_EXPECT_EQ(test, hinfo.minor_hash, 0); +} + +static void test_ext4fs_dirhash_siphash_without_key_returns_einval(struct kunit *test) +{ + struct super_block *sb; + struct ext4_inode_info *ei; + struct inode *dir; + struct ext4_sb_info *sbi; + struct dx_hash_info hinfo = { + .hash_version = DX_HASH_SIPHASH, + }; + int ret; + + sb = kunit_kzalloc(test, sizeof(*sb), GFP_KERNEL); + ei = kunit_kzalloc(test, sizeof(*ei), GFP_KERNEL); + sbi = kunit_kzalloc(test, sizeof(*sbi), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, sb); + KUNIT_ASSERT_NOT_NULL(test, ei); + KUNIT_ASSERT_NOT_NULL(test, sbi); + + ext4_hash_init_fake_ext4_dir(ei, sb, sbi); + dir = &ei->vfs_inode; + + ret = ext4fs_dirhash(dir, "name", strlen("name"), &hinfo); + + KUNIT_EXPECT_EQ(test, ret, -EINVAL); +} + +static void test_ext4fs_dirhash_signed_unsigned_differ_on_nonascii(struct kunit *test) +{ + struct super_block *sb; + struct inode *dir; + static const char input[] = "\x80\xff\x81\xfe\101bc"; + struct dx_hash_info legacy_signed = { + .hash_version = DX_HASH_LEGACY, + }; + struct dx_hash_info legacy_unsigned = { + .hash_version = DX_HASH_LEGACY_UNSIGNED, + }; + struct dx_hash_info md4_signed = { + .hash_version = DX_HASH_HALF_MD4, + }; + struct dx_hash_info md4_unsigned = { + .hash_version = DX_HASH_HALF_MD4_UNSIGNED, + }; + struct dx_hash_info tea_signed = { + .hash_version = DX_HASH_TEA, + }; + struct dx_hash_info tea_unsigned = { + .hash_version = DX_HASH_TEA_UNSIGNED, + }; + int ret; + + sb = kunit_kzalloc(test, sizeof(*sb), GFP_KERNEL); + dir = kunit_kzalloc(test, sizeof(*dir), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, sb); + KUNIT_ASSERT_NOT_NULL(test, dir); + + ext4_hash_init_fake_dir(dir, sb); + + ret = ext4fs_dirhash(dir, input, sizeof(input) - 1, &legacy_signed); + KUNIT_ASSERT_EQ(test, ret, 0); + ret = ext4fs_dirhash(dir, input, sizeof(input) - 1, &legacy_unsigned); + KUNIT_ASSERT_EQ(test, ret, 0); + KUNIT_EXPECT_NE(test, legacy_signed.hash, legacy_unsigned.hash); + + ret = ext4fs_dirhash(dir, input, sizeof(input) - 1, &md4_signed); + KUNIT_ASSERT_EQ(test, ret, 0); + ret = ext4fs_dirhash(dir, input, sizeof(input) - 1, &md4_unsigned); + KUNIT_ASSERT_EQ(test, ret, 0); + KUNIT_EXPECT_TRUE(test, + md4_signed.hash != md4_unsigned.hash || + md4_signed.minor_hash != md4_unsigned.minor_hash); + + ret = ext4fs_dirhash(dir, input, sizeof(input) - 1, &tea_signed); + KUNIT_ASSERT_EQ(test, ret, 0); + ret = ext4fs_dirhash(dir, input, sizeof(input) - 1, &tea_unsigned); + KUNIT_ASSERT_EQ(test, ret, 0); + KUNIT_EXPECT_TRUE(test, + tea_signed.hash != tea_unsigned.hash || + tea_signed.minor_hash != tea_unsigned.minor_hash); +} + +#if IS_ENABLED(CONFIG_UNICODE) +KUNIT_DEFINE_ACTION_WRAPPER(utf8_unload_action, utf8_unload, + struct unicode_map *); +static void test_ext4fs_dirhash_casefolded_names_hash_consistently(struct kunit *test) +{ + struct super_block *sb; + struct ext4_inode_info *ei; + struct ext4_sb_info *sbi; + struct unicode_map *um; + struct dx_hash_info h1 = { + .hash_version = DX_HASH_HALF_MD4, + }; + struct dx_hash_info h2 = { + .hash_version = DX_HASH_HALF_MD4, + }; + int ret, ret1, ret2; + + sb = kunit_kzalloc(test, sizeof(*sb), GFP_KERNEL); + ei = kunit_kzalloc(test, sizeof(*ei), GFP_KERNEL); + sbi = kunit_kzalloc(test, sizeof(*sbi), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, sb); + KUNIT_ASSERT_NOT_NULL(test, ei); + KUNIT_ASSERT_NOT_NULL(test, sbi); + + um = utf8_load(UTF8_LATEST); + if (IS_ERR(um)) { + kunit_skip(test, "utf8_load(UTF8_LATEST) failed: %pe", + um); + return; + } + + ret = kunit_add_action_or_reset(test, utf8_unload_action, um); + KUNIT_ASSERT_EQ(test, ret, 0); + + ext4_hash_init_fake_ext4_dir(ei, sb, sbi); + sb->s_encoding = um; + ei->vfs_inode.i_flags |= S_CASEFOLD; + + KUNIT_ASSERT_TRUE(test, IS_CASEFOLDED(&ei->vfs_inode)); + + ret1 = ext4fs_dirhash(&ei->vfs_inode, "Alpha", 5, &h1); + ret2 = ext4fs_dirhash(&ei->vfs_inode, "aLPHa", 5, &h2); + + KUNIT_ASSERT_EQ(test, ret1, 0); + KUNIT_ASSERT_EQ(test, ret2, 0); + KUNIT_EXPECT_EQ(test, h1.hash, h2.hash); + KUNIT_EXPECT_EQ(test, h1.minor_hash, h2.minor_hash); +} + +static void test_ext4fs_dirhash_casefold_fallback(struct kunit *test) +{ + struct super_block *sb_cf, *sb_plain; + struct ext4_inode_info *ei; + struct ext4_sb_info *sbi; + struct inode *plain_dir; + struct unicode_map *um; + static const char invalid_utf8[] = "\xc3\x28"; + struct dx_hash_info folded_dir = { + .hash_version = DX_HASH_HALF_MD4, + }; + struct dx_hash_info plain = { + .hash_version = DX_HASH_HALF_MD4, + }; + int ret, ret_cf, ret_plain; + + sb_cf = kunit_kzalloc(test, sizeof(*sb_cf), GFP_KERNEL); + sb_plain = kunit_kzalloc(test, sizeof(*sb_plain), GFP_KERNEL); + ei = kunit_kzalloc(test, sizeof(*ei), GFP_KERNEL); + sbi = kunit_kzalloc(test, sizeof(*sbi), GFP_KERNEL); + plain_dir = kunit_kzalloc(test, sizeof(*plain_dir), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, sb_cf); + KUNIT_ASSERT_NOT_NULL(test, sb_plain); + KUNIT_ASSERT_NOT_NULL(test, ei); + KUNIT_ASSERT_NOT_NULL(test, sbi); + KUNIT_ASSERT_NOT_NULL(test, plain_dir); + + um = utf8_load(UTF8_LATEST); + if (IS_ERR(um)) { + kunit_skip(test, "utf8_load(UTF8_LATEST) failed: %pe", + um); + return; + } + + ret = kunit_add_action_or_reset(test, utf8_unload_action, um); + KUNIT_ASSERT_EQ(test, ret, 0); + + ext4_hash_init_fake_ext4_dir(ei, sb_cf, sbi); + sb_cf->s_encoding = um; + ei->vfs_inode.i_flags |= S_CASEFOLD; + + KUNIT_ASSERT_TRUE(test, IS_CASEFOLDED(&ei->vfs_inode)); + + ext4_hash_init_fake_dir(plain_dir, sb_plain); + + ret_cf = ext4fs_dirhash(&ei->vfs_inode, invalid_utf8, + sizeof(invalid_utf8) - 1, &folded_dir); + ret_plain = ext4fs_dirhash(plain_dir, invalid_utf8, + sizeof(invalid_utf8) - 1, &plain); + + KUNIT_ASSERT_EQ(test, ret_cf, 0); + KUNIT_ASSERT_EQ(test, ret_plain, 0); + KUNIT_EXPECT_EQ(test, folded_dir.hash, plain.hash); + KUNIT_EXPECT_EQ(test, folded_dir.minor_hash, plain.minor_hash); +} +#endif + +static struct kunit_case ext4_hash_test_cases[] = { + KUNIT_CASE(test_ext4fs_dirhash_vectors), + KUNIT_CASE(test_ext4fs_dirhash_seed_changes_result), + KUNIT_CASE(test_ext4fs_dirhash_invalid_version_returns_einval), + KUNIT_CASE(test_ext4fs_dirhash_siphash_without_key_returns_einval), + KUNIT_CASE(test_ext4fs_dirhash_signed_unsigned_differ_on_nonascii), +#if IS_ENABLED(CONFIG_UNICODE) + KUNIT_CASE(test_ext4fs_dirhash_casefolded_names_hash_consistently), + KUNIT_CASE(test_ext4fs_dirhash_casefold_fallback), +#endif + {} +}; + +static struct kunit_suite ext4_hash_test_suite = { + .name = "ext4_hash", + .test_cases = ext4_hash_test_cases, +}; + +kunit_test_suites(&ext4_hash_test_suite); + +MODULE_LICENSE("GPL"); diff --git a/fs/ext4/hash.c b/fs/ext4/hash.c index 48483cd015d3..72645bd92582 100644 --- a/fs/ext4/hash.c +++ b/fs/ext4/hash.c @@ -321,3 +321,7 @@ int ext4fs_dirhash(const struct inode *dir, const char *name, int len, #endif return __ext4fs_dirhash(dir, name, len, hinfo); } + +#if IS_ENABLED(CONFIG_EXT4_KUNIT_TESTS) +EXPORT_SYMBOL_FOR_EXT4_TEST(ext4fs_dirhash); +#endif From 3ca1d19c1971ac4f25478eafb741e726bf2d5954 Mon Sep 17 00:00:00 2001 From: Guan-Chun Wu <409411716@gms.tku.edu.tw> Date: Sun, 31 May 2026 16:00:19 +0800 Subject: [PATCH 15/20] ext4: improve str2hashbuf by processing 4-byte chunks and removing function pointers The original byte-by-byte implementation with modulo checks is less efficient. Refactor str2hashbuf_unsigned() and str2hashbuf_signed() to process input in explicit 4-byte chunks instead of using a modulus-based loop to emit words byte by byte. Additionally, the use of function pointers for selecting the appropriate str2hashbuf implementation has been removed. Instead, the functions are directly invoked based on the hash type, eliminating the overhead of dynamic function calls. Performance test (x86_64, Intel Core i7-10700 @ 2.90GHz, average over 10000 runs, using kernel module for testing): len | orig_s | new_s | orig_u | new_u ----+--------+-------+--------+------- 1 | 70 | 71 | 63 | 63 8 | 68 | 64 | 64 | 62 32 | 75 | 70 | 75 | 63 64 | 96 | 71 | 100 | 68 255 | 192 | 108 | 187 | 84 This change improves performance, especially for larger input sizes. Signed-off-by: Guan-Chun Wu <409411716@gms.tku.edu.tw> Link: https://patch.msgid.link/20260531080019.3794809-3-409411716@gms.tku.edu.tw Signed-off-by: Theodore Ts'o --- fs/ext4/hash.c | 64 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 42 insertions(+), 22 deletions(-) diff --git a/fs/ext4/hash.c b/fs/ext4/hash.c index 72645bd92582..978bd92da0ad 100644 --- a/fs/ext4/hash.c +++ b/fs/ext4/hash.c @@ -9,6 +9,7 @@ #include #include #include +#include #include "ext4.h" #define DELTA 0x9E3779B9 @@ -141,21 +142,28 @@ static void str2hashbuf_signed(const char *msg, int len, __u32 *buf, int num) pad = (__u32)len | ((__u32)len << 8); pad |= pad << 16; - val = pad; if (len > num*4) len = num * 4; - for (i = 0; i < len; i++) { - val = ((int) scp[i]) + (val << 8); - if ((i % 4) == 3) { - *buf++ = val; - val = pad; - num--; - } + + while (len >= 4) { + val = ((__u32)scp[0] << 24) + ((__u32)scp[1] << 16) + ((__u32)scp[2] << 8) + scp[3]; + *buf++ = val; + scp += 4; + len -= 4; + num--; } + + val = pad; + + for (i = 0; i < len; i++) + val = scp[i] + (val << 8); + if (--num >= 0) *buf++ = val; + while (--num >= 0) *buf++ = pad; + } static void str2hashbuf_unsigned(const char *msg, int len, __u32 *buf, int num) @@ -167,21 +175,28 @@ static void str2hashbuf_unsigned(const char *msg, int len, __u32 *buf, int num) pad = (__u32)len | ((__u32)len << 8); pad |= pad << 16; - val = pad; if (len > num*4) len = num * 4; - for (i = 0; i < len; i++) { - val = ((int) ucp[i]) + (val << 8); - if ((i % 4) == 3) { - *buf++ = val; - val = pad; - num--; - } + + while (len >= 4) { + val = get_unaligned_be32(ucp); + *buf++ = val; + ucp += 4; + len -= 4; + num--; } + + val = pad; + + for (i = 0; i < len; i++) + val = ucp[i] + (val << 8); + if (--num >= 0) *buf++ = val; + while (--num >= 0) *buf++ = pad; + } /* @@ -205,8 +220,7 @@ static int __ext4fs_dirhash(const struct inode *dir, const char *name, int len, const char *p; int i; __u32 in[8], buf[4]; - void (*str2hashbuf)(const char *, int, __u32 *, int) = - str2hashbuf_signed; + bool use_unsigned = false; /* Initialize the default seed for the hash checksum functions */ buf[0] = 0x67452301; @@ -232,12 +246,15 @@ static int __ext4fs_dirhash(const struct inode *dir, const char *name, int len, hash = dx_hack_hash_signed(name, len); break; case DX_HASH_HALF_MD4_UNSIGNED: - str2hashbuf = str2hashbuf_unsigned; + use_unsigned = true; fallthrough; case DX_HASH_HALF_MD4: p = name; while (len > 0) { - (*str2hashbuf)(p, len, in, 8); + if (use_unsigned) + str2hashbuf_unsigned(p, len, in, 8); + else + str2hashbuf_signed(p, len, in, 8); half_md4_transform(buf, in); len -= 32; p += 32; @@ -246,12 +263,15 @@ static int __ext4fs_dirhash(const struct inode *dir, const char *name, int len, hash = buf[1]; break; case DX_HASH_TEA_UNSIGNED: - str2hashbuf = str2hashbuf_unsigned; + use_unsigned = true; fallthrough; case DX_HASH_TEA: p = name; while (len > 0) { - (*str2hashbuf)(p, len, in, 4); + if (use_unsigned) + str2hashbuf_unsigned(p, len, in, 4); + else + str2hashbuf_signed(p, len, in, 4); TEA_transform(buf, in); len -= 16; p += 16; From 4e3a55f44b42c2aabd4c1cc3bdb6a01a7107121d Mon Sep 17 00:00:00 2001 From: "Matthew Wilcox (Oracle)" Date: Tue, 26 May 2026 20:08:02 +0100 Subject: [PATCH 16/20] ext4: remove mention of PageWriteback Update a comment to refer to the concept of writeback instead of the (now obsolete) detail of how it's implemented. Signed-off-by: Matthew Wilcox (Oracle) Reviewed-by: Baokun Li Reviewed-by: Ojaswin Mujoo Reviewed-by: Jan Kara Link: https://patch.msgid.link/20260526190805.341676-1-willy@infradead.org Signed-off-by: Theodore Ts'o --- fs/ext4/page-io.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c index dc82e7b57e75..bc674aa4a656 100644 --- a/fs/ext4/page-io.c +++ b/fs/ext4/page-io.c @@ -168,7 +168,7 @@ static void ext4_release_io_end(ext4_io_end_t *io_end) * written. On IO failure, check if journal abort is needed. Note that * we are protected from truncate touching same part of extent tree by the * fact that truncate code waits for all DIO to finish (thus exclusion from - * direct IO is achieved) and also waits for PageWriteback bits. Thus we + * direct IO is achieved) and also waits for writeback to complete. Thus we * cannot get to ext4_ext_truncate() before all IOs overlapping that range are * completed (happens from ext4_free_ioend()). */ From bbe9015f23432bd4f5b8590eb178b3b5b7c29f02 Mon Sep 17 00:00:00 2001 From: "Matthew Wilcox (Oracle)" Date: Thu, 28 May 2026 18:14:11 +0100 Subject: [PATCH 17/20] jbd2: remove special jbd2 slabs When jbd2 was originally written, kmalloc() would not guarantee memory alignment for the requested objects. Since commit 59bb47985c1d in 2019, kmalloc has guaranteed natural alignment for power-of-two allocations. We can now remove the jbd2 special slabs and just use kmalloc() directly. Signed-off-by: Matthew Wilcox (Oracle) Reviewed-by: Jan Kara Acked-by: Mike Rapoport (Microsoft) Acked-by: Vlastimil Babka (SUSE) Reviewed-by: Tal Zussman Link: https://patch.msgid.link/20260528171413.1088143-1-willy@infradead.org Signed-off-by: Theodore Ts'o --- fs/jbd2/commit.c | 8 +-- fs/jbd2/journal.c | 125 ++---------------------------------------- fs/jbd2/transaction.c | 8 +-- include/linux/jbd2.h | 3 - 4 files changed, 13 insertions(+), 131 deletions(-) diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c index 8cf61e7185c4..8d5d28aa77f2 100644 --- a/fs/jbd2/commit.c +++ b/fs/jbd2/commit.c @@ -513,10 +513,8 @@ void jbd2_journal_commit_transaction(journal_t *journal) * leave undo-committed data. */ if (jh->b_committed_data) { - struct buffer_head *bh = jh2bh(jh); - spin_lock(&jh->b_state_lock); - jbd2_free(jh->b_committed_data, bh->b_size); + kfree(jh->b_committed_data); jh->b_committed_data = NULL; spin_unlock(&jh->b_state_lock); } @@ -977,7 +975,7 @@ void jbd2_journal_commit_transaction(journal_t *journal) * its triggers if they exist, so we can clear that too. */ if (jh->b_committed_data) { - jbd2_free(jh->b_committed_data, bh->b_size); + kfree(jh->b_committed_data); jh->b_committed_data = NULL; if (jh->b_frozen_data) { jh->b_committed_data = jh->b_frozen_data; @@ -985,7 +983,7 @@ void jbd2_journal_commit_transaction(journal_t *journal) jh->b_frozen_triggers = NULL; } } else if (jh->b_frozen_data) { - jbd2_free(jh->b_frozen_data, bh->b_size); + kfree(jh->b_frozen_data); jh->b_frozen_data = NULL; jh->b_frozen_triggers = NULL; } diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c index e3b2e38e1a1b..4fdf089500f6 100644 --- a/fs/jbd2/journal.c +++ b/fs/jbd2/journal.c @@ -95,8 +95,6 @@ EXPORT_SYMBOL(jbd2_journal_release_jbd_inode); EXPORT_SYMBOL(jbd2_journal_begin_ordered_truncate); EXPORT_SYMBOL(jbd2_inode_cache); -static int jbd2_journal_create_slab(size_t slab_size); - #ifdef CONFIG_JBD2_DEBUG void __jbd2_debug(int level, const char *file, const char *func, unsigned int line, const char *fmt, ...) @@ -385,10 +383,10 @@ int jbd2_journal_write_metadata_buffer(transaction_t *transaction, goto escape_done; spin_unlock(&jh_in->b_state_lock); - tmp = jbd2_alloc(bh_in->b_size, GFP_NOFS | __GFP_NOFAIL); + tmp = kmalloc(bh_in->b_size, GFP_NOFS | __GFP_NOFAIL); spin_lock(&jh_in->b_state_lock); if (jh_in->b_frozen_data) { - jbd2_free(tmp, bh_in->b_size); + kfree(tmp); goto copy_done; } @@ -2064,14 +2062,6 @@ EXPORT_SYMBOL(jbd2_journal_update_sb_errno); int jbd2_journal_load(journal_t *journal) { int err; - journal_superblock_t *sb = journal->j_superblock; - - /* - * Create a slab for this blocksize - */ - err = jbd2_journal_create_slab(be32_to_cpu(sb->s_blocksize)); - if (err) - return err; /* Let the recovery code check whether it needs to recover any * data from the journal. */ @@ -2701,108 +2691,6 @@ size_t journal_tag_bytes(journal_t *journal) return sz - sizeof(__u32); } -/* - * JBD memory management - * - * These functions are used to allocate block-sized chunks of memory - * used for making copies of buffer_head data. Very often it will be - * page-sized chunks of data, but sometimes it will be in - * sub-page-size chunks. (For example, 16k pages on Power systems - * with a 4k block file system.) For blocks smaller than a page, we - * use a SLAB allocator. There are slab caches for each block size, - * which are allocated at mount time, if necessary, and we only free - * (all of) the slab caches when/if the jbd2 module is unloaded. For - * this reason we don't need to a mutex to protect access to - * jbd2_slab[] allocating or releasing memory; only in - * jbd2_journal_create_slab(). - */ -#define JBD2_MAX_SLABS 8 -static struct kmem_cache *jbd2_slab[JBD2_MAX_SLABS]; - -static const char *jbd2_slab_names[JBD2_MAX_SLABS] = { - "jbd2_1k", "jbd2_2k", "jbd2_4k", "jbd2_8k", - "jbd2_16k", "jbd2_32k", "jbd2_64k", "jbd2_128k" -}; - - -static void jbd2_journal_destroy_slabs(void) -{ - int i; - - for (i = 0; i < JBD2_MAX_SLABS; i++) { - kmem_cache_destroy(jbd2_slab[i]); - jbd2_slab[i] = NULL; - } -} - -static int jbd2_journal_create_slab(size_t size) -{ - static DEFINE_MUTEX(jbd2_slab_create_mutex); - int i = order_base_2(size) - 10; - size_t slab_size; - - if (size == PAGE_SIZE) - return 0; - - if (i >= JBD2_MAX_SLABS) - return -EINVAL; - - if (unlikely(i < 0)) - i = 0; - mutex_lock(&jbd2_slab_create_mutex); - if (jbd2_slab[i]) { - mutex_unlock(&jbd2_slab_create_mutex); - return 0; /* Already created */ - } - - slab_size = 1 << (i+10); - jbd2_slab[i] = kmem_cache_create(jbd2_slab_names[i], slab_size, - slab_size, 0, NULL); - mutex_unlock(&jbd2_slab_create_mutex); - if (!jbd2_slab[i]) { - printk(KERN_EMERG "JBD2: no memory for jbd2_slab cache\n"); - return -ENOMEM; - } - return 0; -} - -static struct kmem_cache *get_slab(size_t size) -{ - int i = order_base_2(size) - 10; - - BUG_ON(i >= JBD2_MAX_SLABS); - if (unlikely(i < 0)) - i = 0; - BUG_ON(jbd2_slab[i] == NULL); - return jbd2_slab[i]; -} - -void *jbd2_alloc(size_t size, gfp_t flags) -{ - void *ptr; - - BUG_ON(size & (size-1)); /* Must be a power of 2 */ - - if (size < PAGE_SIZE) - ptr = kmem_cache_alloc(get_slab(size), flags); - else - ptr = (void *)__get_free_pages(flags, get_order(size)); - - /* Check alignment; SLUB has gotten this wrong in the past, - * and this can lead to user data corruption! */ - BUG_ON(((unsigned long) ptr) & (size-1)); - - return ptr; -} - -void jbd2_free(void *ptr, size_t size) -{ - if (size < PAGE_SIZE) - kmem_cache_free(get_slab(size), ptr); - else - free_pages((unsigned long)ptr, get_order(size)); -}; - /* * Journal_head storage management */ @@ -2976,15 +2864,15 @@ static void __journal_remove_journal_head(struct buffer_head *bh) clear_buffer_jbd(bh); } -static void journal_release_journal_head(struct journal_head *jh, size_t b_size) +static void journal_release_journal_head(struct journal_head *jh) { if (jh->b_frozen_data) { printk(KERN_WARNING "%s: freeing b_frozen_data\n", __func__); - jbd2_free(jh->b_frozen_data, b_size); + kfree(jh->b_frozen_data); } if (jh->b_committed_data) { printk(KERN_WARNING "%s: freeing b_committed_data\n", __func__); - jbd2_free(jh->b_committed_data, b_size); + kfree(jh->b_committed_data); } journal_free_journal_head(jh); } @@ -3003,7 +2891,7 @@ void jbd2_journal_put_journal_head(struct journal_head *jh) if (!jh->b_jcount) { __journal_remove_journal_head(bh); jbd_unlock_bh_journal_head(bh); - journal_release_journal_head(jh, bh->b_size); + journal_release_journal_head(jh); __brelse(bh); } else { jbd_unlock_bh_journal_head(bh); @@ -3145,7 +3033,6 @@ static void jbd2_journal_destroy_caches(void) jbd2_journal_destroy_handle_cache(); jbd2_journal_destroy_inode_cache(); jbd2_journal_destroy_transaction_cache(); - jbd2_journal_destroy_slabs(); } static int __init journal_init(void) diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c index aa0be9e9c876..5cc7d097b2ac 100644 --- a/fs/jbd2/transaction.c +++ b/fs/jbd2/transaction.c @@ -1131,7 +1131,7 @@ do_get_write_access(handle_t *handle, struct journal_head *jh, if (!frozen_buffer) { JBUFFER_TRACE(jh, "allocate memory for buffer"); spin_unlock(&jh->b_state_lock); - frozen_buffer = jbd2_alloc(jh2bh(jh)->b_size, + frozen_buffer = kmalloc(jh2bh(jh)->b_size, GFP_NOFS | __GFP_NOFAIL); goto repeat; } @@ -1159,7 +1159,7 @@ do_get_write_access(handle_t *handle, struct journal_head *jh, out: if (unlikely(frozen_buffer)) /* It's usually NULL */ - jbd2_free(frozen_buffer, bh->b_size); + kfree(frozen_buffer); JBUFFER_TRACE(jh, "exit"); return error; @@ -1424,7 +1424,7 @@ int jbd2_journal_get_undo_access(handle_t *handle, struct buffer_head *bh) repeat: if (!jh->b_committed_data) - committed_data = jbd2_alloc(jh2bh(jh)->b_size, + committed_data = kmalloc(jh2bh(jh)->b_size, GFP_NOFS|__GFP_NOFAIL); spin_lock(&jh->b_state_lock); @@ -1445,7 +1445,7 @@ int jbd2_journal_get_undo_access(handle_t *handle, struct buffer_head *bh) out: jbd2_journal_put_journal_head(jh); if (unlikely(committed_data)) - jbd2_free(committed_data, bh->b_size); + kfree(committed_data); return err; } diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h index 7e785aa6d35d..b68561187e90 100644 --- a/include/linux/jbd2.h +++ b/include/linux/jbd2.h @@ -63,9 +63,6 @@ void __jbd2_debug(int level, const char *file, const char *func, #define jbd2_debug(n, fmt, a...) no_printk(fmt, ##a) #endif -extern void *jbd2_alloc(size_t size, gfp_t flags); -extern void jbd2_free(void *ptr, size_t size); - #define JBD2_MIN_JOURNAL_BLOCKS 1024 #define JBD2_DEFAULT_FAST_COMMIT_BLOCKS 256 From 8e1c43af7cf5091d99db38b7c8129e394d7f45b5 Mon Sep 17 00:00:00 2001 From: Hongling Zeng Date: Thu, 4 Jun 2026 15:36:47 +0800 Subject: [PATCH 18/20] ext4: fix ERR_PTR(0) in ext4_mkdir() When mkdir succeeds, ext4_mkdir() returns ERR_PTR(0) which is incorrect. It should return NULL instead for success and ERR_PTR() only with negative error codes for failure. Fixes: 88d5baf69082 ("Change inode_operations.mkdir to return struct dentry *") Signed-off-by: Hongling Zeng Reviewed-by: Jan Kara Reviewed-by: Baokun Li Link: https://patch.msgid.link/20260604073647.211279-1-zenghongling@kylinos.cn Signed-off-by: Theodore Ts'o --- fs/ext4/namei.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c index c0cabf172020..cc49ae04a6f6 100644 --- a/fs/ext4/namei.c +++ b/fs/ext4/namei.c @@ -3054,7 +3054,7 @@ static struct dentry *ext4_mkdir(struct mnt_idmap *idmap, struct inode *dir, out_retry: if (err == -ENOSPC && ext4_should_retry_alloc(dir->i_sb, &retries)) goto retry; - return ERR_PTR(err); + return err ? ERR_PTR(err) : NULL; } /* From ad09aa45965d3fafaf9963bc78109b73c0f9ac8d Mon Sep 17 00:00:00 2001 From: Aditya Prakash Srivastava Date: Mon, 8 Jun 2026 06:52:27 +0000 Subject: [PATCH 19/20] ext4: fix kernel BUG in ext4_write_inline_data_end When the data=journal mount option is used, the ext4_journalled_write_end() function incorrectly calls ext4_write_inline_data_end() without checking if the EXT4_STATE_MAY_INLINE_DATA flag is still set on the inode. If a previous attempt to convert the inline data to an extent failed (e.g. due to ENOSPC), the EXT4_STATE_MAY_INLINE_DATA flag is cleared, but the EXT4_INODE_INLINE_DATA flag remains set. In this scenario, the next call to ext4_write_begin() will not prepare the inline data xattr for writing, but ext4_journalled_write_end() will incorrectly attempt to write to it, triggering a BUG_ON(pos + len > EXT4_I(inode)->i_inline_size) in ext4_write_inline_data() since i_inline_size was not expanded. Fix this by ensuring that ext4_journalled_write_end() only calls ext4_write_inline_data_end() if the EXT4_STATE_MAY_INLINE_DATA flag is set, mirroring the behavior of ext4_write_end() and ext4_da_write_end(). Reported-by: syzbot+0c89d865531d053abb2d@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=0c89d865531d053abb2d Fixes: 3fdcfb668fd7 ("ext4: add journalled write support for inline data") Signed-off-by: Aditya Prakash Srivastava Reviewed-by: Jan Kara Link: https://patch.msgid.link/20260608065227.3018-1-aditya.ansh182@gmail.com Signed-off-by: Theodore Ts'o --- fs/ext4/inode.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index 4678612f82e8..ce99807c5f5b 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -1560,7 +1560,8 @@ static int ext4_journalled_write_end(const struct kiocb *iocb, BUG_ON(!ext4_handle_valid(handle)); - if (ext4_has_inline_data(inode)) + if (ext4_has_inline_data(inode) && + ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA)) return ext4_write_inline_data_end(inode, pos, len, copied, folio); From c143957520c6c9b5cd72e0de8b52b814f0c576fe Mon Sep 17 00:00:00 2001 From: Yun Zhou Date: Mon, 8 Jun 2026 23:25:21 +0800 Subject: [PATCH 20/20] ext4: validate donor file superblock early in EXT4_IOC_MOVE_EXT Reject the EXT4_IOC_MOVE_EXT ioctl early if the donor file does not belong to the same superblock as the original file. Currently, this validation is performed inside ext4_move_extents() by mext_check_validity(), but only after lock_two_nondirectories() has already acquired the inode locks. When the donor fd refers to a file on a different filesystem (e.g., overlayfs), this late validation creates a circular lock dependency: CPU0 (overlayfs write) CPU1 (ext4 ioctl) ---- ---- inode_lock(ovl_inode) mnt_want_write_file(filp) sb_start_write(ext4_sb) [sb_writers] backing_file_write_iter() vfs_iter_write(real_file) file_start_write(real_file) sb_start_write(ext4_sb) [blocked by freeze] lock_two_nondirectories() inode_lock(ovl_inode) [blocked] With a concurrent freeze operation holding sb_writers write side, this forms a deadlock cycle: CPU0 waits for freeze to complete, freeze waits for CPU1's sb_writers reader to exit, CPU1 waits for CPU0's inode lock. Since EXT4_IOC_MOVE_EXT exchanges physical extents between two files, it fundamentally requires both files to reside on the same ext4 filesystem. Moving the superblock check before any lock acquisition is both semantically correct and eliminates the circular dependency by ensuring that cross-filesystem donor fds are rejected before sb_writers or inode locks are taken. Fixes: fcf6b1b729bc ("ext4: refactor ext4_move_extents code base") Reported-by: syzbot+ad6118a7584b607c67f2@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=ad6118a7584b607c67f2 Signed-off-by: Yun Zhou Reviewed-by: Jan Kara Reviewed-by: Andreas Dilger Link: https://patch.msgid.link/20260608152521.1292656-1-yun.zhou@windriver.com Signed-off-by: Theodore Ts'o --- fs/ext4/ioctl.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c index 110e3fb194ec..c8387e6a2c6e 100644 --- a/fs/ext4/ioctl.c +++ b/fs/ext4/ioctl.c @@ -1656,6 +1656,9 @@ static long __ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) if (!(fd_file(donor)->f_mode & FMODE_WRITE)) return -EBADF; + if (file_inode(filp)->i_sb != file_inode(fd_file(donor))->i_sb) + return -EXDEV; + err = mnt_want_write_file(filp); if (err) return err;