All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Luis Henriques (SUSE)" <luis.henriques@linux.dev>
To: Theodore Ts'o <tytso@mit.edu>, Andreas Dilger <adilger@dilger.ca>,
	Jan Kara <jack@suse.cz>,
	Harshad Shirwadkar <harshadshirwadkar@gmail.com>
Cc: linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Luis Henriques (SUSE)" <luis.henriques@linux.dev>
Subject: [PATCH v4] ext4: fix fast commit inode enqueueing during a full journal commit
Date: Thu, 11 Jul 2024 09:35:20 +0100	[thread overview]
Message-ID: <20240711083520.6751-1-luis.henriques@linux.dev> (raw)

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 if it has a tid (i_sync_tid)
greater than the one being handled.  The STAGING queue will then be spliced
back into MAIN.

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.

Signed-off-by: Luis Henriques (SUSE) <luis.henriques@linux.dev>
---
Hi!

v4 of this patch enqueues the inode into STAGING *only* if the current tid
is non-zero.  It will be zero when doing an fc commit, and this would mean
to always re-enqueue the inode.  This fixes the regressions caught by Ted
in v3 with fstests generic/472 generic/496 generic/643.

Also, since 2nd patch of v3 has already been merged, I've rebased this patch
to be applied on top of it.

 fs/ext4/fast_commit.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 3926a05eceee..facbc8dbbaa2 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -1290,6 +1290,16 @@ static void ext4_fc_cleanup(journal_t *journal, int full, tid_t tid)
 				       EXT4_STATE_FC_COMMITTING);
 		if (tid_geq(tid, iter->i_sync_tid))
 			ext4_fc_reset_inode(&iter->vfs_inode);
+		} else if (tid) {
+			/*
+			 * If the tid is valid (i.e. non-zero) re-enqueue the
+			 * inode into STAGING, which will then be splice back
+			 * into MAIN
+			 */
+			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)

             reply	other threads:[~2024-07-11  8:35 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-11  8:35 Luis Henriques (SUSE) [this message]
2024-07-11 13:32 ` [PATCH v4] ext4: fix fast commit inode enqueueing during a full journal commit wangjianjian (C)
2024-07-11 15:16   ` Luis Henriques
2024-07-11 16:16     ` Wang Jianjian
2024-07-11 19:28       ` Andreas Dilger
2024-07-12  0:51         ` wangjianjian (C)
2024-07-12  9:15         ` Luis Henriques
2024-07-12  9:53           ` [RFC PATCH] jbd2: make '0' an invalid transaction sequence Luis Henriques
2024-07-12 10:04             ` wangjianjian (C)
2024-07-12 10:28               ` wangjianjian (C)
2024-07-16  9:52             ` Jan Kara
2024-07-16 13:11               ` Luis Henriques
2024-07-16 10:24 ` [PATCH v4] ext4: fix fast commit inode enqueueing during a full journal commit Jan Kara
2024-07-16 14:13   ` Luis Henriques

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240711083520.6751-1-luis.henriques@linux.dev \
    --to=luis.henriques@linux.dev \
    --cc=adilger@dilger.ca \
    --cc=harshadshirwadkar@gmail.com \
    --cc=jack@suse.cz \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.