[PATCH v9 0/4] ntfs: fix volume flag races and persist the recorded error state

[PATCH v9 0/4] ntfs: fix volume flag races and persist the recorded error state

[Hongling Zeng]

The fs/ntfs runtime metadata-corruption paths only record the in-memory
NVolErrors() flag, and the caller-side dirty-bit marking races with
ntfs_sync_fs(): a volume can end up with a clean on-disk dirty flag
despite modification or recorded corruption, so chkdsk never runs on
the next mount.  Based on ntfs/ntfs-next (9a05b5715cfa).

 1/4 makes the volume flag read-modify-write atomic under the
    $Volume mrec_lock;
 2/4 marks the volume dirty unconditionally on metadata changes,
    dropping the racy caller-side checks in file.c and namei.c;
 3/4 derives the on-disk dirty bit from the recorded error state at
    the persistence points (sync_fs, remount-ro, put_super) and never
    writes a hibernated volume;
 4/4 persists the dirty state after the final put_super() commits so
    late errors cannot unmount clean.

Changes since v8:
 - 2/4 also converts the setattr and fallocate callers, which were
   missed; the IOCB_NOWAIT non-blocking marking moves to a separate
   follow-up;
 - 3/4 removes the then-unreferenced ntfs_clear_volume_flags().

Hongling Zeng (4):
  ntfs: fix volume flag update races
  ntfs: set the volume dirty bit unconditionally on metadata changes
  ntfs: sync the volume dirty bit with the recorded error state
  ntfs: persist the dirty state after the final put_super() commits

 fs/ntfs/file.c   |  20 ++---
 fs/ntfs/namei.c  |  24 ++----
 fs/ntfs/ntfs.h   |   1 -
 fs/ntfs/super.c  | 191 ++++++++++++++++++++++++++++++++++++-----------
 fs/ntfs/volume.h |   4 +
 5 files changed, 171 insertions(+), 69 deletions(-)

-- 
2.25.1

[PATCH v9 1/4] ntfs: fix volume flag update races

[Hongling Zeng]

ntfs_set_volume_flags() and ntfs_clear_volume_flags() both read
vol->vol_flags outside any lock to compute the new value before handing
it to ntfs_write_volume_flags(), which only takes ni->mrec_lock around
the actual write. The read-modify-write is therefore not atomic, and two
concurrent callers can lose an update: ntfs_sync_fs() may derive a
"clean" value from vol->vol_flags while a writer concurrently records an
error and sets VOLUME_IS_DIRTY; the locked write then silently
overwrites the freshly-set dirty bit. The on-disk volume looks clean
despite the recorded errors, so chkdsk will not run on the next mount
and corrupted metadata can persist.

Fix by moving the read-modify-write inside the mrec_lock: pass the bits
to set and to clear separately, and combine them with the current flag
state under the lock inside ntfs_write_volume_flags(). The set/clear
helpers pass only the bits to modify, not the complete flag state. The
bit manipulation is done on CPU-endian values, and the result is
converted back to little-endian before storing it. The wrappers keep
their signatures so callers are unchanged.

Cc: stable@vger.kernel.org
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 fs/ntfs/super.c | 63 +++++++++++++++++++++++++++++++++++--------------
 1 file changed, 45 insertions(+), 18 deletions(-)

diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
index f4a73e45773d..6ba19986a598 100644
--- a/fs/ntfs/super.c
+++ b/fs/ntfs/super.c
@@ -353,31 +353,45 @@ void ntfs_handle_error(struct super_block *sb)
 }
 
 /*
- * ntfs_write_volume_flags - write new flags to the volume information flags
+ * ntfs_write_volume_flags - apply flag changes to the volume information flags
  * @vol:	ntfs volume on which to modify the flags
- * @flags:	new flags value for the volume information flags
+ * @set_bits:	bits to set in the volume information flags
+ * @clear_bits:	bits to clear in the volume information flags
  *
  * Internal function.  You probably want to use ntfs_{set,clear}_volume_flags()
  * instead (see below).
  *
- * Replace the volume information flags on the volume @vol with the value
- * supplied in @flags.  Note, this overwrites the volume information flags, so
- * make sure to combine the flags you want to modify with the old flags and use
- * the result when calling ntfs_write_volume_flags().
+ * Combine @set_bits and @clear_bits with the current in-memory flag state and
+ * write the result back.  The set/clear helpers pass only the bits to modify,
+ * not the complete flag state.  The read-modify-write happens under
+ * ni->mrec_lock so that concurrent set/clear operations cannot lose updates.
+ * All bit manipulation is done on CPU-endian values, and the result is
+ * converted back to little-endian before storing it.
  *
  * Return 0 on success and -errno on error.
  */
-static int ntfs_write_volume_flags(struct ntfs_volume *vol, const __le16 flags)
+static int ntfs_write_volume_flags(struct ntfs_volume *vol,
+		const __le16 set_bits, const __le16 clear_bits,
+		const bool skip_if_errors)
 {
 	struct ntfs_inode *ni = NTFS_I(vol->vol_ino);
 	struct volume_information *vi;
 	struct ntfs_attr_search_ctx *ctx;
+	u16 flags;
 	int err;
 
-	ntfs_debug("Entering, old flags = 0x%x, new flags = 0x%x.",
-			le16_to_cpu(vol->vol_flags), le16_to_cpu(flags));
 	mutex_lock(&ni->mrec_lock);
-	if (vol->vol_flags == flags)
+
+	if (skip_if_errors && NVolErrors(vol))
+		goto done;
+
+	flags = le16_to_cpu(vol->vol_flags);
+	flags |= le16_to_cpu(set_bits) & le16_to_cpu(VOLUME_FLAGS_MASK);
+	flags &= ~(le16_to_cpu(clear_bits) & le16_to_cpu(VOLUME_FLAGS_MASK));
+	ntfs_debug("Entering, old flags = 0x%x, new flags = 0x%x.",
+			le16_to_cpu(vol->vol_flags), flags);
+
+	if (le16_to_cpu(vol->vol_flags) == flags)
 		goto done;
 
 	ctx = ntfs_attr_get_search_ctx(ni, NULL);
@@ -393,7 +407,7 @@ static int ntfs_write_volume_flags(struct ntfs_volume *vol, const __le16 flags)
 
 	vi = (struct volume_information *)((u8 *)ctx->attr +
 			le16_to_cpu(ctx->attr->data.resident.value_offset));
-	vol->vol_flags = vi->flags = flags;
+	vol->vol_flags = vi->flags = cpu_to_le16(flags);
 	mark_mft_record_dirty(ctx->ntfs_ino);
 	ntfs_attr_put_search_ctx(ctx);
 done:
@@ -414,13 +428,14 @@ static int ntfs_write_volume_flags(struct ntfs_volume *vol, const __le16 flags)
  * @flags:	flags to set on the volume
  *
  * Set the bits in @flags in the volume information flags on the volume @vol.
+ * The bits are combined with the current flag state under the lock in
+ * ntfs_write_volume_flags(), so concurrent updates are not lost.
  *
  * Return 0 on success and -errno on error.
  */
 int ntfs_set_volume_flags(struct ntfs_volume *vol, __le16 flags)
 {
-	flags &= VOLUME_FLAGS_MASK;
-	return ntfs_write_volume_flags(vol, vol->vol_flags | flags);
+	return ntfs_write_volume_flags(vol, flags, 0, false);
 }
 
 /*
@@ -429,14 +444,27 @@ int ntfs_set_volume_flags(struct ntfs_volume *vol, __le16 flags)
  * @flags:	flags to clear on the volume
  *
  * Clear the bits in @flags in the volume information flags on the volume @vol.
+ * The bits are combined with the current flag state under the lock in
+ * ntfs_write_volume_flags(), so concurrent updates are not lost.
  *
  * Return 0 on success and -errno on error.
  */
 int ntfs_clear_volume_flags(struct ntfs_volume *vol, __le16 flags)
 {
-	flags &= VOLUME_FLAGS_MASK;
-	flags = vol->vol_flags & cpu_to_le16(~le16_to_cpu(flags));
-	return ntfs_write_volume_flags(vol, flags);
+	return ntfs_write_volume_flags(vol, 0, flags, false);
+}
+
+/*
+ * ntfs_clear_volume_dirty_if_no_errors - clear dirty bit if no errors exist
+ * @vol:	ntfs volume whose dirty bit should be cleared
+ *
+ * Check NVolErrors() and clear VOLUME_IS_DIRTY under the same mrec_lock so
+ * ntfs_sync_fs() cannot clear the dirty bit after a concurrent error has been
+ * recorded.
+ */
+static int ntfs_clear_volume_dirty_if_no_errors(struct ntfs_volume *vol)
+{
+	return ntfs_write_volume_flags(vol, 0, VOLUME_IS_DIRTY, true);
 }
 
 int ntfs_write_volume_label(struct ntfs_volume *vol, char *label)
@@ -1858,8 +1886,7 @@ static int ntfs_sync_fs(struct super_block *sb, int wait)
 		return 0;
 
 	/* If there are some dirty buffers in the bdev inode */
-	if (!NVolErrors(vol) &&
-	    ntfs_clear_volume_flags(vol, VOLUME_IS_DIRTY)) {
+	if (ntfs_clear_volume_dirty_if_no_errors(vol)) {
 		ntfs_warning(sb, "Failed to clear dirty bit in volume information flags.  Run chkdsk.");
 		err = -EIO;
 	}
-- 
2.25.1

[PATCH v9 2/4] ntfs: set the volume dirty bit unconditionally on metadata changes

[Hongling Zeng]

The callers in file.c and namei.c skip ntfs_set_volume_flags() when
the in-memory vol_flags already show VOLUME_IS_DIRTY, but that check
runs without any lock: if it observes the bit set and ntfs_sync_fs()
clears it under the mrec_lock before the caller's metadata update
completes, the set is skipped and the volume can end up clean on disk
despite the modification, so chkdsk will not run on the next mount.

Drop the caller-side checks and call ntfs_set_volume_flags()
unconditionally: ntfs_write_volume_flags() already skips the write
under the mrec_lock when the combined value is unchanged.  That
unconditional call costs one mrec_lock acquisition per metadata
operation even in the already-dirty steady state; it cannot be
avoided, because deciding to skip the call without the lock is itself
what allows a concurrent ntfs_sync_fs() clear to lose the set.

The IOCB_NOWAIT path in ntfs_file_write_iter() goes through the same
sleeping call: a RWF_NOWAIT write can block in the marking, as it
already could before this change whenever the volume appeared clean.
Giving that path a non-blocking variant is left as follow-up work.
The callers keep the pre-existing behavior of proceeding when the
marking fails, so the dirty bit remains best-effort.

This closes the variant where the set is skipped outright.  A clear
for a concurrent, error-free sync can still land between the set and
the end of the metadata operation; that mark-at-start lifecycle is
pre-existing and is not changed by this patch.

Reported-by: Baolin Liu <liubaolin@kylinos.cn>
Cc: stable@vger.kernel.org
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 fs/ntfs/file.c  | 20 +++++++++++---------
 fs/ntfs/namei.c | 24 ++++++++----------------
 2 files changed, 19 insertions(+), 25 deletions(-)

diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
index 007d1614b9ac..cfc7b36b7dff 100644
--- a/fs/ntfs/file.c
+++ b/fs/ntfs/file.c
@@ -325,8 +325,7 @@ int ntfs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
 		goto out;
 	}
 
-	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
-		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
+	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
 
 	if (ia_valid & ATTR_SIZE) {
 		err = ntfs_setattr_size(vi, attr);
@@ -620,8 +619,13 @@ static ssize_t ntfs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 		goto out_lock;
 	}
 
-	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
-		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
+	/*
+	 * The volume must be marked dirty before the modification is made,
+	 * without an unlocked check of the in-memory flag: ntfs_sync_fs()
+	 * can clear the bit concurrently and the modification would then
+	 * land on a volume that is clean on disk.
+	 */
+	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
 
 	pos = iocb->ki_pos;
 	count = ret;
@@ -1153,11 +1157,9 @@ static long ntfs_fallocate(struct file *file, int mode, loff_t offset, loff_t le
 			return err;
 	}
 
-	if (!(vol->vol_flags & VOLUME_IS_DIRTY)) {
-		err = ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
-		if (err)
-			return err;
-	}
+	err = ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
+	if (err)
+		return err;
 
 	old_size = i_size_read(vi);
 
diff --git a/fs/ntfs/namei.c b/fs/ntfs/namei.c
index fdf52fac4329..3e0adb9a0ea4 100644
--- a/fs/ntfs/namei.c
+++ b/fs/ntfs/namei.c
@@ -757,8 +757,7 @@ static int ntfs_create(struct mnt_idmap *idmap, struct inode *dir,
 		return err;
 	}
 
-	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
-		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
+	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
 
 	ni = __ntfs_create(idmap, dir, uname, uname_len, S_IFREG | mode, 0, NULL, 0);
 	kmem_cache_free(ntfs_name_cache, uname);
@@ -1032,8 +1031,7 @@ static int ntfs_unlink(struct inode *dir, struct dentry *dentry)
 		return err;
 	}
 
