Linux EXT4 FS development
 help / color / mirror / Atom feed
* [RFC v6 3/7] ext4: fast commit: avoid waiting for FC_COMMITTING
From: Li Chen @ 2026-04-08 11:20 UTC (permalink / raw)
  To: Zhang Yi, Theodore Ts'o, Andreas Dilger, Baokun Li, Jan Kara,
	Ojaswin Mujoo, Ritesh Harjani (IBM), Zhang Yi, linux-ext4,
	linux-kernel
  Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
	linux-trace-kernel, Li Chen
In-Reply-To: <20260408112020.716706-1-me@linux.beauty>

ext4_fc_track_inode() can be called while holding i_data_sem (e.g.
fallocate). Waiting for EXT4_STATE_FC_COMMITTING in that case risks an
ABBA deadlock: i_data_sem -> wait(FC_COMMITTING) vs FC_COMMITTING ->
wait(i_data_sem) in the commit task.

Now that fast commit snapshots inode state at commit time, updates during
log writing do not need to block. Drop the wait and lockdep assertion in
ext4_fc_track_inode(), and make ext4_fc_del() wait for FC_COMMITTING so an
inode cannot be removed while the commit thread is still using it.

When an inode is modified during a fast commit, mark it with
EXT4_STATE_FC_REQUEUE so cleanup keeps it queued for the next fast commit.
This is needed because jbd2_fc_end_commit() invokes the cleanup callback
with tid == 0, so tid-based requeue logic would requeue every inode.

Testing: tracepoint ext4:ext4_fc_commit_stop with two fsyncs in the same
transaction. nblks is the number of journal blocks written for that fast
commit. Before this change, the second fsync still wrote almost the same
fast commit log (nblks 10->9), because tid == 0 in jbd2_fc_end_commit()
caused the tid-based requeue logic to keep all inodes queued. After this
change, only inodes modified during the commit are requeued, and the
second fsync wrote a nearly empty fast commit (nblks 10->1).

Signed-off-by: Li Chen <me@linux.beauty>
---
 fs/ext4/ext4.h        |   1 +
 fs/ext4/fast_commit.c | 111 ++++++++++++++++++++----------------------
 2 files changed, 53 insertions(+), 59 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 66de888ae411..13fe4fdf9bda 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1995,6 +1995,7 @@ enum {
 	EXT4_STATE_FC_COMMITTING,	/* Fast commit ongoing */
 	EXT4_STATE_FC_FLUSHING_DATA,	/* Fast commit flushing data */
 	EXT4_STATE_ORPHAN_FILE,		/* Inode orphaned in orphan file */
+	EXT4_STATE_FC_REQUEUE,		/* Inode modified during fast commit */
 };
 
 #define EXT4_INODE_BIT_FNS(name, field, offset)				\
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index dc19795dacdd..5ed884cc4b5c 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -61,9 +61,8 @@
  *     setting "EXT4_STATE_FC_COMMITTING" state, and snapshot the inode state
  *     needed for log writing.
  * [5] Unlock the journal by calling jbd2_journal_unlock_updates(). This allows
- *     starting of new handles. If new handles try to start an update on
- *     any of the inodes that are being committed, ext4_fc_track_inode()
- *     will block until those inodes have finished the fast commit.
+ *     starting of new handles. Updates to inodes being fast committed are
+ *     tracked for requeue rather than blocking.
  * [6] Commit all the directory entry updates in the fast commit space.
  * [7] Commit all the changed inodes in the fast commit space.
  * [8] Write tail tag (this tag ensures the atomicity, please read the following
@@ -217,6 +216,7 @@ void ext4_fc_init_inode(struct inode *inode)
 
 	ext4_fc_reset_inode(inode);
 	ext4_clear_inode_state(inode, EXT4_STATE_FC_COMMITTING);
+	ext4_clear_inode_state(inode, EXT4_STATE_FC_REQUEUE);
 	INIT_LIST_HEAD(&ei->i_fc_list);
 	INIT_LIST_HEAD(&ei->i_fc_dilist);
 	ei->i_fc_snap = NULL;
@@ -251,22 +251,30 @@ void ext4_fc_del(struct inode *inode)
 	}
 
 	/*
-	 * Since ext4_fc_del is called from ext4_evict_inode while having a
-	 * handle open, there is no need for us to wait here even if a fast
-	 * commit is going on. That is because, if this inode is being
-	 * committed, ext4_mark_inode_dirty would have waited for inode commit
-	 * operation to finish before we come here. So, by the time we come
-	 * here, inode's EXT4_STATE_FC_COMMITTING would have been cleared. So,
-	 * we shouldn't see EXT4_STATE_FC_COMMITTING to be set on this inode
-	 * here.
-	 *
-	 * We may come here without any handles open in the "no_delete" case of
-	 * ext4_evict_inode as well. However, if that happens, we first mark the
-	 * file system as fast commit ineligible anyway. So, even in that case,
-	 * it is okay to remove the inode from the fc list.
+	 * Wait for ongoing fast commit to finish. We cannot remove the inode
+	 * from fast commit lists while it is being committed.
 	 */
-	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_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
+		prepare_to_wait(wq, &wait.wq_entry, TASK_UNINTERRUPTIBLE);
+		if (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING)) {
+			ext4_fc_unlock(inode->i_sb, alloc_ctx);
+			schedule();
+			alloc_ctx = ext4_fc_lock(inode->i_sb);
+		}
+		finish_wait(wq, &wait.wq_entry);
+	}
+
 	while (ext4_test_inode_state(inode, EXT4_STATE_FC_FLUSHING_DATA)) {
 #if (BITS_PER_LONG < 64)
 		DEFINE_WAIT_BIT(wait, &ei->i_state_flags,
@@ -287,19 +295,22 @@ void ext4_fc_del(struct inode *inode)
 		}
 		finish_wait(wq, &wait.wq_entry);
 	}
+
 	ext4_fc_free_inode_snap(inode);
 	list_del_init(&ei->i_fc_list);
 
 	/*
-	 * Since this inode is getting removed, let's also remove all FC
-	 * dentry create references, since it is not needed to log it anyways.
+	 * Since this inode is getting removed, let's also remove all FC dentry
+	 * create references, since it is not needed to log it anyways.
 	 */
 	if (list_empty(&ei->i_fc_dilist)) {
 		ext4_fc_unlock(inode->i_sb, alloc_ctx);
 		return;
 	}
 
-	fc_dentry = list_first_entry(&ei->i_fc_dilist, struct ext4_fc_dentry_update, fcd_dilist);
+	fc_dentry = list_first_entry(&ei->i_fc_dilist,
+				     struct ext4_fc_dentry_update,
+				     fcd_dilist);
 	WARN_ON(fc_dentry->fcd_op != EXT4_FC_TAG_CREAT);
 	list_del_init(&fc_dentry->fcd_list);
 	list_del_init(&fc_dentry->fcd_dilist);
@@ -371,6 +382,8 @@ static int ext4_fc_track_template(
 
 	tid = handle->h_transaction->t_tid;
 	spin_lock(&ei->i_fc_lock);
+	if (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING))
+		ext4_set_inode_state(inode, EXT4_STATE_FC_REQUEUE);
 	if (tid == ei->i_sync_tid) {
 		update = true;
 	} else {
@@ -557,8 +570,6 @@ static int __track_inode(handle_t *handle, struct inode *inode, void *arg,
 
 void ext4_fc_track_inode(handle_t *handle, struct inode *inode)
 {
-	struct ext4_inode_info *ei = EXT4_I(inode);
-	wait_queue_head_t *wq;
 	int ret;
 
 	if (S_ISDIR(inode->i_mode))
@@ -577,29 +588,11 @@ void ext4_fc_track_inode(handle_t *handle, struct inode *inode)
 		return;
 
 	/*
-	 * If we come here, we may sleep while waiting for the inode to
-	 * commit. We shouldn't be holding i_data_sem when we go to sleep since
-	 * the commit path needs to grab the lock while committing the inode.
+	 * Fast commit snapshots inode state at commit time, so there's no need
+	 * to wait for EXT4_STATE_FC_COMMITTING here. If the inode is already
+	 * on the commit queue, ext4_fc_cleanup() will requeue it for the new
+	 * transaction once the current commit finishes.
 	 */
-	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
-		prepare_to_wait(wq, &wait.wq_entry, TASK_UNINTERRUPTIBLE);
-		if (ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING))
-			schedule();
-		finish_wait(wq, &wait.wq_entry);
-	}
 
 	/*
 	 * From this point on, this inode will not be committed either
@@ -1526,32 +1519,32 @@ static void ext4_fc_cleanup(journal_t *journal, int full, tid_t tid)
 
 	alloc_ctx = ext4_fc_lock(sb);
 	while (!list_empty(&sbi->s_fc_q[FC_Q_MAIN])) {
+		bool requeue;
+
 		ei = list_first_entry(&sbi->s_fc_q[FC_Q_MAIN],
 					struct ext4_inode_info,
 					i_fc_list);
 		list_del_init(&ei->i_fc_list);
 		ext4_fc_free_inode_snap(&ei->vfs_inode);
+		spin_lock(&ei->i_fc_lock);
+		if (full)
+			requeue = !tid_geq(tid, ei->i_sync_tid);
+		else
+			requeue = ext4_test_inode_state(&ei->vfs_inode,
+							EXT4_STATE_FC_REQUEUE);
+		if (!requeue)
+			ext4_fc_reset_inode(&ei->vfs_inode);
+		ext4_clear_inode_state(&ei->vfs_inode, EXT4_STATE_FC_REQUEUE);
 		ext4_clear_inode_state(&ei->vfs_inode,
 				       EXT4_STATE_FC_COMMITTING);
-		if (tid_geq(tid, ei->i_sync_tid)) {
-			ext4_fc_reset_inode(&ei->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).
-			 */
+		spin_unlock(&ei->i_fc_lock);
+		if (requeue)
 			list_add_tail(&ei->i_fc_list,
 				      &sbi->s_fc_q[FC_Q_STAGING]);
-		}
 		/*
 		 * Make sure clearing of EXT4_STATE_FC_COMMITTING is
 		 * visible before we send the wakeup. Pairs with implicit
-		 * barrier in prepare_to_wait() in ext4_fc_track_inode().
+		 * barrier in prepare_to_wait() in ext4_fc_del().
 		 */
 		smp_mb();
 #if (BITS_PER_LONG < 64)
-- 
2.53.0


^ permalink raw reply related

* [RFC v6 2/7] ext4: lockdep: handle i_data_sem subclassing for special inodes
From: Li Chen @ 2026-04-08 11:20 UTC (permalink / raw)
  To: Zhang Yi, Theodore Ts'o, Andreas Dilger, Baokun Li, Jan Kara,
	Ojaswin Mujoo, Ritesh Harjani (IBM), Zhang Yi, linux-ext4,
	linux-kernel
  Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
	linux-trace-kernel, Li Chen
In-Reply-To: <20260408112020.716706-1-me@linux.beauty>

Fast commit can hold s_fc_lock while writing journal blocks. Mapping the
journal inode can take its i_data_sem. Normal inode update paths can take a
data inode i_data_sem and then s_fc_lock, which makes lockdep report a
circular dependency.

lockdep treats all i_data_sem instances as one lock class and cannot
distinguish the journal inode i_data_sem from a regular inode i_data_sem.
The journal inode is not tracked by fast commit and no FC waiters ever
depend on it, so this is not a real ABBA deadlock. Assign the journal inode
a dedicated i_data_sem lockdep subclass to avoid the false positive.

Inode cache objects can be recycled, so also reset i_data_sem to
I_DATA_SEM_NORMAL when allocating an ext4 inode. Otherwise a new inode may
inherit an old subclass (journal/quota/ea) and trigger lockdep warnings.

Signed-off-by: Li Chen <me@linux.beauty>
---
Changes in v6:
- Rebase onto linux-next master as of 2026-04-08.
- Refresh the patch context around upstream ext4_alloc_inode() changes,
  without changing the subclassing logic.

 fs/ext4/ext4.h  | 4 +++-
 fs/ext4/super.c | 8 ++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 98857292c707..66de888ae411 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1016,12 +1016,14 @@ do {										\
  *			  than the first
  *  I_DATA_SEM_QUOTA  - Used for quota inodes only
  *  I_DATA_SEM_EA     - Used for ea_inodes only
+ *  I_DATA_SEM_JOURNAL - Used for journal inode only
  */
 enum {
 	I_DATA_SEM_NORMAL = 0,
 	I_DATA_SEM_OTHER,
 	I_DATA_SEM_QUOTA,
-	I_DATA_SEM_EA
+	I_DATA_SEM_EA,
+	I_DATA_SEM_JOURNAL
 };
 
 struct ext4_fc_inode_snap;
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 578508eb4f1a..286f05834900 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1425,6 +1425,9 @@ static struct inode *ext4_alloc_inode(struct super_block *sb)
 	ext4_fc_init_inode(&ei->vfs_inode);
 	spin_lock_init(&ei->i_fc_lock);
 	mmb_init(&ei->i_metadata_bhs, &ei->vfs_inode.i_data);
+#ifdef CONFIG_LOCKDEP
+	lockdep_set_subclass(&ei->i_data_sem, I_DATA_SEM_NORMAL);
+#endif
 	return &ei->vfs_inode;
 }
 
@@ -5904,6 +5907,11 @@ static struct inode *ext4_get_journal_inode(struct super_block *sb,
 		return ERR_PTR(-EFSCORRUPTED);
 	}
 
+#ifdef CONFIG_LOCKDEP
+	lockdep_set_subclass(&EXT4_I(journal_inode)->i_data_sem,
+			     I_DATA_SEM_JOURNAL);
+#endif
+
 	ext4_debug("Journal inode found at %p: %lld bytes\n",
 		  journal_inode, journal_inode->i_size);
 	return journal_inode;
-- 
2.53.0

