On Fri 06-12-24 19:13:27, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@xxxxxxxxxx>
>
> When the filesystem performs file or filesystem synchronization (e.g.,
> ext4_sync_file()), it queries the journal to determine whether to flush
> the file device through jbd2_trans_will_send_data_barrier(). If the
> target transaction has not started committing, it assumes that the
> journal will submit the flush command, allowing the filesystem to bypass
> a redundant flush command. However, this assumption is not always valid.
> If the journal is not located on the filesystem device, the journal
> commit thread will not submit the flush command unless the variable
> ->t_need_data_flush is set to 1. Consequently, the flush may be missed,
> and data may be lost following a power failure or system crash, even if
> the synchronization appears to succeed.
>
> Unfortunately, we cannot determine with certainty whether the target
> transaction will flush to the filesystem device before it commits.
> However, if it has not started committing, it must be the running
> transaction. Therefore, fix it by always set its t_need_data_flush to 1,
> ensuring that the committing thread will flush the filesystem device.
>
> Fixes: bbd2be369107 ("jbd2: Add function jbd2_trans_will_send_data_barrier()")
> Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
> ---
> fs/jbd2/journal.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 97f487c3d8fc..37632ae18a4e 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -609,7 +609,7 @@ int jbd2_journal_start_commit(journal_t *journal, tid_t *ptid)
> int jbd2_trans_will_send_data_barrier(journal_t *journal, tid_t tid)
> {
> int ret = 0;
> - transaction_t *commit_trans;
> + transaction_t *commit_trans, *running_trans;
>
> if (!(journal->j_flags & JBD2_BARRIER))
> return 0;
> @@ -619,6 +619,16 @@ int jbd2_trans_will_send_data_barrier(journal_t *journal, tid_t tid)
> goto out;
> commit_trans = journal->j_committing_transaction;
> if (!commit_trans || commit_trans->t_tid != tid) {
> + running_trans = journal->j_running_transaction;
> + /*
> + * The query transaction hasn't started committing,
> + * it must still be running.
> + */
> + if (WARN_ON_ONCE(!running_trans ||
> + running_trans->t_tid != tid))
> + goto out;
> +
> + running_trans->t_need_data_flush = 1;
> ret = 1;
> goto out;
> }
> --
> 2.46.1
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
