[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <81ae9161-8403-4e6d-a3da-1b52bd989ac9@huaweicloud.com>
Date: Tue, 18 Mar 2025 12:37:17 +0800
From: Zhang Yi <yi.zhang@...weicloud.com>
To: tytso@....edu
Cc: linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
adilger.kernel@...ger.ca, jack@...e.cz, yi.zhang@...wei.com,
chengzhihao1@...wei.com, yukuai3@...wei.com, yangerkun@...wei.com,
linux-ext4@...r.kernel.org
Subject: Re: [PATCH] jbd2: add a missing data flush during file and fs
synchronization
Hi, Ted.
Just wanted to kindly check if this patch might have been
overlooked?
Thanks,
Yi.
On 2024/12/6 19:13, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@...wei.com>
>
> 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@...wei.com>
> ---
> 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;
> }
Powered by blists - more mailing lists