^ permalink raw reply related

* [RFC v6 1/7] ext4: fast commit: snapshot inode state before writing log
From: Li Chen @ 2026-04-08 11:20 UTC (permalink / raw)
  To: Zhang Yi, Theodore Ts'o, Andreas Dilger, Baokun Li, Jan Kara,
	Ojaswin Mujoo, Ritesh Harjani (IBM), Zhang Yi, linux-ext4,
	linux-kernel
  Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
	linux-trace-kernel, Li Chen
In-Reply-To: <20260408112020.716706-1-me@linux.beauty>

Fast commit writes inode metadata and data range updates after unlocking
journal updates. New handles can start at that point, so the log writing
path must not look at live inode state.

Add a commit-time per-inode snapshot and populate it while journal updates
are locked and existing handles are drained. Store the snapshot behind
ext4_inode_info->i_fc_snap so ext4_inode_info only grows by one pointer.
The snapshot contains a copy of the on-disk inode plus the data range
records needed for fast commit TLVs.

Snapshotting runs under jbd2_journal_lock_updates(). Avoid triggering I/O
there by using ext4_get_inode_loc_noio() and falling back to full commit
if the inode table block is not present or not uptodate.

Log writing then only serializes the snapshot, so it no longer needs to
call ext4_map_blocks() and take i_data_sem under s_fc_lock. The snapshot
is installed and freed under s_fc_lock and is released from fast commit
cleanup and inode eviction.

Signed-off-by: Li Chen <me@linux.beauty>
---
Changes in v6:
- Rebase onto linux-next master as of 2026-04-08.
- Fix the inode debug print format after rebasing.

 fs/ext4/ext4.h        |  22 ++-
 fs/ext4/fast_commit.c | 331 +++++++++++++++++++++++++++++++++++-------
 fs/ext4/inode.c       |  51 +++++++
 3 files changed, 352 insertions(+), 52 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 0cf68f85dfd1..98857292c707 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1024,6 +1024,7 @@ enum {
 	I_DATA_SEM_EA
 };
 
+struct ext4_fc_inode_snap;
 
 /*
  * fourth extended file system inode data in memory
@@ -1080,6 +1081,22 @@ struct ext4_inode_info {
 	/* End of lblk range that needs to be committed in this fast commit */
 	ext4_lblk_t i_fc_lblk_len;
 
+	/*
+	 * Commit-time fast commit snapshots.
+	 *
+	 * i_fc_snap is installed and freed under sbi->s_fc_lock. The fast
+	 * commit log writing path reads the snapshot under sbi->s_fc_lock while
+	 * serializing fast commit TLVs.
+	 *
+	 * The snapshot lifetime is bounded by EXT4_STATE_FC_COMMITTING and the
+	 * corresponding cleanup / eviction paths.
+	 *
+	 * i_fc_snap points to per-inode snapshot data for fast commit:
+	 * - a raw inode snapshot for EXT4_FC_TAG_INODE
+	 * - data range records for EXT4_FC_TAG_{ADD,DEL}_RANGE
+	 */
+	struct ext4_fc_inode_snap *i_fc_snap;
+
 	spinlock_t i_raw_lock;	/* protects updates to the raw inode */
 
 	/* Fast commit wait queue for this inode */
@@ -3083,8 +3100,9 @@ extern int  ext4_file_getattr(struct mnt_idmap *, const struct path *,
 			      struct kstat *, u32, unsigned int);
 extern void ext4_dirty_inode(struct inode *, int);
 extern int ext4_change_inode_journal_flag(struct inode *, int);
-extern int ext4_get_inode_loc(struct inode *, struct ext4_iloc *);
-extern int ext4_get_fc_inode_loc(struct super_block *sb, unsigned long ino,
+int ext4_get_inode_loc(struct inode *inode, struct ext4_iloc *iloc);
+int ext4_get_inode_loc_noio(struct inode *inode, struct ext4_iloc *iloc);
+int ext4_get_fc_inode_loc(struct super_block *sb, unsigned long ino,
 			  struct ext4_iloc *iloc);
 extern int ext4_inode_attach_jinode(struct inode *inode);
 extern int ext4_can_truncate(struct inode *inode);
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 390f25dee2b1..dc19795dacdd 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -55,21 +55,23 @@
  *     deleted while it is being flushed.
  * [2] Flush data buffers to disk and clear "EXT4_STATE_FC_FLUSHING_DATA"
  *     state.
- * [3] Lock the journal by calling jbd2_journal_lock_updates. This ensures that
- *     all the exsiting handles finish and no new handles can start.
- * [4] Mark all the fast commit eligible inodes as undergoing fast commit
- *     by setting "EXT4_STATE_FC_COMMITTING" state.
- * [5] Unlock the journal by calling jbd2_journal_unlock_updates. This allows
+ * [3] Lock the journal by calling jbd2_journal_lock_updates(). This ensures
+ *     that all the existing handles finish and no new handles can start.
+ * [4] Mark all the fast commit eligible inodes as undergoing fast commit by
+ *     setting "EXT4_STATE_FC_COMMITTING" state, and snapshot the inode state
+ *     needed for log writing.
+ * [5] Unlock the journal by calling jbd2_journal_unlock_updates(). This allows
  *     starting of new handles. If new handles try to start an update on
  *     any of the inodes that are being committed, ext4_fc_track_inode()
  *     will block until those inodes have finished the fast commit.
  * [6] Commit all the directory entry updates in the fast commit space.
- * [7] Commit all the changed inodes in the fast commit space and clear
- *     "EXT4_STATE_FC_COMMITTING" for these inodes.
+ * [7] Commit all the changed inodes in the fast commit space.
  * [8] Write tail tag (this tag ensures the atomicity, please read the following
  *     section for more details).
+ * [9] Clear "EXT4_STATE_FC_COMMITTING" and wake up waiters in
+ *     ext4_fc_cleanup().
  *
- * All the inode updates must be enclosed within jbd2_jounrnal_start()
+ * All the inode updates must be enclosed within jbd2_journal_start()
  * and jbd2_journal_stop() similar to JBD2 journaling.
  *
  * Fast Commit Ineligibility
@@ -199,6 +201,8 @@ static void ext4_end_buffer_io_sync(struct buffer_head *bh, int uptodate)
 	unlock_buffer(bh);
 }
 
+static void ext4_fc_free_inode_snap(struct inode *inode);
+
 static inline void ext4_fc_reset_inode(struct inode *inode)
 {
 	struct ext4_inode_info *ei = EXT4_I(inode);
@@ -215,6 +219,7 @@ void ext4_fc_init_inode(struct inode *inode)
 	ext4_clear_inode_state(inode, EXT4_STATE_FC_COMMITTING);
 	INIT_LIST_HEAD(&ei->i_fc_list);
 	INIT_LIST_HEAD(&ei->i_fc_dilist);
+	ei->i_fc_snap = NULL;
 	init_waitqueue_head(&ei->i_fc_wait);
 }
 
@@ -240,6 +245,7 @@ void ext4_fc_del(struct inode *inode)
 
 	alloc_ctx = ext4_fc_lock(inode->i_sb);
 	if (list_empty(&ei->i_fc_list) && list_empty(&ei->i_fc_dilist)) {
+		ext4_fc_free_inode_snap(inode);
 		ext4_fc_unlock(inode->i_sb, alloc_ctx);
 		return;
 	}
@@ -281,6 +287,7 @@ void ext4_fc_del(struct inode *inode)
 		}
 		finish_wait(wq, &wait.wq_entry);
 	}
+	ext4_fc_free_inode_snap(inode);
 	list_del_init(&ei->i_fc_list);
 
 	/*
@@ -845,6 +852,21 @@ static bool ext4_fc_add_dentry_tlv(struct super_block *sb, u32 *crc,
 	return true;
 }
 
+struct ext4_fc_range {
+	struct list_head list;
+	u16 tag;
+	ext4_lblk_t lblk;
+	ext4_lblk_t len;
+	ext4_fsblk_t pblk;
+	bool unwritten;
+};
+
+struct ext4_fc_inode_snap {
+	struct list_head data_list;
+	unsigned int inode_len;
+	u8 inode_buf[];
+};
+
 /*
  * Writes inode in the fast commit space under TLV with tag @tag.
  * Returns 0 on success, error on failure.
@@ -852,21 +874,21 @@ static bool ext4_fc_add_dentry_tlv(struct super_block *sb, u32 *crc,
 static int ext4_fc_write_inode(struct inode *inode, u32 *crc)
 {
 	struct ext4_inode_info *ei = EXT4_I(inode);
-	int inode_len = EXT4_GOOD_OLD_INODE_SIZE;
-	int ret;
-	struct ext4_iloc iloc;
+	struct ext4_fc_inode_snap *snap = ei->i_fc_snap;
 	struct ext4_fc_inode fc_inode;
 	struct ext4_fc_tl tl;
 	u8 *dst;
+	u8 *src;
+	int inode_len;
+	int ret;
 
-	ret = ext4_get_inode_loc(inode, &iloc);
-	if (ret)
-		return ret;
+	if (!snap)
+		return -ECANCELED;
 
-	if (ext4_test_inode_flag(inode, EXT4_INODE_INLINE_DATA))
-		inode_len = EXT4_INODE_SIZE(inode->i_sb);
-	else if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE)
-		inode_len += ei->i_extra_isize;
+	src = snap->inode_buf;
+	inode_len = snap->inode_len;
+	if (!src || inode_len == 0)
+		return -ECANCELED;
 
 	fc_inode.fc_ino = cpu_to_le32(inode->i_ino);
 	tl.fc_tag = cpu_to_le16(EXT4_FC_TAG_INODE);
@@ -882,10 +904,9 @@ static int ext4_fc_write_inode(struct inode *inode, u32 *crc)
 	dst += EXT4_FC_TAG_BASE_LEN;
 	memcpy(dst, &fc_inode, sizeof(fc_inode));
 	dst += sizeof(fc_inode);
-	memcpy(dst, (u8 *)ext4_raw_inode(&iloc), inode_len);
+	memcpy(dst, src, inode_len);
 	ret = 0;
 err:
-	brelse(iloc.bh);
 	return ret;
 }
 
@@ -895,12 +916,74 @@ static int ext4_fc_write_inode(struct inode *inode, u32 *crc)
  */
 static int ext4_fc_write_inode_data(struct inode *inode, u32 *crc)
 {
-	ext4_lblk_t old_blk_size, cur_lblk_off, new_blk_size;
 	struct ext4_inode_info *ei = EXT4_I(inode);
-	struct ext4_map_blocks map;
+	struct ext4_fc_inode_snap *snap = ei->i_fc_snap;
 	struct ext4_fc_add_range fc_ext;
 	struct ext4_fc_del_range lrange;
 	struct ext4_extent *ex;
+	struct ext4_fc_range *range;
+
+	if (!snap)
+		return -ECANCELED;
+
+	list_for_each_entry(range, &snap->data_list, list) {
+		if (range->tag == EXT4_FC_TAG_DEL_RANGE) {
+			lrange.fc_ino = cpu_to_le32(inode->i_ino);
+			lrange.fc_lblk = cpu_to_le32(range->lblk);
+			lrange.fc_len = cpu_to_le32(range->len);
+			if (!ext4_fc_add_tlv(inode->i_sb, EXT4_FC_TAG_DEL_RANGE,
+					     sizeof(lrange), (u8 *)&lrange, crc))
+				return -ENOSPC;
+			continue;
+		}
+
+		fc_ext.fc_ino = cpu_to_le32(inode->i_ino);
+		ex = (struct ext4_extent *)&fc_ext.fc_ex;
+		ex->ee_block = cpu_to_le32(range->lblk);
+		ex->ee_len = cpu_to_le16(range->len);
+		ext4_ext_store_pblock(ex, range->pblk);
+		if (range->unwritten)
+			ext4_ext_mark_unwritten(ex);
+		else
+			ext4_ext_mark_initialized(ex);
+
+		if (!ext4_fc_add_tlv(inode->i_sb, EXT4_FC_TAG_ADD_RANGE,
+				     sizeof(fc_ext), (u8 *)&fc_ext, crc))
+			return -ENOSPC;
+	}
+
+	return 0;
+}
+
+static void ext4_fc_free_ranges(struct list_head *head)
+{
+	struct ext4_fc_range *range, *range_n;
+
+	list_for_each_entry_safe(range, range_n, head, list) {
+		list_del(&range->list);
+		kfree(range);
+	}
+}
+
+static void ext4_fc_free_inode_snap(struct inode *inode)
+{
+	struct ext4_inode_info *ei = EXT4_I(inode);
+	struct ext4_fc_inode_snap *snap = ei->i_fc_snap;
+
+	if (!snap)
+		return;
+
+	ext4_fc_free_ranges(&snap->data_list);
+	kfree(snap);
+	ei->i_fc_snap = NULL;
+}
+
+static int ext4_fc_snapshot_inode_data(struct inode *inode,
+				       struct list_head *ranges)
+{
+	struct ext4_inode_info *ei = EXT4_I(inode);
+	ext4_lblk_t start_lblk, end_lblk, cur_lblk;
+	struct ext4_map_blocks map;
 	int ret;
 
 	spin_lock(&ei->i_fc_lock);
@@ -908,18 +991,21 @@ static int ext4_fc_write_inode_data(struct inode *inode, u32 *crc)
 		spin_unlock(&ei->i_fc_lock);
 		return 0;
 	}
-	old_blk_size = ei->i_fc_lblk_start;
-	new_blk_size = ei->i_fc_lblk_start + ei->i_fc_lblk_len - 1;
+	start_lblk = ei->i_fc_lblk_start;
+	end_lblk = ei->i_fc_lblk_start + ei->i_fc_lblk_len - 1;
 	ei->i_fc_lblk_len = 0;
 	spin_unlock(&ei->i_fc_lock);
 
