| From 6db3c1575a750fd417a70e0178bdf6efa0dd5037 Mon Sep 17 00:00:00 2001 |
| From: "Luis Henriques (SUSE)" <luis.henriques@linux.dev> |
| Date: Wed, 17 Jul 2024 18:22:20 +0100 |
| Subject: ext4: fix fast commit inode enqueueing during a full journal commit |
| |
| From: Luis Henriques (SUSE) <luis.henriques@linux.dev> |
| |
| commit 6db3c1575a750fd417a70e0178bdf6efa0dd5037 upstream. |
| |
| When a full journal commit is on-going, any fast commit has to be enqueued |
| into a different queue: FC_Q_STAGING instead of FC_Q_MAIN. This enqueueing |
| is done only once, i.e. if an inode is already queued in a previous fast |
| commit entry it won't be enqueued again. However, if a full commit starts |
| _after_ the inode is enqueued into FC_Q_MAIN, the next fast commit needs to |
| be done into FC_Q_STAGING. And this is not being done in function |
| ext4_fc_track_template(). |
| |
| This patch fixes the issue by re-enqueuing an inode into the STAGING queue |
| during the fast commit clean-up callback when doing a full commit. However, |
| to prevent a race with a fast-commit, the clean-up callback has to be called |
| with the journal locked. |
| |
| This bug was found using fstest generic/047. This test creates several 32k |
| bytes files, sync'ing each of them after it's creation, and then shutting |
| down the filesystem. Some data may be loss in this operation; for example a |
| file may have it's size truncated to zero. |
| |
| Suggested-by: Jan Kara <jack@suse.cz> |
| Signed-off-by: Luis Henriques (SUSE) <luis.henriques@linux.dev> |
| Reviewed-by: Jan Kara <jack@suse.cz> |
| Link: https://patch.msgid.link/20240717172220.14201-1-luis.henriques@linux.dev |
| Signed-off-by: Theodore Ts'o <tytso@mit.edu> |
| Cc: stable@kernel.org |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| fs/ext4/fast_commit.c | 15 ++++++++++++++- |
| fs/jbd2/journal.c | 2 +- |
| 2 files changed, 15 insertions(+), 2 deletions(-) |
| |
| --- a/fs/ext4/fast_commit.c |
| +++ b/fs/ext4/fast_commit.c |
| @@ -1295,8 +1295,21 @@ static void ext4_fc_cleanup(journal_t *j |
| list_del_init(&iter->i_fc_list); |
| ext4_clear_inode_state(&iter->vfs_inode, |
| EXT4_STATE_FC_COMMITTING); |
| - if (tid_geq(tid, iter->i_sync_tid)) |
| + if (tid_geq(tid, iter->i_sync_tid)) { |
| ext4_fc_reset_inode(&iter->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). |
| + */ |
| + list_add_tail(&EXT4_I(&iter->vfs_inode)->i_fc_list, |
| + &sbi->s_fc_q[FC_Q_STAGING]); |
| + } |
| /* Make sure EXT4_STATE_FC_COMMITTING bit is clear */ |
| smp_mb(); |
| #if (BITS_PER_LONG < 64) |
| --- a/fs/jbd2/journal.c |
| +++ b/fs/jbd2/journal.c |
| @@ -740,9 +740,9 @@ EXPORT_SYMBOL(jbd2_fc_begin_commit); |
| */ |
| static int __jbd2_fc_end_commit(journal_t *journal, tid_t tid, bool fallback) |
| { |
| - jbd2_journal_unlock_updates(journal); |
| if (journal->j_fc_cleanup_callback) |
| journal->j_fc_cleanup_callback(journal, 0, tid); |
| + jbd2_journal_unlock_updates(journal); |
| write_lock(&journal->j_state_lock); |
| journal->j_flags &= ~JBD2_FAST_COMMIT_ONGOING; |
| if (fallback) |