-	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
-		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
+	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
 
 	err = ntfs_delete(ni, NTFS_I(dir), uname, uname_len, true);
 	if (err)
@@ -1076,8 +1074,7 @@ static struct dentry *ntfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
 		return ERR_PTR(err);
 	}
 
-	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
-		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
+	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
 
 	ni = __ntfs_create(idmap, dir, uname, uname_len, mode, 0, NULL, 0);
 	kmem_cache_free(ntfs_name_cache, uname);
@@ -1118,8 +1115,7 @@ static int ntfs_rmdir(struct inode *dir, struct dentry *dentry)
 		return err;
 	}
 
-	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
-		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
+	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
 
 	err = ntfs_delete(ni, NTFS_I(dir), uname, uname_len, true);
 	if (err)
@@ -1305,8 +1301,7 @@ static int ntfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
 		new_dir_first = is_subdir(new_dentry->d_parent,
 					  old_dentry->d_parent);
 
-	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
-		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
+	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
 
 	mutex_lock_nested(&old_ni->mrec_lock, NTFS_INODE_MUTEX_NORMAL);
 	if (new_ni)
@@ -1429,8 +1424,7 @@ static int ntfs_symlink(struct mnt_idmap *idmap, struct inode *dir,
 		goto out;
 	}
 
-	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
-		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
+	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
 
 	ni = __ntfs_create(idmap, dir, usrc, usrc_len, S_IFLNK | 0777, 0,
 			   symname, symlen);
@@ -1474,8 +1468,7 @@ static int ntfs_mknod(struct mnt_idmap *idmap, struct inode *dir,
 		return err;
 	}
 
-	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
-		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
+	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
 
 	switch (mode & S_IFMT) {
 	case S_IFCHR:
@@ -1521,8 +1514,7 @@ static int ntfs_link(struct dentry *old_dentry, struct inode *dir,
 		return -ENOMEM;
 	}
 
-	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
-		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
+	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
 
 	ihold(vi);
 	mutex_lock_nested(&ni->mrec_lock, NTFS_INODE_MUTEX_NORMAL);
-- 
2.25.1

Re: [PATCH v9 2/4] ntfs: set the volume dirty bit unconditionally on metadata changes

[liubaolin]

在 2026/9/11 10:09, Hongling Zeng 写道:
> The callers in file.c and namei.c skip ntfs_set_volume_flags() when
> the in-memory vol_flags already show VOLUME_IS_DIRTY, but that check
> runs without any lock: if it observes the bit set and ntfs_sync_fs()
> clears it under the mrec_lock before the caller's metadata update
> completes, the set is skipped and the volume can end up clean on disk
> despite the modification, so chkdsk will not run on the next mount.
> 
> Drop the caller-side checks and call ntfs_set_volume_flags()
> unconditionally: ntfs_write_volume_flags() already skips the write
> under the mrec_lock when the combined value is unchanged.  That
> unconditional call costs one mrec_lock acquisition per metadata
> operation even in the already-dirty steady state; it cannot be
> avoided, because deciding to skip the call without the lock is itself
> what allows a concurrent ntfs_sync_fs() clear to lose the set.
> 
> The IOCB_NOWAIT path in ntfs_file_write_iter() goes through the same
> sleeping call: a RWF_NOWAIT write can block in the marking, as it
> already could before this change whenever the volume appeared clean.
> Giving that path a non-blocking variant is left as follow-up work.
> The callers keep the pre-existing behavior of proceeding when the
> marking fails, so the dirty bit remains best-effort.
> 
> This closes the variant where the set is skipped outright.  A clear
> for a concurrent, error-free sync can still land between the set and
> the end of the metadata operation; that mark-at-start lifecycle is
> pre-existing and is not changed by this patch.
> 
> Reported-by: Baolin Liu <liubaolin@kylinos.cn>
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> ---
>   fs/ntfs/file.c  | 20 +++++++++++---------
>   fs/ntfs/namei.c | 24 ++++++++----------------
>   2 files changed, 19 insertions(+), 25 deletions(-)
> 
> diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
> index 007d1614b9ac..cfc7b36b7dff 100644
> --- a/fs/ntfs/file.c
> +++ b/fs/ntfs/file.c
> @@ -325,8 +325,7 @@ int ntfs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
>   		goto out;
>   	}
>   
> -	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
> -		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
> +	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>   
>   	if (ia_valid & ATTR_SIZE) {
>   		err = ntfs_setattr_size(vi, attr);
> @@ -620,8 +619,13 @@ static ssize_t ntfs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>   		goto out_lock;
>   	}
>   
> -	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
> -		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
> +	/*
> +	 * The volume must be marked dirty before the modification is made,
> +	 * without an unlocked check of the in-memory flag: ntfs_sync_fs()
> +	 * can clear the bit concurrently and the modification would then
> +	 * land on a volume that is clean on disk.
> +	 */
> +	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);

Hi Hongling,
   The added call removes the unlocked check, but it does not close the 
writer/sync lifecycle race. ntfs_set_volume_flags() protects only the 
read-modify-write of the $Volume record while mrec_lock is held, and 
releases the lock before ntfs_file_write_iter() enters the actual write 
path (ntfs_file_buffered_write() or the direct-I/O path) that modifies 
file data and related MFT metadata.

   For example, the following execution is still possible:

   Writer                                      ntfs_sync_fs()
   ------                                      ------------
   ntfs_set_volume_flags()
       acquire $Volume mrec_lock
       set VOLUME_IS_DIRTY
       update $Volume record
       release $Volume mrec_lock

                                                acquire $Volume mrec_lock
                                            observe NVolErrors() == false
                                                clear VOLUME_IS_DIRTY
                                                release $Volume mrec_lock
                                                commit clean volume flags

   continue ntfs_file_write_iter()
   enter ntfs_file_buffered_write()
   or the direct-I/O write path
   modify file data and related MFT metadata
   commit dirty pages/MFT records

   Thus, the metadata modification can still reach disk after 
ntfs_sync_fs() has persisted a clean on-disk dirty bit. The mrec_lock 
serializes only individual $Volume flag updates; it does not cover the 
subsequent write operation. Therefore, the guarantee described in the 
comment above is incomplete, and the same race applies to the other 
unconditional dirty-bit calls added by this patch.

Thanks,
Baolin.