-	cur_lblk_off = old_blk_size;
-	ext4_debug("will try writing %d to %d for inode %llu\n",
-		   cur_lblk_off, new_blk_size, inode->i_ino);
+	cur_lblk = start_lblk;
+	ext4_debug("snapshot data ranges %u-%u for inode %llu\n",
+		   start_lblk, end_lblk,
+		   (unsigned long long)inode->i_ino);
+
+	while (cur_lblk <= end_lblk) {
+		struct ext4_fc_range *range;
 
-	while (cur_lblk_off <= new_blk_size) {
-		map.m_lblk = cur_lblk_off;
-		map.m_len = new_blk_size - cur_lblk_off + 1;
+		map.m_lblk = cur_lblk;
+		map.m_len = end_lblk - cur_lblk + 1;
 		ret = ext4_map_blocks(NULL, inode, &map,
 				      EXT4_GET_BLOCKS_IO_SUBMIT |
 				      EXT4_EX_NOCACHE);
@@ -927,17 +1013,21 @@ static int ext4_fc_write_inode_data(struct inode *inode, u32 *crc)
 			return -ECANCELED;
 
 		if (map.m_len == 0) {
-			cur_lblk_off++;
+			cur_lblk++;
 			continue;
 		}
 
+		range = kmalloc(sizeof(*range), GFP_NOFS);
+		if (!range)
+			return -ENOMEM;
+
+		range->lblk = map.m_lblk;
+		range->len = map.m_len;
+		range->pblk = 0;
+		range->unwritten = false;
+
 		if (ret == 0) {
-			lrange.fc_ino = cpu_to_le32(inode->i_ino);
-			lrange.fc_lblk = cpu_to_le32(map.m_lblk);
-			lrange.fc_len = cpu_to_le32(map.m_len);
-			if (!ext4_fc_add_tlv(inode->i_sb, EXT4_FC_TAG_DEL_RANGE,
-					    sizeof(lrange), (u8 *)&lrange, crc))
-				return -ENOSPC;
+			range->tag = EXT4_FC_TAG_DEL_RANGE;
 		} else {
 			unsigned int max = (map.m_flags & EXT4_MAP_UNWRITTEN) ?
 				EXT_UNWRITTEN_MAX_LEN : EXT_INIT_MAX_LEN;
@@ -945,26 +1035,67 @@ static int ext4_fc_write_inode_data(struct inode *inode, u32 *crc)
 			/* Limit the number of blocks in one extent */
 			map.m_len = min(max, map.m_len);
 
-			fc_ext.fc_ino = cpu_to_le32(inode->i_ino);
-			ex = (struct ext4_extent *)&fc_ext.fc_ex;
-			ex->ee_block = cpu_to_le32(map.m_lblk);
-			ex->ee_len = cpu_to_le16(map.m_len);
-			ext4_ext_store_pblock(ex, map.m_pblk);
-			if (map.m_flags & EXT4_MAP_UNWRITTEN)
-				ext4_ext_mark_unwritten(ex);
-			else
-				ext4_ext_mark_initialized(ex);
-			if (!ext4_fc_add_tlv(inode->i_sb, EXT4_FC_TAG_ADD_RANGE,
-					    sizeof(fc_ext), (u8 *)&fc_ext, crc))
-				return -ENOSPC;
+			range->tag = EXT4_FC_TAG_ADD_RANGE;
+			range->len = map.m_len;
+			range->pblk = map.m_pblk;
+			range->unwritten = !!(map.m_flags & EXT4_MAP_UNWRITTEN);
 		}
 
-		cur_lblk_off += map.m_len;
+		INIT_LIST_HEAD(&range->list);
+		list_add_tail(&range->list, ranges);
+
+		cur_lblk += map.m_len;
 	}
 
 	return 0;
 }
 
+static int ext4_fc_snapshot_inode(struct inode *inode)
+{
+	struct ext4_inode_info *ei = EXT4_I(inode);
+	struct ext4_fc_inode_snap *snap;
+	int inode_len = EXT4_GOOD_OLD_INODE_SIZE;
+	struct ext4_iloc iloc;
+	LIST_HEAD(ranges);
+	int ret;
+	int alloc_ctx;
+
+	ret = ext4_get_inode_loc_noio(inode, &iloc);
+	if (ret)
+		return ret;
+
+	if (ext4_test_inode_flag(inode, EXT4_INODE_INLINE_DATA))
+		inode_len = EXT4_INODE_SIZE(inode->i_sb);
+	else if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE)
+		inode_len += ei->i_extra_isize;
+
+	snap = kmalloc(struct_size(snap, inode_buf, inode_len), GFP_NOFS);
+	if (!snap) {
+		brelse(iloc.bh);
+		return -ENOMEM;
+	}
+	INIT_LIST_HEAD(&snap->data_list);
+	snap->inode_len = inode_len;
+
+	memcpy(snap->inode_buf, (u8 *)ext4_raw_inode(&iloc), inode_len);
+	brelse(iloc.bh);
+
+	ret = ext4_fc_snapshot_inode_data(inode, &ranges);
+	if (ret) {
+		kfree(snap);
+		ext4_fc_free_ranges(&ranges);
+		return ret;
+	}
+
+	alloc_ctx = ext4_fc_lock(inode->i_sb);
+	ext4_fc_free_inode_snap(inode);
+	ei->i_fc_snap = snap;
+	list_splice_tail_init(&ranges, &snap->data_list);
+	ext4_fc_unlock(inode->i_sb, alloc_ctx);
+
+	return 0;
+}
+
 
 /* Flushes data of all the inodes in the commit queue. */
 static int ext4_fc_flush_data(journal_t *journal)
@@ -1015,6 +1146,11 @@ static int ext4_fc_commit_dentry_updates(journal_t *journal, u32 *crc)
 		 */
 		if (list_empty(&fc_dentry->fcd_dilist))
 			continue;
+		/*
+		 * For EXT4_FC_TAG_CREAT, fcd_dilist is linked on the created
+		 * inode's i_fc_dilist list (kept singular), so we can recover the
+		 * inode through it.
+		 */
 		ei = list_first_entry(&fc_dentry->fcd_dilist,
 				struct ext4_inode_info, i_fc_dilist);
 		inode = &ei->vfs_inode;
@@ -1039,6 +1175,88 @@ static int ext4_fc_commit_dentry_updates(journal_t *journal, u32 *crc)
 	return 0;
 }
 
+static int ext4_fc_snapshot_inodes(journal_t *journal)
+{
+	struct super_block *sb = journal->j_private;
+	struct ext4_sb_info *sbi = EXT4_SB(sb);
+	struct ext4_inode_info *iter;
+	struct ext4_fc_dentry_update *fc_dentry;
+	struct inode **inodes;
+	unsigned int nr_inodes = 0;
+	unsigned int i = 0;
+	int ret = 0;
+	int alloc_ctx;
+
+	alloc_ctx = ext4_fc_lock(sb);
+	list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list)
+		nr_inodes++;
+
+	list_for_each_entry(fc_dentry, &sbi->s_fc_dentry_q[FC_Q_MAIN], fcd_list) {
+		struct ext4_inode_info *ei;
+
+		if (fc_dentry->fcd_op != EXT4_FC_TAG_CREAT)
+			continue;
+		if (list_empty(&fc_dentry->fcd_dilist))
+			continue;
+
+		/* See the comment in ext4_fc_commit_dentry_updates(). */
+		ei = list_first_entry(&fc_dentry->fcd_dilist,
+				      struct ext4_inode_info, i_fc_dilist);
+		if (!list_empty(&ei->i_fc_list))
+			continue;
+
+		nr_inodes++;
+	}
+	ext4_fc_unlock(sb, alloc_ctx);
+
+	if (!nr_inodes)
+		return 0;
+
+	inodes = kvcalloc(nr_inodes, sizeof(*inodes), GFP_NOFS);
+	if (!inodes)
+		return -ENOMEM;
+
+	alloc_ctx = ext4_fc_lock(sb);
+	list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) {
+		inodes[i] = igrab(&iter->vfs_inode);
+		if (inodes[i])
+			i++;
+	}
+
+	list_for_each_entry(fc_dentry, &sbi->s_fc_dentry_q[FC_Q_MAIN], fcd_list) {
+		struct ext4_inode_info *ei;
+
+		if (fc_dentry->fcd_op != EXT4_FC_TAG_CREAT)
+			continue;
+		if (list_empty(&fc_dentry->fcd_dilist))
+			continue;
+
+		/* See the comment in ext4_fc_commit_dentry_updates(). */
+		ei = list_first_entry(&fc_dentry->fcd_dilist,
+				      struct ext4_inode_info, i_fc_dilist);
+		if (!list_empty(&ei->i_fc_list))
+			continue;
+
+		inodes[i] = igrab(&ei->vfs_inode);
+		if (inodes[i])
+			i++;
+	}
+	ext4_fc_unlock(sb, alloc_ctx);
+
+	for (nr_inodes = 0; nr_inodes < i; nr_inodes++) {
+		ret = ext4_fc_snapshot_inode(inodes[nr_inodes]);
+		if (ret)
+			break;
+	}
+
+	for (nr_inodes = 0; nr_inodes < i; nr_inodes++) {
+		if (inodes[nr_inodes])
+			iput(inodes[nr_inodes]);
+	}
+	kvfree(inodes);
+	return ret;
+}
+
 static int ext4_fc_perform_commit(journal_t *journal)
 {
 	struct super_block *sb = journal->j_private;
@@ -1111,7 +1329,11 @@ static int ext4_fc_perform_commit(journal_t *journal)
 				     EXT4_STATE_FC_COMMITTING);
 	}
 	ext4_fc_unlock(sb, alloc_ctx);
+
+	ret = ext4_fc_snapshot_inodes(journal);
 	jbd2_journal_unlock_updates(journal);
+	if (ret)
+		return ret;
 
 	/*
 	 * Step 5: If file system device is different from journal device,
@@ -1308,6 +1530,7 @@ static void ext4_fc_cleanup(journal_t *journal, int full, tid_t tid)
 					struct ext4_inode_info,
 					i_fc_list);
 		list_del_init(&ei->i_fc_list);
+		ext4_fc_free_inode_snap(&ei->vfs_inode);
 		ext4_clear_inode_state(&ei->vfs_inode,
 				       EXT4_STATE_FC_COMMITTING);
 		if (tid_geq(tid, ei->i_sync_tid)) {
@@ -1343,6 +1566,14 @@ static void ext4_fc_cleanup(journal_t *journal, int full, tid_t tid)
 					     struct ext4_fc_dentry_update,
 					     fcd_list);
 		list_del_init(&fc_dentry->fcd_list);
+		if (fc_dentry->fcd_op == EXT4_FC_TAG_CREAT &&
+		    !list_empty(&fc_dentry->fcd_dilist)) {
+			/* See the comment in ext4_fc_commit_dentry_updates(). */
+			ei = list_first_entry(&fc_dentry->fcd_dilist,
+					      struct ext4_inode_info,
+					      i_fc_dilist);
+			ext4_fc_free_inode_snap(&ei->vfs_inode);
+		}
 		list_del_init(&fc_dentry->fcd_dilist);
 
 		release_dentry_name_snapshot(&fc_dentry->fcd_name);
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 64dba7679245..478c81e6d08b 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4939,6 +4939,57 @@ int ext4_get_inode_loc(struct inode *inode, struct ext4_iloc *iloc)
 	return ret;
 }
 
+/*
+ * ext4_get_inode_loc_noio() is a best-effort variant of ext4_get_inode_loc().
+ * It looks up the inode table block in the buffer cache and returns -EAGAIN if
+ * the block is not present or not uptodate, without starting any I/O.
+ */
+int ext4_get_inode_loc_noio(struct inode *inode, struct ext4_iloc *iloc)
+{
+	struct super_block *sb = inode->i_sb;
+	struct ext4_group_desc *gdp;
+	struct buffer_head *bh;
+	ext4_fsblk_t block;
+	int inodes_per_block, inode_offset;
+	unsigned long ino = inode->i_ino;
+
+	iloc->bh = NULL;
+	if (ino < EXT4_ROOT_INO ||
+	    ino > le32_to_cpu(EXT4_SB(sb)->s_es->s_inodes_count))
+		return -EFSCORRUPTED;
+
+	iloc->block_group = (ino - 1) / EXT4_INODES_PER_GROUP(sb);
+	gdp = ext4_get_group_desc(sb, iloc->block_group, NULL);
+	if (!gdp)
+		return -EIO;
+
+	/* Figure out the offset within the block group inode table. */
+	inodes_per_block = EXT4_SB(sb)->s_inodes_per_block;
+	inode_offset = ((ino - 1) % EXT4_INODES_PER_GROUP(sb));
+	iloc->offset = (inode_offset % inodes_per_block) * EXT4_INODE_SIZE(sb);
+
+	block = ext4_inode_table(sb, gdp);
+	if (block <= le32_to_cpu(EXT4_SB(sb)->s_es->s_first_data_block) ||
+	    block >= ext4_blocks_count(EXT4_SB(sb)->s_es)) {
+		ext4_error(sb,
+			   "Invalid inode table block %llu in block_group %u",
+			   block, iloc->block_group);
+		return -EFSCORRUPTED;
+	}
+	block += inode_offset / inodes_per_block;
+
+	bh = sb_find_get_block(sb, block);
+	if (!bh)
+		return -EAGAIN;
+	if (!ext4_buffer_uptodate(bh)) {
+		brelse(bh);
+		return -EAGAIN;
+	}
+
+	iloc->bh = bh;
+	return 0;
+}
+
 
 int ext4_get_fc_inode_loc(struct super_block *sb, unsigned long ino,
 			  struct ext4_iloc *iloc)
-- 
2.53.0

^ permalink raw reply related

* [RFC v6 0/7] ext4: fast commit: snapshot inode state for FC log
From: Li Chen @ 2026-04-08 11:20 UTC (permalink / raw)
  To: Zhang Yi, Theodore Ts'o, Andreas Dilger
  Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, linux-ext4,
	linux-trace-kernel, linux-kernel

