| Message ID | 20260513085818.552432-1-me@linux.beauty |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
| Series | ext4: fix fast commit wait/wake bit mapping on 64-bit | expand |
On Wed 13-05-26 16:58:17, Li Chen wrote: > 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 <sashiko-bot@kernel.org> > Signed-off-by: Li Chen <chenl311@chinatelecom.cn> Nice cleanup and a good spotting. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > 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])) { > -- > 2.53.0 >
On 5/13/2026 4:58 PM, Li Chen wrote: > 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 <sashiko-bot@kernel.org> > Signed-off-by: Li Chen <chenl311@chinatelecom.cn> Ha, This looks good to me! Thanks. Reviewed-by: Zhang Yi <yi.zhang@huawei.com> > --- > 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])) {
On 2026/5/13 16:58, Li Chen wrote: > 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 <sashiko-bot@kernel.org> > Signed-off-by: Li Chen <chenl311@chinatelecom.cn> This cleanup looks good! Just two small nits below. Nits aside: Reviewed-by: Baokun Li <libaokun@linux.alibaba.com> > --- > 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); The forward declarations above are only needed for macro-generated functions that can't be found by name search. These two helpers are spelled out directly, so the declarations are unnecessary here. > #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 > +} Would it be cleaner to place these inside the existing #if (BITS_PER_LONG < 64) / #else block alongside ext4_clear_state_flags(), the same way the rest of the state helpers are organised? That would also avoid repeating the #if guard. Cheers, Baokun > #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])) {
On Wed, 13 May 2026 16:58:17 +0800, Li Chen wrote: > 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. > > [...] Applied, thanks! [1/1] ext4: fix fast commit wait/wake bit mapping on 64-bit commit: 8b3bc93fee6771775243665a0cf31857d6659775 Best regards,
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])) {
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 <sashiko-bot@kernel.org> Signed-off-by: Li Chen <chenl311@chinatelecom.cn> --- fs/ext4/ext4.h | 20 +++++++++++++++++ fs/ext4/fast_commit.c | 50 ++++++++++++++++--------------------------- 2 files changed, 38 insertions(+), 32 deletions(-)