>   
>   	pos = iocb->ki_pos;
>   	count = ret;
> @@ -1153,11 +1157,9 @@ static long ntfs_fallocate(struct file *file, int mode, loff_t offset, loff_t le
>   			return err;
>   	}
>   
> -	if (!(vol->vol_flags & VOLUME_IS_DIRTY)) {
> -		err = ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
> -		if (err)
> -			return err;
> -	}
> +	err = ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
> +	if (err)
> +		return err;
>   
>   	old_size = i_size_read(vi);
>   
> diff --git a/fs/ntfs/namei.c b/fs/ntfs/namei.c
> index fdf52fac4329..3e0adb9a0ea4 100644
> --- a/fs/ntfs/namei.c
> +++ b/fs/ntfs/namei.c
> @@ -757,8 +757,7 @@ static int ntfs_create(struct mnt_idmap *idmap, struct inode *dir,
>   		return err;
>   	}
>   
> -	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
> -		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
> +	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>   
>   	ni = __ntfs_create(idmap, dir, uname, uname_len, S_IFREG | mode, 0, NULL, 0);
>   	kmem_cache_free(ntfs_name_cache, uname);
> @@ -1032,8 +1031,7 @@ static int ntfs_unlink(struct inode *dir, struct dentry *dentry)
>   		return err;
>   	}
>   
> -	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
> -		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
> +	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>   
>   	err = ntfs_delete(ni, NTFS_I(dir), uname, uname_len, true);
>   	if (err)
> @@ -1076,8 +1074,7 @@ static struct dentry *ntfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
>   		return ERR_PTR(err);
>   	}
>   
> -	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
> -		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
> +	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>   
>   	ni = __ntfs_create(idmap, dir, uname, uname_len, mode, 0, NULL, 0);
>   	kmem_cache_free(ntfs_name_cache, uname);
> @@ -1118,8 +1115,7 @@ static int ntfs_rmdir(struct inode *dir, struct dentry *dentry)
>   		return err;
>   	}
>   
> -	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
> -		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
> +	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>   
>   	err = ntfs_delete(ni, NTFS_I(dir), uname, uname_len, true);
>   	if (err)
> @@ -1305,8 +1301,7 @@ static int ntfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
>   		new_dir_first = is_subdir(new_dentry->d_parent,
>   					  old_dentry->d_parent);
>   
> -	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
> -		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
> +	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>   
>   	mutex_lock_nested(&old_ni->mrec_lock, NTFS_INODE_MUTEX_NORMAL);
>   	if (new_ni)
> @@ -1429,8 +1424,7 @@ static int ntfs_symlink(struct mnt_idmap *idmap, struct inode *dir,
>   		goto out;
>   	}
>   
> -	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
> -		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
> +	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>   
>   	ni = __ntfs_create(idmap, dir, usrc, usrc_len, S_IFLNK | 0777, 0,
>   			   symname, symlen);
> @@ -1474,8 +1468,7 @@ static int ntfs_mknod(struct mnt_idmap *idmap, struct inode *dir,
>   		return err;
>   	}
>   
> -	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
> -		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
> +	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>   
>   	switch (mode & S_IFMT) {
>   	case S_IFCHR:
> @@ -1521,8 +1514,7 @@ static int ntfs_link(struct dentry *old_dentry, struct inode *dir,
>   		return -ENOMEM;
>   	}
>   
> -	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
> -		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
> +	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>   
>   	ihold(vi);
>   	mutex_lock_nested(&ni->mrec_lock, NTFS_INODE_MUTEX_NORMAL);

Re:Re: [PATCH v9 2/4] ntfs: set the volume dirty bit unconditionally on metadata changes

[dd]

At 2026-09-13 16:31:30, "liubaolin" <liubaolin12138@163.com> wrote:
>
>
>在 2026/9/11 10:09, Hongling Zeng 写道:
>> The callers in file.c and namei.c skip ntfs_set_volume_flags() when
>> the in-memory vol_flags already show VOLUME_IS_DIRTY, but that check
>> runs without any lock: if it observes the bit set and ntfs_sync_fs()
>> clears it under the mrec_lock before the caller's metadata update
>> completes, the set is skipped and the volume can end up clean on disk
>> despite the modification, so chkdsk will not run on the next mount.
>> 
>> Drop the caller-side checks and call ntfs_set_volume_flags()
>> unconditionally: ntfs_write_volume_flags() already skips the write
>> under the mrec_lock when the combined value is unchanged.  That
>> unconditional call costs one mrec_lock acquisition per metadata
>> operation even in the already-dirty steady state; it cannot be
>> avoided, because deciding to skip the call without the lock is itself
>> what allows a concurrent ntfs_sync_fs() clear to lose the set.
>> 
>> The IOCB_NOWAIT path in ntfs_file_write_iter() goes through the same
>> sleeping call: a RWF_NOWAIT write can block in the marking, as it
>> already could before this change whenever the volume appeared clean.
>> Giving that path a non-blocking variant is left as follow-up work.
>> The callers keep the pre-existing behavior of proceeding when the
>> marking fails, so the dirty bit remains best-effort.
>> 
>> This closes the variant where the set is skipped outright.  A clear
>> for a concurrent, error-free sync can still land between the set and
>> the end of the metadata operation; that mark-at-start lifecycle is
>> pre-existing and is not changed by this patch.
>> 
>> Reported-by: Baolin Liu <liubaolin@kylinos.cn>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
>> ---
>>   fs/ntfs/file.c  | 20 +++++++++++---------
>>   fs/ntfs/namei.c | 24 ++++++++----------------
>>   2 files changed, 19 insertions(+), 25 deletions(-)
>> 
>> diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
>> index 007d1614b9ac..cfc7b36b7dff 100644
>> --- a/fs/ntfs/file.c
>> +++ b/fs/ntfs/file.c
>> @@ -325,8 +325,7 @@ int ntfs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
>>   		goto out;
>>   	}
>>   
>> -	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
>> -		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>> +	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>>   
>>   	if (ia_valid & ATTR_SIZE) {
>>   		err = ntfs_setattr_size(vi, attr);
>> @@ -620,8 +619,13 @@ static ssize_t ntfs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>>   		goto out_lock;
>>   	}
>>   
>> -	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
>> -		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>> +	/*
>> +	 * The volume must be marked dirty before the modification is made,
>> +	 * without an unlocked check of the in-memory flag: ntfs_sync_fs()
>> +	 * can clear the bit concurrently and the modification would then
>> +	 * land on a volume that is clean on disk.
>> +	 */
>> +	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>
>Hi Hongling,
>   The added call removes the unlocked check, but it does not close the 
>writer/sync lifecycle race. ntfs_set_volume_flags() protects only the 
>read-modify-write of the $Volume record while mrec_lock is held, and 
>releases the lock before ntfs_file_write_iter() enters the actual write 
>path (ntfs_file_buffered_write() or the direct-I/O path) that modifies 
>file data and related MFT metadata.
>
>   For example, the following execution is still possible:
>
>   Writer                                      ntfs_sync_fs()
>   ------                                      ------------
>   ntfs_set_volume_flags()
>       acquire $Volume mrec_lock
>       set VOLUME_IS_DIRTY
>       update $Volume record
>       release $Volume mrec_lock
>
>                                                acquire $Volume mrec_lock
>                                            observe NVolErrors() == false
>                                                clear VOLUME_IS_DIRTY
>                                                release $Volume mrec_lock
>                                                commit clean volume flags
>
>   continue ntfs_file_write_iter()
>   enter ntfs_file_buffered_write()
>   or the direct-I/O write path
>   modify file data and related MFT metadata
>   commit dirty pages/MFT records
>
>   Thus, the metadata modification can still reach disk after 
>ntfs_sync_fs() has persisted a clean on-disk dirty bit. The mrec_lock 
>serializes only individual $Volume flag updates; it does not cover the 
>subsequent write operation. Therefore, the guarantee described in the 
>comment above is incomplete, and the same race applies to the other 
>unconditional dirty-bit calls added by this patch.
>
>Thanks,
>Baolin.
>
>>  
Hi Baolin,

  Thanks for the review. You're right: the patch does not serialize the
  dirty-bit update with the whole metadata operation.

  v9 2/4 only closes the skipped-set variant. The unlocked check could skip
  ntfs_set_volume_flags() entirely; the unconditional call always attempts
  the update under the $Volume mrec_lock. The remaining mark-at-start window
  is pre-existing and is already documented in the last paragraph of the
  commit message.

  The ntfs_file_write_iter() comment describes the failure mode of the
  removed unlocked check, not a lifecycle guarantee, but I agree it reads
  like one. I'll post a follow-up patch against ntfs-next that stops
  ntfs_sync_fs() from clearing VOLUME_IS_DIRTY while the volume is mounted
  read-write. The bit is then only cleared at a clean unmount from
  ntfs_put_super(), after inode eviction. This avoids holding the $Volume
  mrec_lock across write operations, at the cost of a possible extra chkdsk
  if the machine crashes between a sync and the unmount.

  Thanks,
  Hongling
 
>>   	pos = iocb->ki_pos;
>>   	count = ret;
>> @@ -1153,11 +1157,9 @@ static long ntfs_fallocate(struct file *file, int mode, loff_t offset, loff_t le
>>   			return err;
>>   	}
>>   
>> -	if (!(vol->vol_flags & VOLUME_IS_DIRTY)) {
>> -		err = ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>> -		if (err)
>> -			return err;
>> -	}
>> +	err = ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>> +	if (err)
>> +		return err;
>>   
>>   	old_size = i_size_read(vi);
>>   
>> diff --git a/fs/ntfs/namei.c b/fs/ntfs/namei.c
>> index fdf52fac4329..3e0adb9a0ea4 100644
>> --- a/fs/ntfs/namei.c
>> +++ b/fs/ntfs/namei.c
>> @@ -757,8 +757,7 @@ static int ntfs_create(struct mnt_idmap *idmap, struct inode *dir,
>>   		return err;
>>   	}
>>   
>> -	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
>> -		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>> +	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>>   
>>   	ni = __ntfs_create(idmap, dir, uname, uname_len, S_IFREG | mode, 0, NULL, 0);
>>   	kmem_cache_free(ntfs_name_cache, uname);
>> @@ -1032,8 +1031,7 @@ static int ntfs_unlink(struct inode *dir, struct dentry *dentry)
>>   		return err;
>>   	}
>>   
>> -	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
>> -		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>> +	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>>   
>>   	err = ntfs_delete(ni, NTFS_I(dir), uname, uname_len, true);
>>   	if (err)
>> @@ -1076,8 +1074,7 @@ static struct dentry *ntfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
>>   		return ERR_PTR(err);
>>   	}
>>   
>> -	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
>> -		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>> +	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>>   
>>   	ni = __ntfs_create(idmap, dir, uname, uname_len, mode, 0, NULL, 0);
>>   	kmem_cache_free(ntfs_name_cache, uname);
>> @@ -1118,8 +1115,7 @@ static int ntfs_rmdir(struct inode *dir, struct dentry *dentry)
>>   		return err;
>>   	}
>>   
>> -	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
>> -		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>> +	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>>   
>>   	err = ntfs_delete(ni, NTFS_I(dir), uname, uname_len, true);
>>   	if (err)
>> @@ -1305,8 +1301,7 @@ static int ntfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
>>   		new_dir_first = is_subdir(new_dentry->d_parent,
>>   					  old_dentry->d_parent);
>>   
>> -	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
>> -		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>> +	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>>   
>>   	mutex_lock_nested(&old_ni->mrec_lock, NTFS_INODE_MUTEX_NORMAL);
>>   	if (new_ni)
>> @@ -1429,8 +1424,7 @@ static int ntfs_symlink(struct mnt_idmap *idmap, struct inode *dir,
>>   		goto out;
>>   	}
>>   
>> -	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
>> -		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>> +	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>>   
>>   	ni = __ntfs_create(idmap, dir, usrc, usrc_len, S_IFLNK | 0777, 0,
>>   			   symname, symlen);
>> @@ -1474,8 +1468,7 @@ static int ntfs_mknod(struct mnt_idmap *idmap, struct inode *dir,
>>   		return err;
>>   	}
>>   
>> -	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
>> -		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>> +	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>>   
>>   	switch (mode & S_IFMT) {
>>   	case S_IFCHR:
>> @@ -1521,8 +1514,7 @@ static int ntfs_link(struct dentry *old_dentry, struct inode *dir,
>>   		return -ENOMEM;
>>   	}
>>   
>> -	if (!(vol->vol_flags & VOLUME_IS_DIRTY))
>> -		ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>> +	ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
>>   
>>   	ihold(vi);
>>   	mutex_lock_nested(&ni->mrec_lock, NTFS_INODE_MUTEX_NORMAL);

[PATCH v9 3/4] ntfs: sync the volume dirty bit with the recorded error state

[Hongling Zeng]

The runtime metadata-corruption paths in fs/ntfs only record the
in-memory NVolErrors() flag; whether VOLUME_IS_DIRTY ever reaches disk
depends on ntfs_set_volume_flags() being called by some other path,
which for most error sites never happens.  A volume can therefore
unmount with a clean on-disk flag despite recorded corruption, and
chkdsk will not run on the next mount.

Persisting the dirty bit from the error paths themselves does not work:
they run under a wide variety of ntfs locks, and the dirty-bit write
takes the $Volume mrec_lock and maps the $Volume mft record, which on
an $MFT page-cache miss takes the $MFT runlist lock for writing.  That
is enough to self-deadlock or form ABBA cycles from several of them:
the $MFT extend undo paths hold the $MFT runlist lock and then take
vol->lcnbmp_lock inside ntfs_cluster_free(); the cluster allocation and
free rollback paths hold vol->lcnbmp_lock; and the whole mft record
allocation tree is reachable from ntfs_write_volume_label()'s
attribute-list maintenance while it holds the $Volume mrec_lock itself.

Instead, make the persistence a property of the sync paths, which run
without ntfs locks held.  The new ntfs_sync_volume_dirty_state() sets
VOLUME_IS_DIRTY when NVolErrors() is recorded and clears it otherwise,
evaluating the error flag under the $Volume mrec_lock.  It is called
from ntfs_sync_fs(), from the remount-to-read-only path of
ntfs_reconfigure(), and from ntfs_put_super(), which previously
evaluated NVolErrors() outside the lock before clearing the dirty bit
unconditionally, and which now also persists the dirty bit for volumes
with recorded errors so they unmount with chkdsk scheduled.  The
ntfs_clear_volume_flags() wrapper, whose last callers this patch
replaces, has no users left and is removed.

The guarantee this provides is eventual, not instantaneous: the error
paths record NVolErrors() with a lock-free set_bit(), so a persistence
point that evaluates the flag just before an error is recorded can
still leave the on-disk bit clean until the next one.  This is sound
because NVolErrors() is sticky for the lifetime of the mount and every
persistence point re-derives the on-disk bit from it; the last one,
ntfs_put_super(), runs after evict_inodes() on a quiesced filesystem,
so a volume that is read-write at unmount time cannot unmount clean.
A volume that is already read-only when the error is recorded
(errors=remount-ro flips the superblock on the first error, as does an
earlier remount-ro) has no persistence point left and keeps whatever
on-disk bit it had; that behaviour is unchanged.  The residual window
is a crash between the error and the next persistence point.

The persistence paths never write a hibernated volume: resuming Windows
from a modified image corrupts it.  Record the mount-time hibernation
verdict in the new NV_Hibernated volume flag and make
ntfs_sync_volume_dirty_state() a no-op while it is set, so the dirty
bit is left exactly as it is on disk and only the in-memory error
state is kept.  Without this, an rw mount of a hibernated volume with
the default errors=continue would gain a filesystem-internal write on
the first sync, remount or unmount.  Other writes to such a mount,
like the mount-time logfile emptying, are pre-existing and unchanged.

Cc: stable@vger.kernel.org
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 fs/ntfs/ntfs.h   |   1 -
 fs/ntfs/super.c  | 129 ++++++++++++++++++++++++++++++++---------------
 fs/ntfs/volume.h |   4 ++
 3 files changed, 93 insertions(+), 41 deletions(-)

diff --git a/fs/ntfs/ntfs.h b/fs/ntfs/ntfs.h
index 45f77848a9cf..a5cd5493c501 100644
--- a/fs/ntfs/ntfs.h
+++ b/fs/ntfs/ntfs.h
@@ -219,7 +219,6 @@ struct option_t {
 };
 extern const struct option_t on_errors_arr[];
 int ntfs_set_volume_flags(struct ntfs_volume *vol, __le16 flags);
-int ntfs_clear_volume_flags(struct ntfs_volume *vol, __le16 flags);
 int ntfs_write_volume_label(struct ntfs_volume *vol, char *label);
 
 /* From fs/ntfs/mst.c */
diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
index 6ba19986a598..733565953302 100644
--- a/fs/ntfs/super.c
+++ b/fs/ntfs/super.c
@@ -262,6 +262,8 @@ static int ntfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
 	return 0;
 }
 