Hi,

(This RFC v6 series is rebased onto linux-next master as of 2026-04-08,
commit f3e6330d7fe4 ("Add linux-next specific files for 20260407").)

Zhang Yi in RFC v3 review pointed out that postponing lockdep assertions only
masks the issue, and that sleeping in ext4_fc_track_inode() while holding
i_data_sem can form a real ABBA deadlock if the fast commit writer also needs
i_data_sem while the inode is in FC_COMMITTING.

Zhang Yi suggested two possible directions to address the root cause:

1. "Ha, the solution seems to have already been listed in the TODOs in
fast_commit.c.

  Change ext4_fc_commit() to lookup logical to physical mapping using extent
  status tree. This would get rid of the need to call ext4_fc_track_inode()
  before acquiring i_data_sem. To do that we would need to ensure that
  modified extents from the extent status tree are not evicted from memory."

2. "Alternatively, recording the mapped range of tracking might also be
feasible."

This series implements a hybrid way: it implements approach 2 by snapshotting inode image
and mapped ranges at commit time, and consuming only snapshots during log
writing.

Approach 2 still needs a mapping source while building the snapshot
(logical-to-physical and unwritten/hole semantics). Calling ext4_map_blocks()
there would take i_data_sem and can block inside the
jbd2_journal_lock_updates() window, which risks deadlocks or unbounded stalls.
So the snapshot path uses approach 1's extent status lookups as a best-effort
mapping source to avoid ext4_map_blocks().

I did not fully implement approach 1 (making extent status lookups
authoritative by preventing reclaim of needed entries) because that would need
additional pinning/integration under memory pressure and a larger correctness
surface. Instead, the extent status tree is treated as a cache and the
snapshot path falls back to full commit on cache misses or unstable mappings
(e.g. delayed allocation).

Lock inversion / deadlock model (before):

CPU0 (metadata update)               CPU1 (fast commit)
--------------------               -----------------
... hold i_data_sem (A)             mutex_lock(s_fc_lock) (B)
    ext4_fc_track_inode()             ext4_fc_write_inode_data()
      mutex_lock(s_fc_lock) (B)         ext4_map_blocks()
      wait FC_COMMITTING (sleep)          down_read(i_data_sem) (A)

This creates i_data_sem (A) -> s_fc_lock (B) on update paths, and
s_fc_lock (B) -> i_data_sem (A) on commit paths. Once CPU0 sleeps while
holding (A), CPU1 can block on (A) while holding (B), completing the ABBA
cycle.

New model (this series):

CPU0 (metadata update)               CPU1 (fast commit)
--------------------               -----------------
... maybe hold i_data_sem (A)        jbd2_journal_lock_updates()
    ext4_fc_track_*()                 snapshot inode + ranges (no map_blocks)
      mutex_lock(s_fc_lock) (B)       jbd2_journal_unlock_updates()
      if FC_COMMITTING: set FC_REQUEUE s_fc_lock (B)
      no sleep                         write FC log from snapshots only
                                    cleanup: clear COMMITTING, requeue if set

The commit path no longer takes i_data_sem while holding s_fc_lock, and
tracking no longer sleeps waiting for FC_COMMITTING. If an inode is updated
during a fast commit, EXT4_STATE_FC_REQUEUE records that fact and the inode
is moved to FC_Q_STAGING for the next commit.
The only remaining FC_COMMITTING waiter is ext4_fc_del(), which drops
s_fc_lock before sleeping.

This series snapshots the on-disk inode and tracked data ranges while journal
updates are locked and existing handles are drained. The log writing phase then
serializes only snapshots, so it no longer needs to call ext4_map_blocks() and
take i_data_sem under s_fc_lock. This is done in two steps: patch 1 drops
ext4_map_blocks() from log writing by introducing commit-time snapshots, and
patch 5 drops ext4_map_blocks() from the snapshot path by using the extent
status cache. The snapshot also records whether a mapped extent is unwritten,
so the ADD_RANGE records (and replay) preserve unwritten semantics.

Snapshotting runs under jbd2_journal_lock_updates(). Since a cache miss in
ext4_get_inode_loc() can start synchronous inode table I/O and stall handle
starts for milliseconds, patch 1 uses ext4_get_inode_loc_noio() and falls back
to full commit if the inode table block is not present or not uptodate.

ext4_fc_track_inode() also stops waiting for FC_COMMITTING. Updates during an
ongoing fast commit are marked with EXT4_STATE_FC_REQUEUE and are replayed in
the next fast commit, while ext4_fc_del() waits for FC_COMMITTING so an inode
cannot be removed while the commit thread is still using it.

The extent status tree is a cache, not an authoritative source, so the snapshot
path falls back to full commit on cache misses or unstable mappings (e.g.
delayed allocation). This includes cases where extent status entries are not
present (or have been reclaimed) under memory pressure. The snapshot path does
not try to rebuild mappings by calling ext4_map_blocks(); instead it simply
marks the transaction fast commit ineligible.

To keep the updates-locked window bounded, the snapshot path caps the number of
snapshotted inodes and ranges per fast commit (currently 1024 inodes and 2048
ranges) and falls back to full commit when the cap is exceeded. The series also
handles the journal inode i_data_sem lockdep false positive via subclassing;
journal inode mapping may still take i_data_sem even when data inode mapping is
avoided.

Patch 6 adds the ext4_fc_lock_updates tracepoint to quantify the updates-locked
window and snapshot fallback reasons. Patch 7 extends
/proc/fs/ext4/<sb_id>/fc_info with best-effort snapshot counters. If the /proc
interface is undesirable, I can drop patch 7 and keep the tracepoint only, or
drop even both.

Testing and measurement were done on a QEMU/KVM guest with virtio-pmem + dax
(ext4 -O fast_commit, mounted dax,noatime). The workload does python3 500x
{4K write + fsync}, fallocate 256M, and python3 500x {creat + fsync(dir)}.
Over 3 cold boots, ext4_fc_lock_updates reported locked_ns p50 2.88-2.92 us,
p99 <= 6.71 us, and max <= 102.71 us, with snap_err always 0. Under stress-ng
memory pressure (stress-ng --vm 4 --vm-bytes 75% --timeout 60s), locked_ns p50
2.94 us, p99 <= 4.97 us, and max <= 20.07 us. The fc_info snapshot failure
counters stayed at 0.
These hold times are in the low microseconds range, and the caps keep the
worst case bounded.

Comments and guidance are very welcome. Please let me know if there are any
concerns about correctness, corner cases, or better approaches.

RFC v5 -> RFC v6:
- Rebase onto linux-next master as of 2026-04-08.
- Address tracepoint review feedback by relying on enum auto-increment for
  snap_err values and by switching the guarded ext4_fc_lock_updates call site
  to trace_call__ext4_fc_lock_updates() to avoid the double static_branch. [1]
- Keep lock window accounting unconditional for fc_info while using the guarded
  direct tracepoint call.
- Fix the inode debug print format exposed by the rebase.

RFC v4 -> RFC v5:
- Patch 6: Make ext4_fc_lock_updates snap_err human readable via
  TRACE_DEFINE_ENUM() + __print_symbolic(), using a single TRACE_SNAP_ERR
  mapping while keeping the enum values stable for tooling.

RFC v3 -> RFC v4:
- Replace lockdep_assert movement with removing the wait in
  ext4_fc_track_inode() and using EXT4_STATE_FC_REQUEUE to capture updates
  during an ongoing fast commit.
- Replace dropping s_fc_lock around log writing with commit-time snapshots of
  inode image and mapped ranges (recording the mapped range of tracking as
  suggested by Zhang Yi) so log writing consumes only snapshots.
- Avoid inode table I/O under jbd2_journal_lock_updates() via
  ext4_get_inode_loc_noio() and fallback to full commit on cache misses.
- Use the extent status cache for snapshot mappings and fall back to full
  commit on cache misses or unstable mappings (e.g. delayed allocation).
- Add tracepoint and /proc snapshot stats to quantify the updates-locked window
  and snapshot fallback reasons.

RFC v2 -> RFC v3:
- rebase on top of
  https://lore.kernel.org/linux-ext4/20251223131342.287864-1-me@linux.beauty/T/#u

RFC v1 -> RFC v2:
- patch 1: move comments to correct place
- patch 2: add it to patchset.
- add missing RFC prefix

RFC v1: https://lore.kernel.org/linux-ext4/20251222032655.87056-1-me@linux.beauty/T/#u
RFC v2: https://lore.kernel.org/linux-ext4/20251222151906.24607-1-me@linux.beauty/T/#t
RFC v3: https://lore.kernel.org/linux-ext4/20251224032943.134063-1-me@linux.beauty/
RFC v4: https://lore.kernel.org/all/20260120112538.132774-1-me@linux.beauty/
RFC v5: https://lore.kernel.org/all/20260317084624.457185-1-me@linux.beauty/t/#u

[1]: https://lore.kernel.org/all/acZJl8QUYEq8voqQ@BLRRASHENOY1.amd.com/T/#u

Thanks,

Li Chen (7):
  ext4: fast commit: snapshot inode state before writing log
  ext4: lockdep: handle i_data_sem subclassing for special inodes
  ext4: fast commit: avoid waiting for FC_COMMITTING
  ext4: fast commit: avoid self-deadlock in inode snapshotting
  ext4: fast commit: avoid i_data_sem by dropping ext4_map_blocks() in
    snapshots
  ext4: fast commit: add lock_updates tracepoint
  ext4: fast commit: export snapshot stats in fc_info

 fs/ext4/ext4.h              |  73 +++-
 fs/ext4/fast_commit.c       | 706 +++++++++++++++++++++++++++++-------
 fs/ext4/inode.c             |  51 +++
 fs/ext4/super.c             |   9 +
 include/trace/events/ext4.h |  61 ++++
 5 files changed, 766 insertions(+), 134 deletions(-)

-- 
2.53.0

^ permalink raw reply

* [syzbot] Monthly ext4 report (Apr 2026)
From: syzbot @ 2026-04-08  6:44 UTC (permalink / raw)
  To: linux-ext4, linux-kernel, syzkaller-bugs

Hello ext4 maintainers/developers,

This is a 31-day syzbot report for the ext4 subsystem.
All related reports/information can be found at:
https://syzkaller.appspot.com/upstream/s/ext4

During the period, 0 new issues were detected and 2 were fixed.
In total, 49 issues are still open and 175 have already been fixed.

Some of the still happening issues:

Ref  Crashes Repro Title
<1>  7355    Yes   KASAN: out-of-bounds Read in ext4_xattr_set_entry
                   https://syzkaller.appspot.com/bug?extid=f792df426ff0f5ceb8d1
<2>  6598    Yes   WARNING in ext4_xattr_inode_update_ref (2)
                   https://syzkaller.appspot.com/bug?extid=76916a45d2294b551fd9
<3>  5264    Yes   possible deadlock in ext4_writepages (2)
                   https://syzkaller.appspot.com/bug?extid=eb5b4ef634a018917f3c
<4>  3018    Yes   kernel BUG in ext4_do_writepages
                   https://syzkaller.appspot.com/bug?extid=d1da16f03614058fdc48
<5>  2945    Yes   INFO: task hung in sync_inodes_sb (5)
                   https://syzkaller.appspot.com/bug?extid=30476ec1b6dc84471133
<6>  2495    Yes   possible deadlock in ext4_destroy_inline_data (2)
                   https://syzkaller.appspot.com/bug?extid=bb2455d02bda0b5701e3
<7>  2178    Yes   INFO: task hung in jbd2_journal_commit_transaction (5)
                   https://syzkaller.appspot.com/bug?extid=3071bdd0a9953bc0d177
<8>  994     Yes   WARNING in ext4_xattr_inode_lookup_create
                   https://syzkaller.appspot.com/bug?extid=fe42a669c87e4a980051
<9>  658     Yes   possible deadlock in do_writepages (2)
                   https://syzkaller.appspot.com/bug?extid=756f498a88797cda9299
<10> 464     Yes   INFO: task hung in do_get_write_access (3)
                   https://syzkaller.appspot.com/bug?extid=e7c786ece54bad9d1e43

---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

To disable reminders for individual bugs, reply with the following command:
#syz set <Ref> no-reminders

To change bug's subsystems, reply with:
#syz set <Ref> subsystems: new-subsystem

You may send multiple commands in a single email message.

^ permalink raw reply

* Re: [RFC PATCH v1 0/6] provenance_time (ptime): a new settable timestamp for cross-filesystem provenance
From: Sean Smith @ 2026-04-08  2:54 UTC (permalink / raw)
  To: Theodore Tso
  Cc: linux-fsdevel, linux-ext4, linux-btrfs, dsterba, david, brauner,
	osandov, almaz, hirofumi, linkinjeon
In-Reply-To: <20260407233618.GB12536@macsyma-wired.lan>



On 4/7/2026 18:36, Theodore Tso wrote:
> Yelch.   This is so *very* non-Unixy / non-POSIX / non-Linux.
> 
> I understand why it's convenient for your particular use case, but
> rename(2) is fundamentally an operation which works on directory
> entries.  We don't copy over extended attributes, or Posix ACL's, or
> Unix permission mode bits, because (a) that would violate POSIX and
> historical Unix behavior, and (b) because rename(2) is fundamentally a
> directory entry operation.  This is despite the fact that it be more
> convenient if we didn't have to copy over things like extended
> attributes, ACL's, permission mode bits, etc.  So you want to make an
> exception for ptime?   Yelch.  Just Yelch.
> 
> 						- Ted

[written by Sean]

Finding an alternative to using rename() to transfer ptime between
inodes during atomic saves seems beyond the scope of what I can address
as someone who relies upon AI agents to review and modify code. From
what my agents have been able to explain to me, the options here are
using rename() to transfer ptime via the kernel, or using renameat2 to
require each application to manually preserve/transfer ptime. The latter
is, well, the phrase herding cats might be an understatement of the
difficulty involved.

As an individual, I don't see how I could convince every major
open-source and closed source application developer for Linux to code
their application to preserve and transfer ptime. That seems like
something only leadership in the community has any chance of doing, and
even then, that would be a very slow-moving process.

It also doesn't solve the immediate needs of increasing number of users
who are trying to ditch Windows for Linux. Windows 11 has pushed one too
many people too far, and they, like me, have had enough.

Maybe senior developers can find an alternative to rename() that works.
I can cross my fingers and hope. Discussing matters with my agents we
couldn't find one. The need for ptime is very real, and the code in my
patch gets the job done, but a solution that respects conventions
requires a level of expertise I don't have. Perhaps in 4-8 months when
my AI harness is more mature and smarter models are available we can
solve this.

If the rename() code making atomic saves work prevents an upstream
merge, I understand. It would be messy adding ptime to the kernel only
to have it disappear anytime any unpatched application modified a file.
Avoiding such failure modes is why it seemed necessary to take a kernel
level approach.

Additionally, the reason an xattr ptime isn't a viable solution is
because applications do not reliably support xattr transfer. Thus it
would not seem likely ptime would receive better support.

I can patch every application I use which is open-source, or I can patch
the kernel. Rational analysis requires that I patch the kernel.

I'll continue using the rename-over approach in my own system, and
maintaining a github repo so that others who need it can patch their own
kernels. As the phrase goes, it's better than nothing. If there's a path
that respects conventions, I'm genuinely interested in guidance.

- Sean


^ permalink raw reply

* Re: [PATCH v4 0/4] jbd2/ext4/ocfs2: lockless jinode dirty range
From: Li Chen @ 2026-04-08  2:26 UTC (permalink / raw)
  To: Theodore Ts'o, Jan Kara, Mark Fasheh, linux-ext4, ocfs2-devel
  Cc: Andreas Dilger, Joel Becker, Joseph Qi, linux-kernel
In-Reply-To: <20260306085643.465275-1-me@linux.beauty>

Hi,

One more note: if there are any remaining concerns or comments on the
series, please let me know and I'll address them promptly.

Thanks,
Li

^ permalink raw reply

* Re: [PATCH v4 0/4] jbd2/ext4/ocfs2: lockless jinode dirty range
From: Li Chen @ 2026-04-08  2:12 UTC (permalink / raw)
  To: Theodore Ts'o, Jan Kara, Mark Fasheh, linux-ext4, ocfs2-devel
  Cc: Andreas Dilger, Joel Becker, Joseph Qi, linux-kernel
In-Reply-To: <20260306085643.465275-1-me@linux.beauty>

Hi,

Just a gentle ping on this series posted on March 6, 2026.

The current v4 has Reviewed-by tags from Jan on all four patches.
If there are no further concerns, could you please consider it for
merging?

I'm happy to resend if that would help.

Thanks,
Li

^ permalink raw reply

* Re: [RFC PATCH v1 0/6] provenance_time (ptime): a new settable timestamp for cross-filesystem provenance
From: Theodore Tso @ 2026-04-07 23:36 UTC (permalink / raw)
  To: Sean Smith
  Cc: linux-fsdevel, linux-ext4, linux-btrfs, dsterba, david, brauner,
	osandov, almaz, hirofumi, linkinjeon
In-Reply-To: <20260407000558.417-1-DefendTheDisabled@gmail.com>

On Mon, Apr 06, 2026 at 07:05:55PM -0500, Sean Smith wrote:
> The patches implement rename-over preservation in all 5
> filesystem rename handlers. When rename(source, target)
> replaces an existing file, and the source has ptime=0 (the
> default for any newly-created temp file) while the target
> has ptime != 0, the filesystem copies the target's ptime to
> the source before destroying the target's inode. This runs
> inside the rename transaction, atomic with the rename itself.

Yelch.   This is so *very* non-Unixy / non-POSIX / non-Linux.

I understand why it's convenient for your particular use case, but
rename(2) is fundamentally an operation which works on directory
entries.  We don't copy over extended attributes, or Posix ACL's, or
Unix permission mode bits, because (a) that would violate POSIX and
historical Unix behavior, and (b) because rename(2) is fundamentally a
directory entry operation.  This is despite the fact that it be more
convenient if we didn't have to copy over things like extended
attributes, ACL's, permission mode bits, etc.  So you want to make an
exception for ptime?   Yelch.  Just Yelch.

						- Ted

^ permalink raw reply

* Re: [PATCH 0/3] show orphan file inode detail info
From: Theodore Tso @ 2026-04-07 20:28 UTC (permalink / raw)
  To: Jan Kara; +Cc: Ye Bin, adilger.kernel, linux-ext4, linux-fsdevel
In-Reply-To: <n4sccudy5avcgnkdhc27rzofzoprxqtwhfrlmsh3yyrj6vbc6d@mmu73gmtawkq>

On Tue, Apr 07, 2026 at 12:29:23PM +0200, Jan Kara wrote:
> I agree listing orphan inodes for a superblock is useful and the usefulness
> could actually go beyond ext4. I imagine the very same problem is there for
> XFS or btrfs so perhaps we could think for a while whether we can provide
> an interface that wouldn't be ext4 specific? Perhaps an ioctl
> (GET_ORPHAN_FILES) that would return an fd and reading from that fd would
> return entries for orphan inodes?

I'm really not a fan of ioctl's returning a fd, but that does seem to
be a thing these days, for better or for worse, and I agree that
having a portable solution that works across multiple file systems
would be a good thing.

> Also regarding information reported about orphan inodes - won't it be better
> interface to just return a list of file handles? Userspace can then do
> whatever it needs with them - open, statx, calling ioctl, etc - so we
> thwart feature creep with people asking us to add more information to the
> interface. This also offloads a lot of security questions about the
> interface to appropriate syscalls. So overall it looks like a win to me.

The problem with using a file handle is that the only way to get the
pathname is to open the file handle, and then call readlink on
/proc/self/fd/NN.  And inodes on the orphan inode list have been
unlinked, so we don't want to allow people to be able to open them.  I
suppose we could allow this via O_PATH, but I'm not sure that this is
guaranteed to work across all filesystems' file handles?

	      	   	      		   - Ted

^ permalink raw reply

* Re: [RFC PATCH v1 0/6] provenance_time (ptime): a new settable timestamp for cross-filesystem provenance
From: Darrick J. Wong @ 2026-04-07 15:17 UTC (permalink / raw)
  To: Sean Smith
  Cc: tytso, linux-fsdevel, linux-ext4, linux-btrfs, dsterba, david,
	brauner, osandov, hirofumi, linkinjeon
In-Reply-To: <e5be4e66-5fb0-41d3-807f-d26f78949c3d@gmail.com>

On Tue, Apr 07, 2026 at 01:06:09AM -0500, Sean Smith wrote:
> 
> [written with AI assistance]
> 
> On 4/6/2026 20:42, Darrick J. Wong wrote:
> 
> > "Standard"... I was about to write a sardonic reply here, but then I
> > remembred that Linux finally *does* have a standard means to transfer
> > some of those newer file attributes: file_getattr/file_setattr.
> >
> > (Go Andrey!)
> >
> > So, I guess all you really need to do is extend struct file_attr and now
> > userspace has a fairly convenient means to propagate the provenance
> > time. 🙂
> 
> Thank you for pointing to file_getattr/file_setattr — this
> is a significantly better API path than our utimensat
> extension. The size-versioned struct file_attr eliminates
> the glibc times[2] limitation entirely, which was one of
> the main upstream concerns with the current approach.
> 
> We will investigate extending struct file_attr with ptime
> fields for v2. The on-disk storage across all 5 filesystems
> and the rename-over preservation are API-independent and
> would remain unchanged. The change is re-plumbing the
> userspace write path from utimensat to file_setattr.
> 
> Two design questions:
> 
> Would you recommend fa_ptime_sec (__u64) + fa_ptime_nsec
> (__u32) matching the statx timespec pattern, or a different
> representation?

That uses less space in the struct, so yes.

> Pali Rohar has announced plans for mask fields in file_attr.
> Should we coordinate with his mask work so ptime can be
> selectively set without read-modify-write?

fa_xflags already provides that coverage for the other fields in struct
file_attr, e.g. a getattr caller can ignore fa_extsize if
FS_XFLAG_EXTSIZE isn't set; and setattr will (iirc) reject nonzero
fa_extsize if FS_XFLAG_EXTSIZE isn't set.

Pali Rohar's work would make it possible to discover which fa_xflags
fields are settable or clearable for a given file.

> > So does the provenance time cover just the file's contents, or the other
> > attributes and xattrs?
> 
> Content only. ptime records when the file's data first came
> into existence. Metadata changes (permissions, owner,
> xattrs) update ctime but leave ptime unchanged. This
> matches the semantics of Windows Date Created and macOS
> creation time.
> 
> > The reason I ask is, does the ptime get copied over for an FICLONE,
> > which maps all of one file's data blocks into another?
> 
> It should, conceptually — the content's provenance doesn't
> change when you clone it. Currently FICLONE shares data
> extents but does not copy inode metadata (timestamps,
> permissions), so ptime would not be automatically
> preserved. The calling tool (e.g., cp --reflink) handles
> timestamp copying separately via the write path.
> 
> The question is whether FICLONE should be enhanced to copy
> ptime from source to destination at the kernel level —
> similar to how rename-over preserves ptime. There is an
> argument for it: if the kernel handles provenance during
> clone, tools don't need to know. But FICLONE doesn't
> currently copy mtime either, so adding ptime alone would
> be inconsistent. Worth discussing.

FICLONE is currently treated like a write, which means that mtime is
updated on the destination file.  For filesystems supporting ptime you'd
probably want the kernel to copy the ptime from src to dest after the
remapping completes.

(Same for exchange-range)

> Btrfs subvolume snapshots are a different case — they do
> preserve ptime because the inodes are COW copies of the
> originals.
> 
> > And by extension, would it also need to be exchanged if you told
> > XFS_IOC_EXCHANGE_RANGE to exchange all contents between two files?
> 
> Yes — if the content moves, the provenance should move
> with it. If files A and B exchange data extents, their
> ptimes should swap. ptime follows the content, not the
> inode identity.
> 
> > (I know, I know, you said XFS was TBDHBD ;))
> 
> Worth considering for a future XFS implementation — and
> the file_attr route you suggested would give XFS a clean
> integration path for ptime alongside FICLONE and
> EXCHANGE_RANGE.
> 
> > Last question: Is the provenance time only useful if the file is
> > immutable?  Either directly via chattr +i, or by enabling fsverity?
> 
> No — ptime is useful regardless of mutability. It records
> when the document was born, the same way Windows Date
> Created works. Editing a document updates mtime but not
> the creation date. Both are independently valuable:
> 
>   ptime: "This file was first created March 15, 2019"
>   mtime: "It was last modified today"
>   btime: "This inode was created when I copied it here"
> 
> Immutable files (chattr +i, fsverity) are a special case
> where ptime has extra forensic strength — the content
> provably hasn't changed since the provenance date. But for
> the primary use case — preserving creation dates across
> cross-platform migrations — mutability doesn't diminish
> ptime's value. A document's creation date remains meaningful
> regardless of subsequent edits.

Got it.

--D

> Sean
> 
> 
> 

^ permalink raw reply

* Re: [PATCH v2 3/3] ext4: derive f_fsid from block device to avoid collisions
From: Theodore Tso @ 2026-04-07 14:47 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Anand Jain, Darrick J. Wong, linux-ext4, linux-btrfs, linux-xfs,
	Anand Jain
In-Reply-To: <adSUiB9L0sFAd04U@infradead.org>

On Mon, Apr 06, 2026 at 10:22:16PM -0700, Christoph Hellwig wrote:
> > Dilemma:
> > While statfs(2) [1] suggests f_fsid is "some random stuff," we know
> > userspace (NFS, systemd) often treats it as a persistent handle.
> > 
> > Do you prefer one of the names above, or is there a more idiomatic ext4
> > naming convention I should follow?
> > 
>
> My take is that anything that should persist should be an on-disk
> feature flag, not a mount option.  But I'm not in charge for ext4

My take is that f_fsid is random stuff, as documented by the
specification, so anyone who tries to depend on it needs to be kept in
a padding room where they can't hurt themselves or their users.

And as far as NFS is concerned, file handles should be based on
the super block UUID, not statfs's f_fsid, and anyone who wants to
mount a snapshot as an NFS exported file system at the same time that
the original file system is mounted is _also_ should be gently coaxed
into a padding room where they can't hurt themselves or their users.

The solution that we've used for people who are cloning block devices
for things like cloud images has been for *years* has been to use
"tune2fs -U random /dev/sda1".  And this works on mounted file system,
and (for example) built into various cloud images for Google Cloud
Engine.

If we want to change statfs's f_fsid, from one set of "Random stuff"
to another set of "Random stuff", I don't really mind, but I don't
think it's worth *either* a mount option, *or* a feature flag, as
either would be confusing for system adminsitrators when some file
systems behave one way, and other file systems behave another.

	       	   	    	  - Ted

^ permalink raw reply

* Re: [PATCH] ext2: reject inodes with zero i_nlink and valid mode in ext2_iget()
From: Jan Kara @ 2026-04-07 14:00 UTC (permalink / raw)
  To: Vasiliy Kovalev
  Cc: Jan Kara, linux-ext4, Andrew Morton, Alexey Dobriyan,
	linux-kernel, lvc-project
In-Reply-To: <20260404152011.2590197-1-kovalev@altlinux.org>