+static int ntfs_sync_volume_dirty_state(struct ntfs_volume *vol);
+
 static int ntfs_reconfigure(struct fs_context *fc)
 {
 	struct super_block *sb = fc->root->d_sb;
@@ -312,10 +314,24 @@ static int ntfs_reconfigure(struct fs_context *fc)
 		}
 	} else if (!sb_rdonly(sb) && (fc->sb_flags & SB_RDONLY)) {
 		/* Remounting read-only. */
-		if (!NVolErrors(vol)) {
-			if (ntfs_clear_volume_flags(vol, VOLUME_IS_DIRTY))
-				ntfs_warning(sb,
-					"Failed to clear dirty bit in volume information flags.  Run chkdsk.");
+		/*
+		 * With errors recorded the dirty bit is set rather than
+		 * cleared, and it is committed right away: the VFS does
+		 * not sync the filesystem during a remount, and once the
+		 * remount succeeds no further persistence point exists -
+		 * ntfs_sync_fs() is only ever invoked for read-write
+		 * superblocks (all its VFS callers skip read-only ones)
+		 * and ntfs_put_super() skips them, so the only remaining
+		 * write would be the evict-time commit at unmount, which
+		 * a crash never reaches.  An error recorded only after
+		 * the remount is still never persisted.
+		 */
+		if (ntfs_sync_volume_dirty_state(vol)) {
+			ntfs_warning(sb,
+				"Failed to update dirty bit in volume information flags.  Run chkdsk.");
+		} else if (NInoDirty(NTFS_I(vol->vol_ino))) {
+			ntfs_commit_inode(vol->vol_ino);
+			blkdev_issue_flush(sb->s_bdev);
 		}
 	}
 
@@ -357,9 +373,10 @@ void ntfs_handle_error(struct super_block *sb)
  * @vol:	ntfs volume on which to modify the flags
  * @set_bits:	bits to set in the volume information flags
  * @clear_bits:	bits to clear in the volume information flags
+ * @dirty_if_errors:	force VOLUME_IS_DIRTY on when NVolErrors() is set
  *
  * Internal function.  You probably want to use ntfs_{set,clear}_volume_flags()
- * instead (see below).
+ * or ntfs_sync_volume_dirty_state() instead (see below).
  *
  * Combine @set_bits and @clear_bits with the current in-memory flag state and
  * write the result back.  The set/clear helpers pass only the bits to modify,
@@ -368,11 +385,18 @@ void ntfs_handle_error(struct super_block *sb)
  * All bit manipulation is done on CPU-endian values, and the result is
  * converted back to little-endian before storing it.
  *
+ * When @dirty_if_errors is true and errors have been recorded on @vol,
+ * VOLUME_IS_DIRTY is forced on after the requested changes.  NVolErrors() is
+ * evaluated under the same mrec_lock, which orders this against other
+ * locked flag updates; the runtime error paths themselves record the flag
+ * lock-free, so see ntfs_sync_volume_dirty_state() for the guarantee this
+ * provides against them.
+ *
  * Return 0 on success and -errno on error.
  */
 static int ntfs_write_volume_flags(struct ntfs_volume *vol,
 		const __le16 set_bits, const __le16 clear_bits,
-		const bool skip_if_errors)
+		const bool dirty_if_errors)
 {
 	struct ntfs_inode *ni = NTFS_I(vol->vol_ino);
 	struct volume_information *vi;
@@ -382,12 +406,11 @@ static int ntfs_write_volume_flags(struct ntfs_volume *vol,
 
 	mutex_lock(&ni->mrec_lock);
 
-	if (skip_if_errors && NVolErrors(vol))
-		goto done;
-
 	flags = le16_to_cpu(vol->vol_flags);
 	flags |= le16_to_cpu(set_bits) & le16_to_cpu(VOLUME_FLAGS_MASK);
 	flags &= ~(le16_to_cpu(clear_bits) & le16_to_cpu(VOLUME_FLAGS_MASK));
+	if (dirty_if_errors && NVolErrors(vol))
+		flags |= le16_to_cpu(VOLUME_IS_DIRTY);
 	ntfs_debug("Entering, old flags = 0x%x, new flags = 0x%x.",
 			le16_to_cpu(vol->vol_flags), flags);
 
@@ -439,31 +462,43 @@ int ntfs_set_volume_flags(struct ntfs_volume *vol, __le16 flags)
 }
 
 /*
- * ntfs_clear_volume_flags - clear bits in the volume information flags
- * @vol:	ntfs volume on which to modify the flags
- * @flags:	flags to clear on the volume
+ * ntfs_sync_volume_dirty_state - persist the dirty bit per the error state
+ * @vol:	ntfs volume whose dirty bit to persist
  *
- * Clear the bits in @flags in the volume information flags on the volume @vol.
- * The bits are combined with the current flag state under the lock in
- * ntfs_write_volume_flags(), so concurrent updates are not lost.
+ * Set VOLUME_IS_DIRTY if errors have been recorded on @vol and clear it
+ * otherwise, under the $Volume mrec_lock.
  *
- * Return 0 on success and -errno on error.
- */
-int ntfs_clear_volume_flags(struct ntfs_volume *vol, __le16 flags)
-{
-	return ntfs_write_volume_flags(vol, 0, flags, false);
-}
-
-/*
- * ntfs_clear_volume_dirty_if_no_errors - clear dirty bit if no errors exist
- * @vol:	ntfs volume whose dirty bit should be cleared
+ * The guarantee this provides is eventual, not instantaneous: the runtime
+ * error paths record NVolErrors() with a lock-free set_bit(), so a
+ * persistence point that evaluates the flag just before an error is
+ * recorded can still leave the on-disk bit clean.  This is sound because
+ * NVolErrors() is sticky (nothing clears it for the lifetime of the mount)
+ * and every persistence point re-derives the on-disk bit from it; the
+ * last one, ntfs_put_super(), runs after evict_inodes() on a quiesced
+ * filesystem, so a volume that is read-write at unmount time cannot
+ * unmount clean.  A volume that is already read-only when the error is
+ * recorded (errors=remount-ro flips the superblock on the first error,
+ * as does an earlier remount-ro) has no persistence point left and
+ * keeps whatever on-disk bit it had; that behaviour is unchanged.  The
+ * residual window is a crash between the error and the next
+ * persistence point.
+ *
+ * This is the single point that persists the in-memory error state to disk.
+ * The runtime error paths only record NVolErrors() because they run under a
+ * variety of ntfs locks the dirty-bit write cannot be taken under (runlist
+ * locks, vol->lcnbmp_lock, vol->mftbmp_lock, mrec_locks); the first
+ * ntfs_sync_fs(), a remount, or the unmount then persists the flag here.
+ *
+ * A hibernated volume is not written from these persistence paths:
+ * resuming Windows from a modified image corrupts it, so the dirty bit
+ * is left as it is on disk and only the in-memory error state is kept.
  *
- * Check NVolErrors() and clear VOLUME_IS_DIRTY under the same mrec_lock so
- * ntfs_sync_fs() cannot clear the dirty bit after a concurrent error has been
- * recorded.
+ * Return 0 on success and -errno on error.
  */
-static int ntfs_clear_volume_dirty_if_no_errors(struct ntfs_volume *vol)
+static int ntfs_sync_volume_dirty_state(struct ntfs_volume *vol)
 {
+	if (NVolHibernated(vol))
+		return 0;
 	return ntfs_write_volume_flags(vol, 0, VOLUME_IS_DIRTY, true);
 }
 
@@ -1615,6 +1650,11 @@ static bool load_system_files(struct ntfs_volume *vol)
 			ntfs_error(sb, "%s.  Mounting read-only%s", es1, es2);
 		}
 		NVolSetErrors(vol);
+		/*
+		 * Remember it for the lifetime of the mount: see
+		 * ntfs_sync_volume_dirty_state().
+		 */
+		NVolSetHibernated(vol);
 	}
 
 	/* If (still) a read-write mount, empty the logfile. */
@@ -1772,22 +1812,31 @@ static void ntfs_put_super(struct super_block *sb)
 	ntfs_commit_inode(vol->mft_ino);
 
 	/*
-	 * If a read-write mount and no volume errors have occurred, mark the
-	 * volume clean.  Also, re-commit all affected inodes.
+	 * If a read-write mount, persist the error state in the volume flags:
+	 * mark the volume clean if no volume errors have occurred, and make
+	 * sure VOLUME_IS_DIRTY is on disk if any have, so chkdsk runs on the
+	 * next mount.  Also, re-commit all affected inodes.
 	 */
 	if (!sb_rdonly(sb)) {
+		if (ntfs_sync_volume_dirty_state(vol)) {
+			ntfs_warning(sb,
+				"Failed to sync dirty bit in volume information flags.  Run chkdsk.");
+		} else if (NVolErrors(vol)) {
+			/*
+			 * The dirty bit is on disk now; only warn when the
+			 * sync actually succeeded, or this message would
+			 * contradict the one above.
+			 */
+			ntfs_warning(sb,
+				"Volume has errors.  Leaving volume marked dirty.  Run chkdsk.");
+		}
+		/* Commits the updated volume flags if they were written. */
+		ntfs_commit_inode(vol->vol_ino);
 		if (!NVolErrors(vol)) {
-			if (ntfs_clear_volume_flags(vol, VOLUME_IS_DIRTY))
-				ntfs_warning(sb,
-					"Failed to clear dirty bit in volume information flags.  Run chkdsk.");
-			ntfs_commit_inode(vol->vol_ino);
 			ntfs_commit_inode(vol->root_ino);
 			if (vol->mftmirr_ino)
 				ntfs_commit_inode(vol->mftmirr_ino);
 			ntfs_commit_inode(vol->mft_ino);
-		} else {
-			ntfs_warning(sb,
-				"Volume has errors.  Leaving volume marked dirty.  Run chkdsk.");
 		}
 	}
 