On Sat 04-04-26 18:20:11, Vasiliy Kovalev wrote:
> ext2_iget() already rejects inodes with i_nlink == 0 when i_mode is
> zero or i_dtime is set, treating them as deleted. However, the case of
> i_nlink == 0 with a non-zero mode and zero dtime slips through. Since
> ext2 has no orphan list, such a combination can only result from
> filesystem corruption - a legitimate inode deletion always sets either
> i_dtime or clears i_mode before freeing the inode.
> 
> A crafted image can exploit this gap to present such an inode to the
> VFS, which then triggers WARN_ON inside drop_nlink() (fs/inode.c) via
> ext2_unlink(), ext2_rename() and ext2_rmdir():
> 
> WARNING: CPU: 3 PID: 609 at fs/inode.c:336 drop_nlink+0xad/0xd0 fs/inode.c:336
> CPU: 3 UID: 0 PID: 609 Comm: syz-executor Not tainted 6.12.77+ #1
> Call Trace:
>  <TASK>
>  inode_dec_link_count include/linux/fs.h:2518 [inline]
>  ext2_unlink+0x26c/0x300 fs/ext2/namei.c:295
>  vfs_unlink+0x2fc/0x9b0 fs/namei.c:4477
>  do_unlinkat+0x53e/0x730 fs/namei.c:4541
>  __x64_sys_unlink+0xc6/0x110 fs/namei.c:4587
>  do_syscall_64+0xf5/0x220 arch/x86/entry/common.c:78
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
>  </TASK>
> 
> WARNING: CPU: 0 PID: 646 at fs/inode.c:336 drop_nlink+0xad/0xd0 fs/inode.c:336
> CPU: 0 UID: 0 PID: 646 Comm: syz.0.17 Not tainted 6.12.77+ #1
> Call Trace:
>  <TASK>
>  inode_dec_link_count include/linux/fs.h:2518 [inline]
>  ext2_rename+0x35e/0x850 fs/ext2/namei.c:374
>  vfs_rename+0xf2f/0x2060 fs/namei.c:5021
>  do_renameat2+0xbe2/0xd50 fs/namei.c:5178
>  __x64_sys_rename+0x7e/0xa0 fs/namei.c:5223
>  do_syscall_64+0xf5/0x220 arch/x86/entry/common.c:78
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
>  </TASK>
> 
> WARNING: CPU: 0 PID: 634 at fs/inode.c:336 drop_nlink+0xad/0xd0 fs/inode.c:336
> CPU: 0 UID: 0 PID: 634 Comm: syz-executor Not tainted 6.12.77+ #1
> Call Trace:
>  <TASK>
>  inode_dec_link_count include/linux/fs.h:2518 [inline]
>  ext2_rmdir+0xca/0x110 fs/ext2/namei.c:311
>  vfs_rmdir+0x204/0x690 fs/namei.c:4348
>  do_rmdir+0x372/0x3e0 fs/namei.c:4407
>  __x64_sys_unlinkat+0xf0/0x130 fs/namei.c:4577
>  do_syscall_64+0xf5/0x220 arch/x86/entry/common.c:78
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
>  </TASK>
> 
> Extend the existing i_nlink == 0 check to also catch this case,
> reporting the corruption via ext2_error() and returning -EFSCORRUPTED.
> This rejects the inode at load time and prevents it from reaching any
> of the namei.c paths.
> 
> Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Vasiliy Kovalev <kovalev@altlinux.org>

Thanks. I've added the patch to my tree.

								Honza

> ---
>  fs/ext2/inode.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> index dbfe9098a124..39d972722f5f 100644
> --- a/fs/ext2/inode.c
> +++ b/fs/ext2/inode.c
> @@ -1430,9 +1430,17 @@ struct inode *ext2_iget (struct super_block *sb, unsigned long ino)
>  	 * the test is that same one that e2fsck uses
>  	 * NeilBrown 1999oct15
>  	 */
> -	if (inode->i_nlink == 0 && (inode->i_mode == 0 || ei->i_dtime)) {
> -		/* this inode is deleted */
> -		ret = -ESTALE;
> +	if (inode->i_nlink == 0) {
> +		if (inode->i_mode == 0 || ei->i_dtime) {
> +			/* this inode is deleted */
> +			ret = -ESTALE;
> +		} else {
> +			ext2_error(sb, __func__,
> +				   "inode %lu has zero i_nlink with mode 0%o and no dtime, "
> +				   "filesystem may be corrupt",
> +				   ino, inode->i_mode);
> +			ret = -EFSCORRUPTED;
> +		}
>  		goto bad_inode;
>  	}
>  	inode->i_blocks = le32_to_cpu(raw_inode->i_blocks);
> -- 
> 2.50.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply

* Re: [PATCH v3] ext2: use get_random_u32() where appropriate
From: Jan Kara @ 2026-04-07 11:18 UTC (permalink / raw)
  To: David Carlier; +Cc: Jan Kara, linux-ext4, linux-kernel
In-Reply-To: <20260405154717.4705-1-devnexen@gmail.com>

On Sun 05-04-26 16:47:17, David Carlier wrote:
> Use the typed random integer helpers instead of
> get_random_bytes() when filling a single integer variable.
> The helpers return the value directly, require no pointer
> or size argument, and better express intent.
> 
> Signed-off-by: David Carlier <devnexen@gmail.com>

Thanks! I've added the patch to my tree.

								Honza

> ---
>  fs/ext2/super.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ext2/super.c b/fs/ext2/super.c
> index 603f2641fe10..e4136490c883 100644
> --- a/fs/ext2/super.c
> +++ b/fs/ext2/super.c
> @@ -1151,7 +1151,7 @@ static int ext2_fill_super(struct super_block *sb, struct fs_context *fc)
>  		goto failed_mount2;
>  	}
>  	sbi->s_gdb_count = db_count;
> -	get_random_bytes(&sbi->s_next_generation, sizeof(u32));
> +	sbi->s_next_generation = get_random_u32();
>  	spin_lock_init(&sbi->s_next_gen_lock);
>  
>  	/* per filesystem reservation list head & lock */
> -- 
> 2.53.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply

* Re: [PATCH 0/3] show orphan file inode detail info
From: Jan Kara @ 2026-04-07 10:29 UTC (permalink / raw)
  To: Ye Bin; +Cc: tytso, adilger.kernel, linux-ext4, jack, linux-fsdevel
In-Reply-To: <20260403082507.1882703-1-yebin@huaweicloud.com>

Hi!

On Fri 03-04-26 16:25:04, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
> 
> In actual production environments, the issue of inconsistency between
> df and du is frequently encountered. In many cases, the cause of the
> problem can be identified through the use of lsof. However, when
> overlayfs is combined with project quota configuration, the issue becomes
> more complex and troublesome to diagnose. First, to determine the project
> ID, one needs to obtain orphaned nodes using `fsck.ext4 -fn /dev/xx`, and
> then retrieve file information through `debugfs`. However, the file names
> cannot always be obtained, and it is often unclear which files they are.
> To identify which files these are, one would need to use crash for online
> debugging or use kprobe to gather information incrementally. However, some
> customers in production environments do not agree to upload any tools, and
> online debugging might impact the business. There are also scenarios where
> files are opened in kernel mode, which do not generate file descriptors(fds),
> making it impossible to identify which files were deleted but still have
> references through lsof. This patchset adds a procfs interface to query
> information about orphaned nodes, which can assist in the analysis and
> localization of such issues.

I agree listing orphan inodes for a superblock is useful and the usefulness
could actually go beyond ext4. I imagine the very same problem is there for
XFS or btrfs so perhaps we could think for a while whether we can provide
an interface that wouldn't be ext4 specific? Perhaps an ioctl
(GET_ORPHAN_FILES) that would return an fd and reading from that fd would
return entries for orphan inodes?

Also regarding information reported about orphan inodes - won't it be better
interface to just return a list of file handles? Userspace can then do
whatever it needs with them - open, statx, calling ioctl, etc - so we
thwart feature creep with people asking us to add more information to the
interface. This also offloads a lot of security questions about the
interface to appropriate syscalls. So overall it looks like a win to me.

								Honza

> 
> Ye Bin (3):
>   ext4: register 'orphan_list' procfs
>   ext4: show inode orphan list detail information
>   ext4: show orphan file inode detail info
> 
>  fs/ext4/ext4.h   |   1 +
>  fs/ext4/orphan.c | 227 +++++++++++++++++++++++++++++++++++++++++++++++
>  fs/ext4/sysfs.c  |   2 +
>  3 files changed, 230 insertions(+)
> 
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply

* Re: [BUG] lseek in sparse files broken on ext3 mounted as ext4
From: Alexander Monakov @ 2026-04-07  7:46 UTC (permalink / raw)
  To: linux-ext4; +Cc: Zhang Yi
In-Reply-To: <594c17d9-c00f-e485-96fb-cedf27ce3aa3@ispras.ru>

On Sat, 28 Mar 2026, Alexander Monakov wrote:

> Hi!
> 
> Mounting ext3 with '-o delalloc' is explicitly rejected by the kernel
> ("EXT4-fs: Mount option(s) incompatible with ext3" in dmesg).
> 
> At the same time, mounting ext3 with '-t ext4' is accepted, and enables
> delayed allocation. In this case, lseek with SEEK_DATA/SEEK_HOLE requests
> does not work correctly and breaks userspace programs such as install(1)
> from coreutils.
> 
> To reproduce, it is sufficient to prepare an ext3 image as usual and mount it
> as ext4:
> 
> truncate -s 1G img-ext3
> mkfs.ext3 img-ext3
> mkdir mnt-ext3
> mount -t ext4 img-ext3 mnt-ext3
> 
> and run the following repro script:
> 
> #!/bin/sh
> echo | dd of=src bs=1 count=1 seek=64K
> strace -v -o install.strace install src dst
> cmp src dst
> 
> the output should be
> 
> src dst differ: char 65537, line 1
> 
> with
> 
> lseek(3, 0, SEEK_DATA)                  = -1 ENXIO
> 
> in install.strace on the first lseek call, meaning that it reports
> "no more data until EOF" (and hence install does not copy anything).
> 
> Either mounting with '-o nodelalloc' or fdatasync'ing the file before install
> (conv=fdatasync in dd or 'sync -d src' after dd) avoids the problem.
> 
> The old ext4 wiki [1], the KernelNewbies wiki [2], and the Arch wiki [3]
> all claim that mounting ext3 as ext4 is expected to work correctly.
> 
> [1] https://archive.kernel.org/oldwiki/ext4.wiki.kernel.org/index.php/UpgradeToExt4.html
> [2] https://kernelnewbies.org/Ext4
> [3] https://wiki.archlinux.org/title/Ext4

I would like to ping this, and take this chance to cc Zhang Yi: it seems you
have experience in this area, do you have some ideas what is happening here?
How does delayed allocation interact with lseek like that?

I think it would be nice to solve this problem, because it is just silent
data corruption (no sign in dmesg of anything going wrong) in a setup that
was claimed to be safe by ext4 wiki (and propagated elsewhere).

Downstream this was discovered in Gentoo bug 970253 (I'm not the affected user,
I assisted with root-causing the bug): https://bugs.gentoo.org/970253

Thanks.
Alexander

^ permalink raw reply

* Re: [RFC PATCH v1 0/6] provenance_time (ptime): a new settable timestamp for cross-filesystem provenance
From: Sean Smith @ 2026-04-07  6:06 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: tytso, linux-fsdevel, linux-ext4, linux-btrfs, dsterba, david,
	brauner, osandov, hirofumi, linkinjeon
In-Reply-To: <20260407014129.GC6192@frogsfrogsfrogs>


[written with AI assistance]

On 4/6/2026 20:42, Darrick J. Wong wrote:

> "Standard"... I was about to write a sardonic reply here, but then I
> remembred that Linux finally *does* have a standard means to transfer
> some of those newer file attributes: file_getattr/file_setattr.
>
> (Go Andrey!)
>
> So, I guess all you really need to do is extend struct file_attr and now
> userspace has a fairly convenient means to propagate the provenance
> time. 🙂

Thank you for pointing to file_getattr/file_setattr — this
is a significantly better API path than our utimensat
extension. The size-versioned struct file_attr eliminates
the glibc times[2] limitation entirely, which was one of
the main upstream concerns with the current approach.

We will investigate extending struct file_attr with ptime
fields for v2. The on-disk storage across all 5 filesystems
and the rename-over preservation are API-independent and
would remain unchanged. The change is re-plumbing the
userspace write path from utimensat to file_setattr.

Two design questions:

Would you recommend fa_ptime_sec (__u64) + fa_ptime_nsec
(__u32) matching the statx timespec pattern, or a different
representation?

Pali Rohar has announced plans for mask fields in file_attr.
Should we coordinate with his mask work so ptime can be
selectively set without read-modify-write?

> So does the provenance time cover just the file's contents, or the other
> attributes and xattrs?

Content only. ptime records when the file's data first came
into existence. Metadata changes (permissions, owner,
xattrs) update ctime but leave ptime unchanged. This
matches the semantics of Windows Date Created and macOS
creation time.

> The reason I ask is, does the ptime get copied over for an FICLONE,
> which maps all of one file's data blocks into another?

It should, conceptually — the content's provenance doesn't
change when you clone it. Currently FICLONE shares data
extents but does not copy inode metadata (timestamps,
permissions), so ptime would not be automatically
preserved. The calling tool (e.g., cp --reflink) handles
timestamp copying separately via the write path.

The question is whether FICLONE should be enhanced to copy
ptime from source to destination at the kernel level —
similar to how rename-over preserves ptime. There is an
argument for it: if the kernel handles provenance during
clone, tools don't need to know. But FICLONE doesn't
currently copy mtime either, so adding ptime alone would
be inconsistent. Worth discussing.

Btrfs subvolume snapshots are a different case — they do
preserve ptime because the inodes are COW copies of the
originals.

> And by extension, would it also need to be exchanged if you told
> XFS_IOC_EXCHANGE_RANGE to exchange all contents between two files?

Yes — if the content moves, the provenance should move
with it. If files A and B exchange data extents, their
ptimes should swap. ptime follows the content, not the
inode identity.

> (I know, I know, you said XFS was TBDHBD ;))

Worth considering for a future XFS implementation — and
the file_attr route you suggested would give XFS a clean
integration path for ptime alongside FICLONE and
EXCHANGE_RANGE.

> Last question: Is the provenance time only useful if the file is
> immutable?  Either directly via chattr +i, or by enabling fsverity?

No — ptime is useful regardless of mutability. It records
when the document was born, the same way Windows Date
Created works. Editing a document updates mtime but not
the creation date. Both are independently valuable:

  ptime: "This file was first created March 15, 2019"
  mtime: "It was last modified today"
  btime: "This inode was created when I copied it here"

Immutable files (chattr +i, fsverity) are a special case
where ptime has extra forensic strength — the content
provably hasn't changed since the provenance date. But for
the primary use case — preserving creation dates across
cross-platform migrations — mutability doesn't diminish
ptime's value. A document's creation date remains meaningful
regardless of subsequent edits.

Sean



^ permalink raw reply

* Re: [PATCH v2 3/3] ext4: derive f_fsid from block device to avoid collisions
From: Christoph Hellwig @ 2026-04-07  5:22 UTC (permalink / raw)
  To: Anand Jain
  Cc: Theodore Tso, Christoph Hellwig, Darrick J. Wong, linux-ext4,
	linux-btrfs, linux-xfs, Anand Jain
In-Reply-To: <5bda3d00-df35-4ea1-b313-2fef6e5c5682@gmail.com>

On Sat, Apr 04, 2026 at 04:59:08PM +0800, Anand Jain wrote:
> Context:
> Currently, ext4's f_fsid is consistent across reboots but fails to be
> unique when dealing with cloned filesystems (sharing the same UUID). Per
> statfs(2) [1], the primary requirement is that the (f_fsid, ino) pair
> uniquely identifies a file. The man page makes no explicit guarantee
> regarding consistency across mount cycles or reboots.
> 
> Proposal:
> With this fix, f_fsid becomes f(uuid, dev_t). This ensures OS-wide
> uniqueness and maintains consistency as long as the underlying dev_t
> remains stable.
> 
> Dilemma:
> While statfs(2) [1] suggests f_fsid is "some random stuff," we know
> userspace (NFS, systemd) often treats it as a persistent handle.
> 
> Do you prefer one of the names above, or is there a more idiomatic ext4
> naming convention I should follow?
> 
> Given the ambiguity in the man page, is gating this behind an -o option
> necessary, or should we consider making uniqueness the default behavior?
> 

My take is that anything that should persist should be an on-disk
feature flag, not a mount option.  But I'm not in charge for ext4.


^ permalink raw reply

* Re: [RFC PATCH v1 0/6] provenance_time (ptime): a new settable timestamp for cross-filesystem provenance
From: Darrick J. Wong @ 2026-04-07  1:42 UTC (permalink / raw)
  To: Sean Smith
  Cc: tytso, linux-fsdevel, linux-ext4, linux-btrfs, dsterba, david,
	brauner, osandov, hirofumi, linkinjeon
In-Reply-To: <20260407000558.417-1-DefendTheDisabled@gmail.com>

[drop almaz because the kernel.org mailer immediately refused]

On Mon, Apr 06, 2026 at 07:05:55PM -0500, Sean Smith wrote:
> [written with AI assistance]
> 
> On Sun, Apr 05, 2026 at 06:54:42PM -0400, Theodore Tso wrote:
> 
> Thanks for the substantive engagement — it helps clarify where
> the proposal needs to justify itself.
> 
> > On Sun, Apr 05, 2026 at 02:49:56PM -0500, Sean Smith wrote:
> > > 
> > >   1. Application atomic saves destroy xattrs. Programs that save
> > >      via write-to-temp + rename() replace the inode, permanently
> > >      destroying all extended attributes. Only the VFS sees both
> > >      inodes during rename -- no userspace mechanism can intercept
> > >      this and copy metadata across.
> > 
> > The VFS could potentially copy the xattr on a rename, no?
> 
> It could, but even scoping to user.* means adding conditional
> xattr-copy logic into every filesystem's rename handler — with
> dynamic allocation and xattr tree lookups on a hot path. ptime
> avoids this: one inline inode field, clear semantics, same VFS
> patterns as atime/mtime/btime.
> 
> > >   2. Every tool in the copy chain must explicitly opt in to xattr
> > >      preservation. cp requires --preserve=xattr, rsync requires -X,
> > >      tar requires --xattrs. Each missing flag causes silent data
> > >      loss. Transparent preservation through arbitrary tool flows
> > >      is not achievable in userspace.
> > 
> > But this is true for your proposed ptime as well.  You have to change
> > every single tool to copy over the ptime.  Worse, you have to change
> > the format of tar in a non-standard on-disk format change to support
> > this new ptime timestamp.  And rsync will require a non-standard
> > protocol change to support the new timestamp.
> 
> You are right that copy tools require patches. If ptime only
> improved the copy-tool situation, I would agree it does not
> justify new kernel surface over xattrs.
> 
> The structural difference is in the default adoption path.
> xattr preservation is permanently per-invocation opt-in: each
> tool call needs the correct flag, and the default is to drop
> them. A kernel timestamp exposed through statx/utimensat
> follows the same API pattern as mtime — standard libraries
> and tools naturally evolve to preserve all standard timestamps
> by default. ptime has a path to default-preservation that
> xattrs structurally cannot reach.

"Standard"... I was about to write a sardonic reply here, but then I
remembred that Linux finally *does* have a standard means to transfer
some of those newer file attributes: file_getattr/file_setattr.

(Go Andrey!)

So, I guess all you really need to do is extend struct file_attr and now
userspace has a fairly convenient means to propagate the provenance
time. :)

> On the formats: the tar patch uses a vendor-prefixed PAX
> header (SCHILY.ptime), backward-compatible — old readers
> ignore it cleanly. The rsync patch plugs into the existing
> --crtimes machinery that already supports macOS and Cygwin.
> 
> > > Atomic saves are the default behavior of mainstream applications
> > > (LibreOffice, Vim, Kate, etc.).
> > 
> > You will also have to change mainstream applications to copy ptime
> > from the original file to the file.new before the atomic rename.
> > Using ptime doesn't change this.  So you will need to make this
> > non-standard, Linux-specific change to all of these mainstream
> > applications.
> 
> This is where the cover letter was not clear enough, and it
> is the core reason ptime must be a kernel timestamp.
> 
> The patches implement rename-over preservation in all 5
> filesystem rename handlers. When rename(source, target)
> replaces an existing file, and the source has ptime=0 (the
> default for any newly-created temp file) while the target
> has ptime != 0, the filesystem copies the target's ptime to
> the source before destroying the target's inode. This runs
> inside the rename transaction, atomic with the rename itself.
> 
> Most GUI applications — LibreOffice, Kate, Qt and GNOME
> apps — save via write-to-temp + rename-over-original. For
> these, ptime survives automatically with no application
> changes:
> 
>   1. App writes to temp file              (ptime = 0)
>   2. rename(temp, document.odt)
>   3. Kernel: source ptime=0, target!=0 -> copies ptime
>   4. ptime preserved. No app change.
> 
> This is not universal: editors that use rename-away +
> create-new (Vim with default backupcopy=no, Emacs) do not
> trigger rename-over, and the spec documents this as a known
> limitation. But the write-to-temp + rename-over pattern is
> the dominant GUI save path, and the kernel handles it
> transparently — something no xattr mechanism can provide
> without application cooperation.

So does the provenance time cover just the file's contents, or the other
attributes and xattrs?

The reason I ask is, does the ptime get copied over for an FICLONE,
which maps all of one file's data blocks into another?

And by extension, would it also need to be exchanged if you told
XFS_IOC_EXCHANGE_RANGE to exchange all contents between two files?

(I know, I know, you said XFS was TBDHBD ;))

Last question: Is the provenance time only useful if the file is
immutable?  Either directly via chattr +i, or by enabling fsverity?

--D

> > Is it worth it?  It's a huge amount of cost being spread across a very
> > large part of the open source ecosystem just this fairly narrow use
> > case.  Personally, I'm not convinced it's worth the effort.
> 
> I think the use case is broader than I conveyed. Any workflow
> that copies files from NTFS, APFS, or HFS+ onto native Linux
> filesystems loses user-visible creation time unless carried
> out-of-band. This affects personal migrations, enterprise
> backups, dual-boot users, and professional workflows in
> photography, legal, scientific data, and media production.
> Windows, macOS, and SMB have supported a settable creation
> timestamp for decades — Linux is the outlier.
> 
> Users already expend significant resources working around
> this gap — metadata manifests, scripts to stamp creation
> dates into filenames or xattrs, side-channel databases —
> or simply accept the data loss. The cost is already being
> paid, continuously and redundantly across the ecosystem.
> One upstream investment in ptime converts that distributed
> ongoing cost into a bounded effort.
> 
> ptime is separate from btime by design: it preserves btime's
> value as immutable forensic metadata while providing a
> settable timestamp that travels with file content across
> filesystem boundaries.
> 
> On ecosystem cost: the kernel surface is ~240 lines across
> 28 files. For context, I am a disabled Medicaid recipient
> who came to this from a disability rights litigation
> workflow — I need file provenance preserved across an
> NTFS-to-Btrfs migration for legal work. The complete
> implementation — kernel patches across 5 filesystems,
> tool patches, and xfstests — was produced in a few days using 
> agentic development tools, which suggests the adoption cost may 
> be meaningfully lower than traditional estimates as these 
> tools become available across the ecosystem.
> 
> I understand a new timestamp is permanent API surface and
> the bar should be high. My claim is that rename-over
> preservation — automatic ptime survival through application
> saves, without application changes — makes this materially
> different from an xattr workaround, and justifies that cost.
> 
> Sean
> 

^ permalink raw reply

* Re: [PATCH v2] ext4: fix missing brelse() in ext4_xattr_inode_dec_ref_all()
From: Zhang Yi @ 2026-04-07  1:13 UTC (permalink / raw)
  To: skoyama.kernel, linux-ext4
  Cc: tytso, adilger.kernel, libaokun, jack, ojaswin, ritesh.list,
	bhupesh, Sohei Koyama, Andreas Dilger, stable
In-Reply-To: <20260406074830.8480-1-skoyama@ddn.com>

On 4/6/2026 3:48 PM, skoyama.kernel@gmail.com wrote:
> From: Sohei Koyama <skoyama@ddn.com>
> 
> The commit c8e008b60492 ("ext4: ignore xattrs past end")
> introduced a refcount leak in when block_csum is false.
> 
> ext4_xattr_inode_dec_ref_all() calls ext4_get_inode_loc() to
> get iloc.bh, but never releases it with brelse().
> 
> Fixes: c8e008b60492 ("ext4: ignore xattrs past end")
> Signed-off-by: Sohei Koyama <skoyama@ddn.com>
> Reviewed-by: Andreas Dilger <adilger@dilger.ca>
> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> Cc: stable@vger.kernel.org

Looks good to me.

Reviewed-by: Zhang Yi <yi.zhang@huawei.com>