@@ -1886,8 +1935,8 @@ static int ntfs_sync_fs(struct super_block *sb, int wait)
 		return 0;
 
 	/* If there are some dirty buffers in the bdev inode */
-	if (ntfs_clear_volume_dirty_if_no_errors(vol)) {
-		ntfs_warning(sb, "Failed to clear dirty bit in volume information flags.  Run chkdsk.");
+	if (ntfs_sync_volume_dirty_state(vol)) {
+		ntfs_warning(sb, "Failed to sync dirty bit in volume information flags.  Run chkdsk.");
 		err = -EIO;
 	}
 	sync_inodes_sb(sb);
diff --git a/fs/ntfs/volume.h b/fs/ntfs/volume.h
index bc85a9592245..c7cd27b6dc1a 100644
--- a/fs/ntfs/volume.h
+++ b/fs/ntfs/volume.h
@@ -181,6 +181,8 @@ struct ntfs_volume {
  *				Windows-reserved names (CON, AUX, NUL, COM1,
  *				LPT1, etc.) or invalid characters.
  *
+ * NV_Hibernated		Windows is hibernated on the volume; the sync
+ *				paths must not write the volume flags.
  * NV_Discard			Issue discard/TRIM commands for freed clusters.
  * NV_DisableSparse		Disable creation of sparse regions.
  * NV_NativeSymlinkRel		Translate absolute Windows reparse targets (native_symlink=rel).
@@ -199,6 +201,7 @@ enum {
 	NV_ShowHiddenFiles,
 	NV_HideDotFiles,
 	NV_CheckWindowsNames,
+	NV_Hibernated,
 	NV_Discard,
 	NV_DisableSparse,
 	NV_NativeSymlinkRel,
@@ -237,6 +240,7 @@ DEFINE_NVOL_BIT_OPS(SysImmutable)
 DEFINE_NVOL_BIT_OPS(ShowHiddenFiles)
 DEFINE_NVOL_BIT_OPS(HideDotFiles)
 DEFINE_NVOL_BIT_OPS(CheckWindowsNames)
+DEFINE_NVOL_BIT_OPS(Hibernated)
 DEFINE_NVOL_BIT_OPS(Discard)
 DEFINE_NVOL_BIT_OPS(DisableSparse)
 DEFINE_NVOL_BIT_OPS(NativeSymlinkRel)
-- 
2.25.1

Re: [PATCH v9 3/4] ntfs: sync the volume dirty bit with the recorded error state

[liubaolin]

在 2026/9/11 10:09, Hongling Zeng 写道:
> The runtime metadata-corruption paths in fs/ntfs only record the
> in-memory NVolErrors() flag; whether VOLUME_IS_DIRTY ever reaches disk
> depends on ntfs_set_volume_flags() being called by some other path,
> which for most error sites never happens.  A volume can therefore
> unmount with a clean on-disk flag despite recorded corruption, and
> chkdsk will not run on the next mount.
> 
> Persisting the dirty bit from the error paths themselves does not work:
> they run under a wide variety of ntfs locks, and the dirty-bit write
> takes the $Volume mrec_lock and maps the $Volume mft record, which on
> an $MFT page-cache miss takes the $MFT runlist lock for writing.  That
> is enough to self-deadlock or form ABBA cycles from several of them:
> the $MFT extend undo paths hold the $MFT runlist lock and then take
> vol->lcnbmp_lock inside ntfs_cluster_free(); the cluster allocation and
> free rollback paths hold vol->lcnbmp_lock; and the whole mft record
> allocation tree is reachable from ntfs_write_volume_label()'s
> attribute-list maintenance while it holds the $Volume mrec_lock itself.
> 
> Instead, make the persistence a property of the sync paths, which run
> without ntfs locks held.  The new ntfs_sync_volume_dirty_state() sets
> VOLUME_IS_DIRTY when NVolErrors() is recorded and clears it otherwise,
> evaluating the error flag under the $Volume mrec_lock.  It is called
> from ntfs_sync_fs(), from the remount-to-read-only path of
> ntfs_reconfigure(), and from ntfs_put_super(), which previously
> evaluated NVolErrors() outside the lock before clearing the dirty bit
> unconditionally, and which now also persists the dirty bit for volumes
> with recorded errors so they unmount with chkdsk scheduled.  The
> ntfs_clear_volume_flags() wrapper, whose last callers this patch
> replaces, has no users left and is removed.
> 
> The guarantee this provides is eventual, not instantaneous: the error
> paths record NVolErrors() with a lock-free set_bit(), so a persistence
> point that evaluates the flag just before an error is recorded can
> still leave the on-disk bit clean until the next one.  This is sound
> because NVolErrors() is sticky for the lifetime of the mount and every
> persistence point re-derives the on-disk bit from it; the last one,
> ntfs_put_super(), runs after evict_inodes() on a quiesced filesystem,
> so a volume that is read-write at unmount time cannot unmount clean.
> A volume that is already read-only when the error is recorded
> (errors=remount-ro flips the superblock on the first error, as does an
> earlier remount-ro) has no persistence point left and keeps whatever
> on-disk bit it had; that behaviour is unchanged.  The residual window
> is a crash between the error and the next persistence point.
> 
> The persistence paths never write a hibernated volume: resuming Windows
> from a modified image corrupts it.  Record the mount-time hibernation
> verdict in the new NV_Hibernated volume flag and make
> ntfs_sync_volume_dirty_state() a no-op while it is set, so the dirty
> bit is left exactly as it is on disk and only the in-memory error
> state is kept.  Without this, an rw mount of a hibernated volume with
> the default errors=continue would gain a filesystem-internal write on
> the first sync, remount or unmount.  Other writes to such a mount,
> like the mount-time logfile emptying, are pre-existing and unchanged.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> ---
>   fs/ntfs/ntfs.h   |   1 -
>   fs/ntfs/super.c  | 129 ++++++++++++++++++++++++++++++++---------------
>   fs/ntfs/volume.h |   4 ++
>   3 files changed, 93 insertions(+), 41 deletions(-)
> 
> diff --git a/fs/ntfs/ntfs.h b/fs/ntfs/ntfs.h
> index 45f77848a9cf..a5cd5493c501 100644
> --- a/fs/ntfs/ntfs.h
> +++ b/fs/ntfs/ntfs.h
> @@ -219,7 +219,6 @@ struct option_t {
>   };
>   extern const struct option_t on_errors_arr[];
>   int ntfs_set_volume_flags(struct ntfs_volume *vol, __le16 flags);
> -int ntfs_clear_volume_flags(struct ntfs_volume *vol, __le16 flags);
>   int ntfs_write_volume_label(struct ntfs_volume *vol, char *label);
>   
>   /* From fs/ntfs/mst.c */
> diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
> index 6ba19986a598..733565953302 100644
> --- a/fs/ntfs/super.c
> +++ b/fs/ntfs/super.c
> @@ -262,6 +262,8 @@ static int ntfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
>   	return 0;
>   }
>   
> +static int ntfs_sync_volume_dirty_state(struct ntfs_volume *vol);
> +
>   static int ntfs_reconfigure(struct fs_context *fc)
>   {
>   	struct super_block *sb = fc->root->d_sb;
> @@ -312,10 +314,24 @@ static int ntfs_reconfigure(struct fs_context *fc)
>   		}
>   	} else if (!sb_rdonly(sb) && (fc->sb_flags & SB_RDONLY)) {
>   		/* Remounting read-only. */
> -		if (!NVolErrors(vol)) {
> -			if (ntfs_clear_volume_flags(vol, VOLUME_IS_DIRTY))
> -				ntfs_warning(sb,
> -					"Failed to clear dirty bit in volume information flags.  Run chkdsk.");
> +		/*
> +		 * With errors recorded the dirty bit is set rather than
> +		 * cleared, and it is committed right away: the VFS does
> +		 * not sync the filesystem during a remount, and once the
> +		 * remount succeeds no further persistence point exists -
> +		 * ntfs_sync_fs() is only ever invoked for read-write
> +		 * superblocks (all its VFS callers skip read-only ones)
> +		 * and ntfs_put_super() skips them, so the only remaining
> +		 * write would be the evict-time commit at unmount, which
> +		 * a crash never reaches.  An error recorded only after
> +		 * the remount is still never persisted.
> +		 */
> +		if (ntfs_sync_volume_dirty_state(vol)) {
> +			ntfs_warning(sb,
> +				"Failed to update dirty bit in volume information flags.  Run chkdsk.");
> +		} else if (NInoDirty(NTFS_I(vol->vol_ino))) {
> +			ntfs_commit_inode(vol->vol_ino);
> +			blkdev_issue_flush(sb->s_bdev);

Hi Hongling,
    The comment says that the dirty state must be committed immediately 
during remount-to-read-only because no later ntfs_sync_fs() persistence 
point is available. However, this code does not verify that this 
immediate persistence actually succeeds.

   ntfs_commit_inode() is a void wrapper, so failures while writing the 
$Volume record or updating $MFTMirr can only be observed indirectly 
through NVolErrors(), but that state is not checked after the commit. In 
addition, the return value of blkdev_issue_flush() is ignored.

   As a result, remount can complete successfully even though 
VOLUME_IS_DIRTY was not durably written to stable storage. A crash after 
the remount may then leave the volume clean on disk, despite the 
comment’s stated requirement that the dirty state be committed 
immediately. Please detect and handle errors from the complete $Volume 
commit and block-device flush sequence before treating the dirty-state 
update as successful.

Thanks,
Baolin.

>   		}
>   	}
>   
> @@ -357,9 +373,10 @@ void ntfs_handle_error(struct super_block *sb)
>    * @vol:	ntfs volume on which to modify the flags
>    * @set_bits:	bits to set in the volume information flags
>    * @clear_bits:	bits to clear in the volume information flags
> + * @dirty_if_errors:	force VOLUME_IS_DIRTY on when NVolErrors() is set
>    *
>    * Internal function.  You probably want to use ntfs_{set,clear}_volume_flags()
> - * instead (see below).
> + * or ntfs_sync_volume_dirty_state() instead (see below).
>    *
>    * Combine @set_bits and @clear_bits with the current in-memory flag state and
>    * write the result back.  The set/clear helpers pass only the bits to modify,
> @@ -368,11 +385,18 @@ void ntfs_handle_error(struct super_block *sb)
>    * All bit manipulation is done on CPU-endian values, and the result is
>    * converted back to little-endian before storing it.
>    *
> + * When @dirty_if_errors is true and errors have been recorded on @vol,
> + * VOLUME_IS_DIRTY is forced on after the requested changes.  NVolErrors() is
> + * evaluated under the same mrec_lock, which orders this against other
> + * locked flag updates; the runtime error paths themselves record the flag
> + * lock-free, so see ntfs_sync_volume_dirty_state() for the guarantee this
> + * provides against them.
> + *
>    * Return 0 on success and -errno on error.
>    */
>   static int ntfs_write_volume_flags(struct ntfs_volume *vol,
>   		const __le16 set_bits, const __le16 clear_bits,
> -		const bool skip_if_errors)
> +		const bool dirty_if_errors)
>   {
>   	struct ntfs_inode *ni = NTFS_I(vol->vol_ino);
>   	struct volume_information *vi;
> @@ -382,12 +406,11 @@ static int ntfs_write_volume_flags(struct ntfs_volume *vol,
>   
>   	mutex_lock(&ni->mrec_lock);
>   
> -	if (skip_if_errors && NVolErrors(vol))
> -		goto done;
> -
>   	flags = le16_to_cpu(vol->vol_flags);
>   	flags |= le16_to_cpu(set_bits) & le16_to_cpu(VOLUME_FLAGS_MASK);
>   	flags &= ~(le16_to_cpu(clear_bits) & le16_to_cpu(VOLUME_FLAGS_MASK));
> +	if (dirty_if_errors && NVolErrors(vol))
> +		flags |= le16_to_cpu(VOLUME_IS_DIRTY);
>   	ntfs_debug("Entering, old flags = 0x%x, new flags = 0x%x.",
>   			le16_to_cpu(vol->vol_flags), flags);
>   
> @@ -439,31 +462,43 @@ int ntfs_set_volume_flags(struct ntfs_volume *vol, __le16 flags)
>   }
>   
>   /*
> - * ntfs_clear_volume_flags - clear bits in the volume information flags
> - * @vol:	ntfs volume on which to modify the flags
> - * @flags:	flags to clear on the volume
> + * ntfs_sync_volume_dirty_state - persist the dirty bit per the error state
> + * @vol:	ntfs volume whose dirty bit to persist
>    *
> - * Clear the bits in @flags in the volume information flags on the volume @vol.
> - * The bits are combined with the current flag state under the lock in
> - * ntfs_write_volume_flags(), so concurrent updates are not lost.
> + * Set VOLUME_IS_DIRTY if errors have been recorded on @vol and clear it
> + * otherwise, under the $Volume mrec_lock.
>    *
> - * Return 0 on success and -errno on error.
> - */
> -int ntfs_clear_volume_flags(struct ntfs_volume *vol, __le16 flags)
> -{
> -	return ntfs_write_volume_flags(vol, 0, flags, false);
> -}
> -
> -/*
> - * ntfs_clear_volume_dirty_if_no_errors - clear dirty bit if no errors exist
> - * @vol:	ntfs volume whose dirty bit should be cleared
> + * The guarantee this provides is eventual, not instantaneous: the runtime
> + * error paths record NVolErrors() with a lock-free set_bit(), so a
> + * persistence point that evaluates the flag just before an error is
> + * recorded can still leave the on-disk bit clean.  This is sound because
> + * NVolErrors() is sticky (nothing clears it for the lifetime of the mount)
> + * and every persistence point re-derives the on-disk bit from it; the
> + * last one, ntfs_put_super(), runs after evict_inodes() on a quiesced
> + * filesystem, so a volume that is read-write at unmount time cannot
> + * unmount clean.  A volume that is already read-only when the error is
> + * recorded (errors=remount-ro flips the superblock on the first error,
> + * as does an earlier remount-ro) has no persistence point left and
> + * keeps whatever on-disk bit it had; that behaviour is unchanged.  The
> + * residual window is a crash between the error and the next
> + * persistence point.
> + *
> + * This is the single point that persists the in-memory error state to disk.
> + * The runtime error paths only record NVolErrors() because they run under a
> + * variety of ntfs locks the dirty-bit write cannot be taken under (runlist
> + * locks, vol->lcnbmp_lock, vol->mftbmp_lock, mrec_locks); the first
> + * ntfs_sync_fs(), a remount, or the unmount then persists the flag here.
> + *
> + * A hibernated volume is not written from these persistence paths:
> + * resuming Windows from a modified image corrupts it, so the dirty bit
> + * is left as it is on disk and only the in-memory error state is kept.
>    *
> - * Check NVolErrors() and clear VOLUME_IS_DIRTY under the same mrec_lock so
> - * ntfs_sync_fs() cannot clear the dirty bit after a concurrent error has been
> - * recorded.
> + * Return 0 on success and -errno on error.
>    */
> -static int ntfs_clear_volume_dirty_if_no_errors(struct ntfs_volume *vol)
> +static int ntfs_sync_volume_dirty_state(struct ntfs_volume *vol)
>   {
> +	if (NVolHibernated(vol))
> +		return 0;
>   	return ntfs_write_volume_flags(vol, 0, VOLUME_IS_DIRTY, true);
>   }
>   
> @@ -1615,6 +1650,11 @@ static bool load_system_files(struct ntfs_volume *vol)
>   			ntfs_error(sb, "%s.  Mounting read-only%s", es1, es2);
>   		}
>   		NVolSetErrors(vol);
> +		/*
> +		 * Remember it for the lifetime of the mount: see
> +		 * ntfs_sync_volume_dirty_state().
> +		 */
> +		NVolSetHibernated(vol);
>   	}
>   
>   	/* If (still) a read-write mount, empty the logfile. */
> @@ -1772,22 +1812,31 @@ static void ntfs_put_super(struct super_block *sb)
>   	ntfs_commit_inode(vol->mft_ino);
>   
>   	/*
> -	 * If a read-write mount and no volume errors have occurred, mark the
> -	 * volume clean.  Also, re-commit all affected inodes.
> +	 * If a read-write mount, persist the error state in the volume flags:
> +	 * mark the volume clean if no volume errors have occurred, and make
> +	 * sure VOLUME_IS_DIRTY is on disk if any have, so chkdsk runs on the
> +	 * next mount.  Also, re-commit all affected inodes.
>   	 */
>   	if (!sb_rdonly(sb)) {
> +		if (ntfs_sync_volume_dirty_state(vol)) {
> +			ntfs_warning(sb,
> +				"Failed to sync dirty bit in volume information flags.  Run chkdsk.");
> +		} else if (NVolErrors(vol)) {
> +			/*
> +			 * The dirty bit is on disk now; only warn when the
> +			 * sync actually succeeded, or this message would
> +			 * contradict the one above.
> +			 */
> +			ntfs_warning(sb,
> +				"Volume has errors.  Leaving volume marked dirty.  Run chkdsk.");
> +		}
> +		/* Commits the updated volume flags if they were written. */
> +		ntfs_commit_inode(vol->vol_ino);
>   		if (!NVolErrors(vol)) {
> -			if (ntfs_clear_volume_flags(vol, VOLUME_IS_DIRTY))
> -				ntfs_warning(sb,
> -					"Failed to clear dirty bit in volume information flags.  Run chkdsk.");
> -			ntfs_commit_inode(vol->vol_ino);
>   			ntfs_commit_inode(vol->root_ino);
>   			if (vol->mftmirr_ino)
>   				ntfs_commit_inode(vol->mftmirr_ino);
>   			ntfs_commit_inode(vol->mft_ino);
> -		} else {
> -			ntfs_warning(sb,
> -				"Volume has errors.  Leaving volume marked dirty.  Run chkdsk.");
>   		}
>   	}
>   
> @@ -1886,8 +1935,8 @@ static int ntfs_sync_fs(struct super_block *sb, int wait)
>   		return 0;
>   
>   	/* If there are some dirty buffers in the bdev inode */
> -	if (ntfs_clear_volume_dirty_if_no_errors(vol)) {
> -		ntfs_warning(sb, "Failed to clear dirty bit in volume information flags.  Run chkdsk.");
> +	if (ntfs_sync_volume_dirty_state(vol)) {
> +		ntfs_warning(sb, "Failed to sync dirty bit in volume information flags.  Run chkdsk.");
>   		err = -EIO;
>   	}
>   	sync_inodes_sb(sb);
> diff --git a/fs/ntfs/volume.h b/fs/ntfs/volume.h
> index bc85a9592245..c7cd27b6dc1a 100644
> --- a/fs/ntfs/volume.h
> +++ b/fs/ntfs/volume.h
> @@ -181,6 +181,8 @@ struct ntfs_volume {
>    *				Windows-reserved names (CON, AUX, NUL, COM1,
>    *				LPT1, etc.) or invalid characters.
>    *
> + * NV_Hibernated		Windows is hibernated on the volume; the sync
> + *				paths must not write the volume flags.
>    * NV_Discard			Issue discard/TRIM commands for freed clusters.
>    * NV_DisableSparse		Disable creation of sparse regions.
>    * NV_NativeSymlinkRel		Translate absolute Windows reparse targets (native_symlink=rel).
> @@ -199,6 +201,7 @@ enum {
>   	NV_ShowHiddenFiles,
>   	NV_HideDotFiles,
>   	NV_CheckWindowsNames,
> +	NV_Hibernated,
>   	NV_Discard,
>   	NV_DisableSparse,
>   	NV_NativeSymlinkRel,
> @@ -237,6 +240,7 @@ DEFINE_NVOL_BIT_OPS(SysImmutable)
>   DEFINE_NVOL_BIT_OPS(ShowHiddenFiles)
>   DEFINE_NVOL_BIT_OPS(HideDotFiles)
>   DEFINE_NVOL_BIT_OPS(CheckWindowsNames)
> +DEFINE_NVOL_BIT_OPS(Hibernated)
>   DEFINE_NVOL_BIT_OPS(Discard)
>   DEFINE_NVOL_BIT_OPS(DisableSparse)
>   DEFINE_NVOL_BIT_OPS(NativeSymlinkRel)

Re:Re: [PATCH v9 3/4] ntfs: sync the volume dirty bit with the recorded error state

[dd]

Hi Baolin,

Thanks for the review. I checked the commit path in detail.

Since ntfs_commit_inode() discards the return value from
__ntfs_write_inode(), I will use the latter directly and verify that the
$Volume inode is clean after a successful commit:

        err = __ntfs_write_inode(vol->vol_ino, 1);

The remount-to-read-only path will fail if
ntfs_sync_volume_dirty_state(), the inode commit, or
blkdev_issue_flush() fails. The same checks will be added to
ntfs_put_super(), where failures will be reported as warnings since
put_super() cannot return an error.

NVolErrors() will not be used to detect commit failures because it is
sticky. The errors=remount-ro path is unaffected.

I will send this as a follow-up patch.

Thanks,
Hongling


At 2026-09-13 21:46:45, "liubaolin" <liubaolin12138@163.com> wrote:
>
>
>在 2026/9/11 10:09, Hongling Zeng 写道:
>> The runtime metadata-corruption paths in fs/ntfs only record the
>> in-memory NVolErrors() flag; whether VOLUME_IS_DIRTY ever reaches disk
>> depends on ntfs_set_volume_flags() being called by some other path,
>> which for most error sites never happens.  A volume can therefore
>> unmount with a clean on-disk flag despite recorded corruption, and
>> chkdsk will not run on the next mount.
>> 
>> Persisting the dirty bit from the error paths themselves does not work:
>> they run under a wide variety of ntfs locks, and the dirty-bit write
>> takes the $Volume mrec_lock and maps the $Volume mft record, which on
>> an $MFT page-cache miss takes the $MFT runlist lock for writing.  That
>> is enough to self-deadlock or form ABBA cycles from several of them:
>> the $MFT extend undo paths hold the $MFT runlist lock and then take
>> vol->lcnbmp_lock inside ntfs_cluster_free(); the cluster allocation and
>> free rollback paths hold vol->lcnbmp_lock; and the whole mft record
>> allocation tree is reachable from ntfs_write_volume_label()'s
>> attribute-list maintenance while it holds the $Volume mrec_lock itself.
>> 
>> Instead, make the persistence a property of the sync paths, which run
>> without ntfs locks held.  The new ntfs_sync_volume_dirty_state() sets
>> VOLUME_IS_DIRTY when NVolErrors() is recorded and clears it otherwise,
>> evaluating the error flag under the $Volume mrec_lock.  It is called
>> from ntfs_sync_fs(), from the remount-to-read-only path of
>> ntfs_reconfigure(), and from ntfs_put_super(), which previously
>> evaluated NVolErrors() outside the lock before clearing the dirty bit
>> unconditionally, and which now also persists the dirty bit for volumes
>> with recorded errors so they unmount with chkdsk scheduled.  The
>> ntfs_clear_volume_flags() wrapper, whose last callers this patch
>> replaces, has no users left and is removed.
>> 
>> The guarantee this provides is eventual, not instantaneous: the error
>> paths record NVolErrors() with a lock-free set_bit(), so a persistence
>> point that evaluates the flag just before an error is recorded can
>> still leave the on-disk bit clean until the next one.  This is sound
>> because NVolErrors() is sticky for the lifetime of the mount and every
>> persistence point re-derives the on-disk bit from it; the last one,
>> ntfs_put_super(), runs after evict_inodes() on a quiesced filesystem,
>> so a volume that is read-write at unmount time cannot unmount clean.
>> A volume that is already read-only when the error is recorded
>> (errors=remount-ro flips the superblock on the first error, as does an
>> earlier remount-ro) has no persistence point left and keeps whatever
>> on-disk bit it had; that behaviour is unchanged.  The residual window
>> is a crash between the error and the next persistence point.
>> 
>> The persistence paths never write a hibernated volume: resuming Windows
>> from a modified image corrupts it.  Record the mount-time hibernation
>> verdict in the new NV_Hibernated volume flag and make
>> ntfs_sync_volume_dirty_state() a no-op while it is set, so the dirty
>> bit is left exactly as it is on disk and only the in-memory error
>> state is kept.  Without this, an rw mount of a hibernated volume with
>> the default errors=continue would gain a filesystem-internal write on
>> the first sync, remount or unmount.  Other writes to such a mount,
>> like the mount-time logfile emptying, are pre-existing and unchanged.
>> 
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
>> ---
>>   fs/ntfs/ntfs.h   |   1 -
>>   fs/ntfs/super.c  | 129 ++++++++++++++++++++++++++++++++---------------
>>   fs/ntfs/volume.h |   4 ++
>>   3 files changed, 93 insertions(+), 41 deletions(-)
>> 
>> diff --git a/fs/ntfs/ntfs.h b/fs/ntfs/ntfs.h
>> index 45f77848a9cf..a5cd5493c501 100644
>> --- a/fs/ntfs/ntfs.h
>> +++ b/fs/ntfs/ntfs.h
>> @@ -219,7 +219,6 @@ struct option_t {
>>   };
>>   extern const struct option_t on_errors_arr[];
>>   int ntfs_set_volume_flags(struct ntfs_volume *vol, __le16 flags);
>> -int ntfs_clear_volume_flags(struct ntfs_volume *vol, __le16 flags);
>>   int ntfs_write_volume_label(struct ntfs_volume *vol, char *label);
>>   
>>   /* From fs/ntfs/mst.c */
>> diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
>> index 6ba19986a598..733565953302 100644
>> --- a/fs/ntfs/super.c
>> +++ b/fs/ntfs/super.c
>> @@ -262,6 +262,8 @@ static int ntfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
>>   	return 0;
>>   }
>>   
>> +static int ntfs_sync_volume_dirty_state(struct ntfs_volume *vol);
>> +
>>   static int ntfs_reconfigure(struct fs_context *fc)
>>   {
>>   	struct super_block *sb = fc->root->d_sb;
>> @@ -312,10 +314,24 @@ static int ntfs_reconfigure(struct fs_context *fc)
>>   		}
>>   	} else if (!sb_rdonly(sb) && (fc->sb_flags & SB_RDONLY)) {
>>   		/* Remounting read-only. */
>> -		if (!NVolErrors(vol)) {
>> -			if (ntfs_clear_volume_flags(vol, VOLUME_IS_DIRTY))
>> -				ntfs_warning(sb,
>> -					"Failed to clear dirty bit in volume information flags.  Run chkdsk.");
>> +		/*
>> +		 * With errors recorded the dirty bit is set rather than
>> +		 * cleared, and it is committed right away: the VFS does
>> +		 * not sync the filesystem during a remount, and once the
>> +		 * remount succeeds no further persistence point exists -
>> +		 * ntfs_sync_fs() is only ever invoked for read-write
>> +		 * superblocks (all its VFS callers skip read-only ones)
>> +		 * and ntfs_put_super() skips them, so the only remaining
>> +		 * write would be the evict-time commit at unmount, which
>> +		 * a crash never reaches.  An error recorded only after
>> +		 * the remount is still never persisted.
>> +		 */
>> +		if (ntfs_sync_volume_dirty_state(vol)) {
>> +			ntfs_warning(sb,
>> +				"Failed to update dirty bit in volume information flags.  Run chkdsk.");
>> +		} else if (NInoDirty(NTFS_I(vol->vol_ino))) {
>> +			ntfs_commit_inode(vol->vol_ino);
>> +			blkdev_issue_flush(sb->s_bdev);
>
>Hi Hongling,
>    The comment says that the dirty state must be committed immediately 
>during remount-to-read-only because no later ntfs_sync_fs() persistence 
>point is available. However, this code does not verify that this 
>immediate persistence actually succeeds.
>
>   ntfs_commit_inode() is a void wrapper, so failures while writing the 
>$Volume record or updating $MFTMirr can only be observed indirectly 
>through NVolErrors(), but that state is not checked after the commit. In 
>addition, the return value of blkdev_issue_flush() is ignored.
>
>   As a result, remount can complete successfully even though 
>VOLUME_IS_DIRTY was not durably written to stable storage. A crash after 
>the remount may then leave the volume clean on disk, despite the 
>comment’s stated requirement that the dirty state be committed 
>immediately. Please detect and handle errors from the complete $Volume 
>commit and block-device flush sequence before treating the dirty-state 
>update as successful.
>
>Thanks,
>Baolin.
>
>>   		}
>>   	}
>>   
>> @@ -357,9 +373,10 @@ void ntfs_handle_error(struct super_block *sb)
>>    * @vol:	ntfs volume on which to modify the flags
>>    * @set_bits:	bits to set in the volume information flags
>>    * @clear_bits:	bits to clear in the volume information flags
>> + * @dirty_if_errors:	force VOLUME_IS_DIRTY on when NVolErrors() is set
>>    *
>>    * Internal function.  You probably want to use ntfs_{set,clear}_volume_flags()
>> - * instead (see below).
>> + * or ntfs_sync_volume_dirty_state() instead (see below).
>>    *
>>    * Combine @set_bits and @clear_bits with the current in-memory flag state and
>>    * write the result back.  The set/clear helpers pass only the bits to modify,
>> @@ -368,11 +385,18 @@ void ntfs_handle_error(struct super_block *sb)
>>    * All bit manipulation is done on CPU-endian values, and the result is
>>    * converted back to little-endian before storing it.
>>    *
>> + * When @dirty_if_errors is true and errors have been recorded on @vol,
>> + * VOLUME_IS_DIRTY is forced on after the requested changes.  NVolErrors() is
>> + * evaluated under the same mrec_lock, which orders this against other
>> + * locked flag updates; the runtime error paths themselves record the flag
>> + * lock-free, so see ntfs_sync_volume_dirty_state() for the guarantee this
>> + * provides against them.
>> + *
>>    * Return 0 on success and -errno on error.
>>    */
>>   static int ntfs_write_volume_flags(struct ntfs_volume *vol,
>>   		const __le16 set_bits, const __le16 clear_bits,
>> -		const bool skip_if_errors)
>> +		const bool dirty_if_errors)
>>   {
>>   	struct ntfs_inode *ni = NTFS_I(vol->vol_ino);
>>   	struct volume_information *vi;
>> @@ -382,12 +406,11 @@ static int ntfs_write_volume_flags(struct ntfs_volume *vol,
>>   
>>   	mutex_lock(&ni->mrec_lock);
>>   
>> -	if (skip_if_errors && NVolErrors(vol))
>> -		goto done;
>> -
>>   	flags = le16_to_cpu(vol->vol_flags);
>>   	flags |= le16_to_cpu(set_bits) & le16_to_cpu(VOLUME_FLAGS_MASK);
>>   	flags &= ~(le16_to_cpu(clear_bits) & le16_to_cpu(VOLUME_FLAGS_MASK));
>> +	if (dirty_if_errors && NVolErrors(vol))
>> +		flags |= le16_to_cpu(VOLUME_IS_DIRTY);
>>   	ntfs_debug("Entering, old flags = 0x%x, new flags = 0x%x.",
>>   			le16_to_cpu(vol->vol_flags), flags);
>>   
>> @@ -439,31 +462,43 @@ int ntfs_set_volume_flags(struct ntfs_volume *vol, __le16 flags)
>>   }
>>   
>>   /*
>> - * ntfs_clear_volume_flags - clear bits in the volume information flags
>> - * @vol:	ntfs volume on which to modify the flags
>> - * @flags:	flags to clear on the volume
>> + * ntfs_sync_volume_dirty_state - persist the dirty bit per the error state
>> + * @vol:	ntfs volume whose dirty bit to persist
>>    *
>> - * Clear the bits in @flags in the volume information flags on the volume @vol.
>> - * The bits are combined with the current flag state under the lock in
>> - * ntfs_write_volume_flags(), so concurrent updates are not lost.
>> + * Set VOLUME_IS_DIRTY if errors have been recorded on @vol and clear it
>> + * otherwise, under the $Volume mrec_lock.
>>    *
>> - * Return 0 on success and -errno on error.
>> - */
>> -int ntfs_clear_volume_flags(struct ntfs_volume *vol, __le16 flags)
>> -{
>> -	return ntfs_write_volume_flags(vol, 0, flags, false);
>> -}
>> -
>> -/*
>> - * ntfs_clear_volume_dirty_if_no_errors - clear dirty bit if no errors exist
>> - * @vol:	ntfs volume whose dirty bit should be cleared
>> + * The guarantee this provides is eventual, not instantaneous: the runtime
>> + * error paths record NVolErrors() with a lock-free set_bit(), so a
>> + * persistence point that evaluates the flag just before an error is
>> + * recorded can still leave the on-disk bit clean.  This is sound because
>> + * NVolErrors() is sticky (nothing clears it for the lifetime of the mount)
>> + * and every persistence point re-derives the on-disk bit from it; the
>> + * last one, ntfs_put_super(), runs after evict_inodes() on a quiesced
>> + * filesystem, so a volume that is read-write at unmount time cannot
>> + * unmount clean.  A volume that is already read-only when the error is
>> + * recorded (errors=remount-ro flips the superblock on the first error,
>> + * as does an earlier remount-ro) has no persistence point left and
>> + * keeps whatever on-disk bit it had; that behaviour is unchanged.  The
>> + * residual window is a crash between the error and the next
>> + * persistence point.
>> + *
>> + * This is the single point that persists the in-memory error state to disk.
>> + * The runtime error paths only record NVolErrors() because they run under a
>> + * variety of ntfs locks the dirty-bit write cannot be taken under (runlist
>> + * locks, vol->lcnbmp_lock, vol->mftbmp_lock, mrec_locks); the first
>> + * ntfs_sync_fs(), a remount, or the unmount then persists the flag here.
>> + *
>> + * A hibernated volume is not written from these persistence paths:
>> + * resuming Windows from a modified image corrupts it, so the dirty bit
>> + * is left as it is on disk and only the in-memory error state is kept.
>>    *
>> - * Check NVolErrors() and clear VOLUME_IS_DIRTY under the same mrec_lock so
>> - * ntfs_sync_fs() cannot clear the dirty bit after a concurrent error has been
>> - * recorded.
>> + * Return 0 on success and -errno on error.
>>    */
>> -static int ntfs_clear_volume_dirty_if_no_errors(struct ntfs_volume *vol)
>> +static int ntfs_sync_volume_dirty_state(struct ntfs_volume *vol)
>>   {
>> +	if (NVolHibernated(vol))
>> +		return 0;
>>   	return ntfs_write_volume_flags(vol, 0, VOLUME_IS_DIRTY, true);
>>   }
>>   
>> @@ -1615,6 +1650,11 @@ static bool load_system_files(struct ntfs_volume *vol)
>>   			ntfs_error(sb, "%s.  Mounting read-only%s", es1, es2);
>>   		}
>>   		NVolSetErrors(vol);
>> +		/*
>> +		 * Remember it for the lifetime of the mount: see
>> +		 * ntfs_sync_volume_dirty_state().
>> +		 */
>> +		NVolSetHibernated(vol);
>>   	}
>>   
>>   	/* If (still) a read-write mount, empty the logfile. */
>> @@ -1772,22 +1812,31 @@ static void ntfs_put_super(struct super_block *sb)
>>   	ntfs_commit_inode(vol->mft_ino);
>>   
>>   	/*
>> -	 * If a read-write mount and no volume errors have occurred, mark the
>> -	 * volume clean.  Also, re-commit all affected inodes.
>> +	 * If a read-write mount, persist the error state in the volume flags:
>> +	 * mark the volume clean if no volume errors have occurred, and make
>> +	 * sure VOLUME_IS_DIRTY is on disk if any have, so chkdsk runs on the
>> +	 * next mount.  Also, re-commit all affected inodes.
>>   	 */
>>   	if (!sb_rdonly(sb)) {
>> +		if (ntfs_sync_volume_dirty_state(vol)) {
>> +			ntfs_warning(sb,
>> +				"Failed to sync dirty bit in volume information flags.  Run chkdsk.");
>> +		} else if (NVolErrors(vol)) {
>> +			/*
>> +			 * The dirty bit is on disk now; only warn when the
>> +			 * sync actually succeeded, or this message would
>> +			 * contradict the one above.
>> +			 */
>> +			ntfs_warning(sb,
>> +				"Volume has errors.  Leaving volume marked dirty.  Run chkdsk.");
>> +		}
>> +		/* Commits the updated volume flags if they were written. */
>> +		ntfs_commit_inode(vol->vol_ino);
>>   		if (!NVolErrors(vol)) {
>> -			if (ntfs_clear_volume_flags(vol, VOLUME_IS_DIRTY))
>> -				ntfs_warning(sb,
>> -					"Failed to clear dirty bit in volume information flags.  Run chkdsk.");
>> -			ntfs_commit_inode(vol->vol_ino);
>>   			ntfs_commit_inode(vol->root_ino);
>>   			if (vol->mftmirr_ino)
>>   				ntfs_commit_inode(vol->mftmirr_ino);
>>   			ntfs_commit_inode(vol->mft_ino);
>> -		} else {
>> -			ntfs_warning(sb,
>> -				"Volume has errors.  Leaving volume marked dirty.  Run chkdsk.");
>>   		}
>>   	}
>>   
>> @@ -1886,8 +1935,8 @@ static int ntfs_sync_fs(struct super_block *sb, int wait)
>>   		return 0;
>>   
>>   	/* If there are some dirty buffers in the bdev inode */
>> -	if (ntfs_clear_volume_dirty_if_no_errors(vol)) {
>> -		ntfs_warning(sb, "Failed to clear dirty bit in volume information flags.  Run chkdsk.");
>> +	if (ntfs_sync_volume_dirty_state(vol)) {
>> +		ntfs_warning(sb, "Failed to sync dirty bit in volume information flags.  Run chkdsk.");
>>   		err = -EIO;
>>   	}
>>   	sync_inodes_sb(sb);
>> diff --git a/fs/ntfs/volume.h b/fs/ntfs/volume.h
>> index bc85a9592245..c7cd27b6dc1a 100644
>> --- a/fs/ntfs/volume.h
>> +++ b/fs/ntfs/volume.h
>> @@ -181,6 +181,8 @@ struct ntfs_volume {
>>    *				Windows-reserved names (CON, AUX, NUL, COM1,
>>    *				LPT1, etc.) or invalid characters.
>>    *
>> + * NV_Hibernated		Windows is hibernated on the volume; the sync
>> + *				paths must not write the volume flags.
>>    * NV_Discard			Issue discard/TRIM commands for freed clusters.
>>    * NV_DisableSparse		Disable creation of sparse regions.
>>    * NV_NativeSymlinkRel		Translate absolute Windows reparse targets (native_symlink=rel).
>> @@ -199,6 +201,7 @@ enum {
>>   	NV_ShowHiddenFiles,
>>   	NV_HideDotFiles,
>>   	NV_CheckWindowsNames,
>> +	NV_Hibernated,
>>   	NV_Discard,
>>   	NV_DisableSparse,
>>   	NV_NativeSymlinkRel,
>> @@ -237,6 +240,7 @@ DEFINE_NVOL_BIT_OPS(SysImmutable)
>>   DEFINE_NVOL_BIT_OPS(ShowHiddenFiles)
>>   DEFINE_NVOL_BIT_OPS(HideDotFiles)
>>   DEFINE_NVOL_BIT_OPS(CheckWindowsNames)
>> +DEFINE_NVOL_BIT_OPS(Hibernated)
>>   DEFINE_NVOL_BIT_OPS(Discard)
>>   DEFINE_NVOL_BIT_OPS(DisableSparse)
>>   DEFINE_NVOL_BIT_OPS(NativeSymlinkRel)

[PATCH v9 4/4] ntfs: persist the dirty state after the final put_super() commits

[Hongling Zeng]

The just-in-case mftmirr/mft commits and the final write_inode_now()
in ntfs_put_super() can record NVolErrors() after the dirty state has
been persisted, so errors from those points would leave the volume
unmounted with a clean on-disk dirty bit - contradicting the "cannot
unmount clean" guarantee ntfs_sync_volume_dirty_state() is meant to
provide.

Move the persistence to the end of ntfs_put_super(): keep the gated
re-commits and the tail commits where they are, run
ntfs_sync_volume_dirty_state() and the $Volume commit after the last
write_inode_now(), and release vol->vol_ino only after the sync.

The release order of the special inodes matters for the $Volume
commit: writing the $Volume record mirrors it through
ntfs_sync_mft_mirror() (record number 3 is below vol->mftmirr_size),
which fails with -EIO and leaves the mirror stale once
vol->mftmirr_ino is gone, so the mirror inode is released only after
that commit.  vol->vol_ino is then put before vol->mft_ino is dropped:
if the commit failed before it could clear the dirty flag,
ntfs_evict_big_inode() commits the inode again on its way out, and
__ntfs_write_inode() resolves the runlist through vol->mft_ino.

Reported-by: Baolin Liu <liubaolin@kylinos.cn>
Cc: stable@vger.kernel.org
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 fs/ntfs/super.c | 75 ++++++++++++++++++++++++++++++++++---------------
 1 file changed, 52 insertions(+), 23 deletions(-)

diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
index 733565953302..3574c224fe28 100644
--- a/fs/ntfs/super.c
+++ b/fs/ntfs/super.c
@@ -1812,26 +1812,13 @@ static void ntfs_put_super(struct super_block *sb)
 	ntfs_commit_inode(vol->mft_ino);
 
 	/*
-	 * If a read-write mount, persist the error state in the volume flags:
-	 * mark the volume clean if no volume errors have occurred, and make
-	 * sure VOLUME_IS_DIRTY is on disk if any have, so chkdsk runs on the
-	 * next mount.  Also, re-commit all affected inodes.
+	 * If a read-write mount, re-commit all affected inodes once more.
+	 * The dirty state itself is persisted at the end of ntfs_put_super(),
+	 * after the last commits and the final write_inode_now(): those can
+	 * still record errors via __ntfs_write_inode(), and the sync must
+	 * evaluate NVolErrors() with the last setter already run.
 	 */
 	if (!sb_rdonly(sb)) {
-		if (ntfs_sync_volume_dirty_state(vol)) {
-			ntfs_warning(sb,
-				"Failed to sync dirty bit in volume information flags.  Run chkdsk.");
-		} else if (NVolErrors(vol)) {
-			/*
-			 * The dirty bit is on disk now; only warn when the
-			 * sync actually succeeded, or this message would
-			 * contradict the one above.
-			 */
-			ntfs_warning(sb,
-				"Volume has errors.  Leaving volume marked dirty.  Run chkdsk.");
-		}
-		/* Commits the updated volume flags if they were written. */
-		ntfs_commit_inode(vol->vol_ino);
 		if (!NVolErrors(vol)) {
 			ntfs_commit_inode(vol->root_ino);
 			if (vol->mftmirr_ino)
@@ -1840,9 +1827,6 @@ static void ntfs_put_super(struct super_block *sb)
 		}
 	}
 
-	iput(vol->vol_ino);
-	vol->vol_ino = NULL;
-
 	/* NTFS 3.0+ specific clean up. */
 	if (vol->major_ver >= 3) {
 		if (vol->extend_ino) {
@@ -1872,8 +1856,6 @@ static void ntfs_put_super(struct super_block *sb)
 		/* Re-commit the mft mirror and mft just in case. */
 		ntfs_commit_inode(vol->mftmirr_ino);
 		ntfs_commit_inode(vol->mft_ino);
-		iput(vol->mftmirr_ino);
-		vol->mftmirr_ino = NULL;
 	}
 	/*
 	 * We should have no dirty inodes left, due to
@@ -1883,6 +1865,53 @@ static void ntfs_put_super(struct super_block *sb)
 	ntfs_commit_inode(vol->mft_ino);
 	write_inode_now(vol->mft_ino, 1);
 
+	/*
+	 * If a read-write mount, persist the error state in the volume flags:
+	 * mark the volume clean if no volume errors have occurred, and make
+	 * sure VOLUME_IS_DIRTY is on disk if any have, so chkdsk runs on the
+	 * next mount.
+	 */
+	if (!sb_rdonly(sb)) {
+		if (ntfs_sync_volume_dirty_state(vol)) {
+			ntfs_warning(sb,
+				"Failed to sync dirty bit in volume information flags.  Run chkdsk.");
+		} else if (NVolErrors(vol)) {
+			/*
+			 * The dirty bit is on disk now; only warn when the
+			 * sync actually succeeded, or this message would
+			 * contradict the one above.
+			 */
+			ntfs_warning(sb,
+				"Volume has errors.  Leaving volume marked dirty.  Run chkdsk.");
+		}
+		/*
+		 * Commits the updated volume flags if they were written.
+		 * The mft mirror must still be around for this: the
+		 * $Volume record (mft record number 3, below
+		 * vol->mftmirr_size) is mirrored by write_mft_record()
+		 * through ntfs_sync_mft_mirror(), which fails with -EIO
+		 * and leaves the mirror stale once vol->mftmirr_ino is
+		 * gone, so the mirror inode is only released after this
+		 * commit.
+		 */
+		ntfs_commit_inode(vol->vol_ino);
+	}
+
+	/*
+	 * Release $Volume while the mft inode is still available: if the
+	 * commit above failed before it could clear the dirty flag,
+	 * ntfs_evict_big_inode() commits the inode again on its way out,
+	 * and __ntfs_write_inode() needs vol->mft_ino to look up the
+	 * runlist of the record to write.
+	 */
+	iput(vol->vol_ino);
+	vol->vol_ino = NULL;
+
+	if (vol->mftmirr_ino) {
+		iput(vol->mftmirr_ino);
+		vol->mftmirr_ino = NULL;
+	}
+
 	iput(vol->mft_ino);
 	vol->mft_ino = NULL;
 	blkdev_issue_flush(sb->s_bdev);
-- 
2.25.1