> ---
>  fs/ext4/xattr.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index 7bf9ba19a89d..19c72e38fb82 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -1165,7 +1165,7 @@ ext4_xattr_inode_dec_ref_all(handle_t *handle, struct inode *parent,
>  {
>  	struct inode *ea_inode;
>  	struct ext4_xattr_entry *entry;
> -	struct ext4_iloc iloc;
> +	struct ext4_iloc iloc = { .bh = NULL };
>  	bool dirty = false;
>  	unsigned int ea_ino;
>  	int err;
> @@ -1260,6 +1260,8 @@ ext4_xattr_inode_dec_ref_all(handle_t *handle, struct inode *parent,
>  			ext4_warning_inode(parent,
>  					   "handle dirty metadata err=%d", err);
>  	}
> +
> +	brelse(iloc.bh);
>  }
>  
>  /*


^ permalink raw reply

* Re: [RFC PATCH v1 0/6] provenance_time (ptime): a new settable timestamp for cross-filesystem provenance
From: Sean Smith @ 2026-04-07  0:05 UTC (permalink / raw)
  To: tytso
  Cc: defendthedisabled, linux-fsdevel, linux-ext4, linux-btrfs,
	dsterba, david, brauner, osandov, almaz, hirofumi, linkinjeon
In-Reply-To: <20260405225442.GA1763@macsyma-wired.lan>

[written with AI assistance]

On Sun, Apr 05, 2026 at 06:54:42PM -0400, Theodore Tso wrote:

Thanks for the substantive engagement — it helps clarify where
the proposal needs to justify itself.

> On Sun, Apr 05, 2026 at 02:49:56PM -0500, Sean Smith wrote:
> > 
> >   1. Application atomic saves destroy xattrs. Programs that save
> >      via write-to-temp + rename() replace the inode, permanently
> >      destroying all extended attributes. Only the VFS sees both
> >      inodes during rename -- no userspace mechanism can intercept
> >      this and copy metadata across.
> 
> The VFS could potentially copy the xattr on a rename, no?

It could, but even scoping to user.* means adding conditional
xattr-copy logic into every filesystem's rename handler — with
dynamic allocation and xattr tree lookups on a hot path. ptime
avoids this: one inline inode field, clear semantics, same VFS
patterns as atime/mtime/btime.

> >   2. Every tool in the copy chain must explicitly opt in to xattr
> >      preservation. cp requires --preserve=xattr, rsync requires -X,
> >      tar requires --xattrs. Each missing flag causes silent data
> >      loss. Transparent preservation through arbitrary tool flows
> >      is not achievable in userspace.
> 
> But this is true for your proposed ptime as well.  You have to change
> every single tool to copy over the ptime.  Worse, you have to change
> the format of tar in a non-standard on-disk format change to support
> this new ptime timestamp.  And rsync will require a non-standard
> protocol change to support the new timestamp.

You are right that copy tools require patches. If ptime only
improved the copy-tool situation, I would agree it does not
justify new kernel surface over xattrs.

The structural difference is in the default adoption path.
xattr preservation is permanently per-invocation opt-in: each
tool call needs the correct flag, and the default is to drop
them. A kernel timestamp exposed through statx/utimensat
follows the same API pattern as mtime — standard libraries
and tools naturally evolve to preserve all standard timestamps
by default. ptime has a path to default-preservation that
xattrs structurally cannot reach.

On the formats: the tar patch uses a vendor-prefixed PAX
header (SCHILY.ptime), backward-compatible — old readers
ignore it cleanly. The rsync patch plugs into the existing
--crtimes machinery that already supports macOS and Cygwin.

> > Atomic saves are the default behavior of mainstream applications
> > (LibreOffice, Vim, Kate, etc.).
> 
> You will also have to change mainstream applications to copy ptime
> from the original file to the file.new before the atomic rename.
> Using ptime doesn't change this.  So you will need to make this
> non-standard, Linux-specific change to all of these mainstream
> applications.

This is where the cover letter was not clear enough, and it
is the core reason ptime must be a kernel timestamp.

The patches implement rename-over preservation in all 5
filesystem rename handlers. When rename(source, target)
replaces an existing file, and the source has ptime=0 (the
default for any newly-created temp file) while the target
has ptime != 0, the filesystem copies the target's ptime to
the source before destroying the target's inode. This runs
inside the rename transaction, atomic with the rename itself.

Most GUI applications — LibreOffice, Kate, Qt and GNOME
apps — save via write-to-temp + rename-over-original. For
these, ptime survives automatically with no application
changes:

  1. App writes to temp file              (ptime = 0)
  2. rename(temp, document.odt)
  3. Kernel: source ptime=0, target!=0 -> copies ptime
  4. ptime preserved. No app change.

This is not universal: editors that use rename-away +
create-new (Vim with default backupcopy=no, Emacs) do not
trigger rename-over, and the spec documents this as a known
limitation. But the write-to-temp + rename-over pattern is
the dominant GUI save path, and the kernel handles it
transparently — something no xattr mechanism can provide
without application cooperation.

> Is it worth it?  It's a huge amount of cost being spread across a very
> large part of the open source ecosystem just this fairly narrow use
> case.  Personally, I'm not convinced it's worth the effort.

I think the use case is broader than I conveyed. Any workflow
that copies files from NTFS, APFS, or HFS+ onto native Linux
filesystems loses user-visible creation time unless carried
out-of-band. This affects personal migrations, enterprise
backups, dual-boot users, and professional workflows in
photography, legal, scientific data, and media production.
Windows, macOS, and SMB have supported a settable creation
timestamp for decades — Linux is the outlier.

Users already expend significant resources working around
this gap — metadata manifests, scripts to stamp creation
dates into filenames or xattrs, side-channel databases —
or simply accept the data loss. The cost is already being
paid, continuously and redundantly across the ecosystem.
One upstream investment in ptime converts that distributed
ongoing cost into a bounded effort.

ptime is separate from btime by design: it preserves btime's
value as immutable forensic metadata while providing a
settable timestamp that travels with file content across
filesystem boundaries.

On ecosystem cost: the kernel surface is ~240 lines across
28 files. For context, I am a disabled Medicaid recipient
who came to this from a disability rights litigation
workflow — I need file provenance preserved across an
NTFS-to-Btrfs migration for legal work. The complete
implementation — kernel patches across 5 filesystems,
tool patches, and xfstests — was produced in a few days using 
agentic development tools, which suggests the adoption cost may 
be meaningfully lower than traditional estimates as these 
tools become available across the ecosystem.

I understand a new timestamp is permanent API surface and
the bar should be high. My claim is that rename-over
preservation — automatic ptime survival through application
saves, without application changes — makes this materially
different from an xattr workaround, and justifies that cost.

Sean

^ permalink raw reply

* Re: [PATCH v2] ext4: fix missing brelse() in ext4_xattr_inode_dec_ref_all()
From: Baokun Li @ 2026-04-06 14:38 UTC (permalink / raw)
  To: skoyama.kernel, linux-ext4
  Cc: tytso, adilger.kernel, jack, ojaswin, ritesh.list, yi.zhang,
	bhupesh, Sohei Koyama, Andreas Dilger, stable
In-Reply-To: <20260406074830.8480-1-skoyama@ddn.com>



On 2026/4/6 15:48, skoyama.kernel@gmail.com wrote:
> From: Sohei Koyama <skoyama@ddn.com>
>
> The commit c8e008b60492 ("ext4: ignore xattrs past end")
> introduced a refcount leak in when block_csum is false.
>
> ext4_xattr_inode_dec_ref_all() calls ext4_get_inode_loc() to
> get iloc.bh, but never releases it with brelse().
>
> Fixes: c8e008b60492 ("ext4: ignore xattrs past end")
> Signed-off-by: Sohei Koyama <skoyama@ddn.com>
> Reviewed-by: Andreas Dilger <adilger@dilger.ca>
> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> Cc: stable@vger.kernel.org

Looks good, feel free to add:

Reviewed-by: Baokun Li <libaokun@linux.alibaba.com>

> ---
>  fs/ext4/xattr.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index 7bf9ba19a89d..19c72e38fb82 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -1165,7 +1165,7 @@ ext4_xattr_inode_dec_ref_all(handle_t *handle, struct inode *parent,
>  {
>  	struct inode *ea_inode;
>  	struct ext4_xattr_entry *entry;
> -	struct ext4_iloc iloc;
> +	struct ext4_iloc iloc = { .bh = NULL };
>  	bool dirty = false;
>  	unsigned int ea_ino;
>  	int err;
> @@ -1260,6 +1260,8 @@ ext4_xattr_inode_dec_ref_all(handle_t *handle, struct inode *parent,
>  			ext4_warning_inode(parent,
>  					   "handle dirty metadata err=%d", err);
>  	}
> +
> +	brelse(iloc.bh);
>  }
>  
>  /*


^ permalink raw reply

* [PATCH v2] ext4: fix missing brelse() in ext4_xattr_inode_dec_ref_all()
From: skoyama.kernel @ 2026-04-06  7:48 UTC (permalink / raw)
  To: linux-ext4
  Cc: tytso, adilger.kernel, libaokun, jack, ojaswin, ritesh.list,
	yi.zhang, bhupesh, Sohei Koyama, Andreas Dilger, stable

From: Sohei Koyama <skoyama@ddn.com>

The commit c8e008b60492 ("ext4: ignore xattrs past end")
introduced a refcount leak in when block_csum is false.

ext4_xattr_inode_dec_ref_all() calls ext4_get_inode_loc() to
get iloc.bh, but never releases it with brelse().

Fixes: c8e008b60492 ("ext4: ignore xattrs past end")
Signed-off-by: Sohei Koyama <skoyama@ddn.com>
Reviewed-by: Andreas Dilger <adilger@dilger.ca>
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
Cc: stable@vger.kernel.org
---
 fs/ext4/xattr.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 7bf9ba19a89d..19c72e38fb82 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1165,7 +1165,7 @@ ext4_xattr_inode_dec_ref_all(handle_t *handle, struct inode *parent,
 {
 	struct inode *ea_inode;
 	struct ext4_xattr_entry *entry;
-	struct ext4_iloc iloc;
+	struct ext4_iloc iloc = { .bh = NULL };
 	bool dirty = false;
 	unsigned int ea_ino;
 	int err;
@@ -1260,6 +1260,8 @@ ext4_xattr_inode_dec_ref_all(handle_t *handle, struct inode *parent,
 			ext4_warning_inode(parent,
 					   "handle dirty metadata err=%d", err);
 	}
+
+	brelse(iloc.bh);
 }
 
 /*
-- 
2.39.3 (Apple Git-146)


^ permalink raw reply related

* Re: [RFC PATCH v1 0/6] provenance_time (ptime): a new settable timestamp for cross-filesystem provenance
From: Theodore Tso @ 2026-04-05 22:54 UTC (permalink / raw)
  To: Sean Smith
  Cc: linux-fsdevel, linux-ext4, linux-btrfs, dsterba, david, brauner,
	osandov, almaz, hirofumi, linkinjeon
In-Reply-To: <20260405195007.1306-1-DefendTheDisabled@gmail.com>

On Sun, Apr 05, 2026 at 02:49:56PM -0500, Sean Smith wrote:
> 
>   1. Application atomic saves destroy xattrs. Programs that save
>      via write-to-temp + rename() replace the inode, permanently
>      destroying all extended attributes. Only the VFS sees both
>      inodes during rename -- no userspace mechanism can intercept
>      this and copy metadata across.

The VFS could potentially copy the xattr on a rename, no?

>   2. Every tool in the copy chain must explicitly opt in to xattr
>      preservation. cp requires --preserve=xattr, rsync requires -X,
>      tar requires --xattrs. Each missing flag causes silent data
>      loss. Transparent preservation through arbitrary tool flows
>      is not achievable in userspace.

But this is true for your proposed ptime as well.  You have to change
every single tool to copy over the ptime.  Worse, you have to change
the format of tar in a non-standard on-disk format change to support
this new ptime timestamp.  And rsync will require a non-standard
protocol change to support the new timestamp.

> Atomic saves are the default behavior of mainstream applications
> (LibreOffice, Vim, Kate, etc.).

You will also have to change mainstream applications to copy ptime
from the original file to the file.new before the atomic rename.
Using ptime doesn't change this.  So you will need to make this
non-standard, Linux-specific change to all of these mainstream
applications.

Is it worth it?  It's a huge amount of cost being spread across a very
large part of the open source ecosystem just this fairly narrow use
case.  Personally, I'm not convinced it's worth the effort.

						- Ted

^ permalink raw reply

* [PATCH 6/6] exfat: map ptime to exFAT creation time with rename-over
From: Sean Smith @ 2026-04-05 19:50 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: linux-ext4, linux-btrfs, tytso, dsterba, david, brauner, osandov,
	almaz, hirofumi, linkinjeon, Sean Smith
In-Reply-To: <20260405195007.1306-1-DefendTheDisabled@gmail.com>

Map ptime to the exFAT creation time field. exFAT creation time
has 10ms precision.

Getattr: report creation time as ptime.
Setattr: write ptime to i_crtime.
Rename-over: save target creation time before __exfat_rename, restore
  after. Preserves creation time across atomic saves.

Signed-off-by: Sean Smith <DefendTheDisabled@gmail.com>
---
 fs/btrfs/inode.c |  3 ++-
 fs/exfat/file.c  |  9 +++++++++
 fs/exfat/namei.c | 21 ++++++++++++++++++---
 3 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index dce80561a..918dfd4c5 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8715,7 +8715,8 @@ static int btrfs_rename(struct mnt_idmap *idmap,
 		/* Note: if rename fails below, ptime mutation is harmless —
 		 * the source file keeps its previous ptime=0 semantics since
 		 * the rename didn't complete. The in-memory value will be
-		 * overwritten on next inode read from disk. */
+		 * overwritten on next inode read from disk.
+		 */
 
 		ret = btrfs_update_inode(trans, BTRFS_I(old_inode));
 		if (unlikely(ret)) {
diff --git a/fs/exfat/file.c b/fs/exfat/file.c
index 536c8078f..b6438bd79 100644
--- a/fs/exfat/file.c
+++ b/fs/exfat/file.c
@@ -277,6 +277,11 @@ int exfat_getattr(struct mnt_idmap *idmap, const struct path *path,
 	stat->result_mask |= STATX_BTIME;
 	stat->btime.tv_sec = ei->i_crtime.tv_sec;
 	stat->btime.tv_nsec = ei->i_crtime.tv_nsec;
+	if (request_mask & STATX_PTIME) {
+		stat->result_mask |= STATX_PTIME;
+		stat->ptime.tv_sec = ei->i_crtime.tv_sec;
+		stat->ptime.tv_nsec = ei->i_crtime.tv_nsec;
+	}
 	stat->blksize = EXFAT_SB(inode->i_sb)->cluster_size;
 	return 0;
 }
@@ -337,6 +342,10 @@ int exfat_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
 	if (attr->ia_valid & ATTR_SIZE)
 		inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));
 
+	if (attr->ia_valid & ATTR_PTIME) {
+		struct exfat_inode_info *exi = EXFAT_I(inode);
+		exi->i_crtime = attr->ia_ptime;
+	}
 	setattr_copy(idmap, inode, attr);
 	exfat_truncate_inode_atime(inode);
 
diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
index dfe957493..9c0b59e00 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -1262,9 +1262,24 @@ static int exfat_rename(struct mnt_idmap *idmap,
 	old_inode = old_dentry->d_inode;
 	new_inode = new_dentry->d_inode;
 
-	err = __exfat_rename(old_dir, EXFAT_I(old_inode), new_dir, new_dentry);
-	if (err)
-		goto unlock;
+	/* ptime rename-over: save target creation time */
+	{
+		struct timespec64 saved_crtime = {};
+		bool inherit_crtime = false;
+
+		if (new_inode && S_ISREG(old_inode->i_mode) &&
+		    S_ISREG(new_inode->i_mode) && old_inode->i_nlink == 1) {
+			saved_crtime = EXFAT_I(new_inode)->i_crtime;
+			inherit_crtime = true;
+		}
+
+		err = __exfat_rename(old_dir, EXFAT_I(old_inode), new_dir, new_dentry);
+		if (err)
+			goto unlock;
+
+		if (inherit_crtime)
+			EXFAT_I(old_inode)->i_crtime = saved_crtime;
+	}
 
 	inode_inc_iversion(new_dir);
 	simple_rename_timestamp(old_dir, old_dentry, new_dir, new_dentry);
-- 
2.53.0


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox