mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Hongling Zeng <zhongling0719@126.com>
To: Hyunchul Lee <hyc.lee@gmail.com>,
	 Hongling Zeng <zenghongling@kylinos.cn>
Cc: linkinjeon@kernel.org, ntfs@lists.linux.dev,
	 linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] ntfs: handle a failed size rollback in the write error path
Date: Tue, 20 Oct 2026 11:22:58 +0800	[thread overview]
Message-ID: <6AD6DE92.9030305@126.com> (raw)
In-Reply-To: <atbX6ZAHk8U4Xsuo@hyunchul-PC02>


在 2026年10月20日 10:54, Hyunchul Lee 写道:
> Hi Hongling,
>
> On Mon, Oct 19, 2026 at 02:49:18PM +0800, Hongling Zeng wrote:
>> >ntfs_file_write_iter() extends the data and initialized sizes before
>> >it copies the user data and restores them when the write then fails.
>> >The restoration is best-effort: both results are discarded, even
>> >though the extension is already recorded in the dirty MFT record.
>> >When the restoration fails too, the extended sizes reach the disk
>> >while the VFS inode keeps reporting the old ones: after a remount the
>> >file appears grown again, with an uninitialized tail where the failed
>> >write never put any data, and nothing records the failure.
>> >
>> >Keep both undo results: on failure, set NVolSetErrors(), which the next
>> >persistence point persists as VOLUME_IS_DIRTY, and log a message for
>> >the errors= policy.
>> >
>> >Fixes: 9c87959601e8 ("ntfs: update file operations")
>> >Suggested-by: Hyunchul Lee<hyc.lee@gmail.com>
>> >Signed-off-by: Hongling Zeng<zenghongling@kylinos.cn>
>> >---
>> >Changes in v2:
>> >- do not roll the initialized size back on a resident data attribute,
>> >   where ntfs_attr_set_initialized_size() always fails with -EINVAL and
>> >   the data size rollback restores both coupled sizes.
>> >---
>> >  fs/ntfs/file.c | 46 +++++++++++++++++++++++++++++++++++++---------
>> >  1 file changed, 37 insertions(+), 9 deletions(-)
>> >
>> >diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
>> >index d810ca5904bd..597fd70ed30b 100644
>> >--- a/fs/ntfs/file.c
>> >+++ b/fs/ntfs/file.c
>> >@@ -934,22 +934,50 @@ ssize_t ntfs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>> >  				&ntfs_iomap_folio_ops, NULL);
>> >  out:
>> >  	if (ret < 0 && ret != -EIOCBQUEUED) {
>> >-		if (ni->initialized_size != old_init_size) {
>> >-			struct ntfs_inode *mrec_ni =
>> >-				ntfs_base_inode(ni);
>> >+		int undo_err = 0;
>> >+		int init_err = 0;
>> >+
>> >+		/* ntfs_attr_set_initialized_size() rejects a resident one. */
>> >+		if (ni->initialized_size != old_init_size &&
>> >+		    NInoNonResident(ni)) {
>> >+			struct ntfs_inode *mrec_ni = ntfs_base_inode(ni);
>> >  
>> >  			mutex_lock(&mrec_ni->mrec_lock);
>> >-			ntfs_attr_set_initialized_size(ni, old_init_size);
>> >+			init_err = ntfs_attr_set_initialized_size(ni,
>> >+					old_init_size);
>> >  			mutex_unlock(&mrec_ni->mrec_lock);
>> >+			if (init_err && !undo_err)
>> >+				undo_err = init_err;
>> >  		}
>> >  		if (ni->data_size != old_data_size) {
>> >-			struct ntfs_inode *mrec_ni =
>> >-				ntfs_base_inode(ni);
>> >+			struct ntfs_inode *mrec_ni = ntfs_base_inode(ni);
>> >+			int data_err = 0;
>> >  
>> >  			truncate_setsize(vi, old_data_size);
>> >-			mutex_lock(&mrec_ni->mrec_lock);
>> >-			ntfs_attr_truncate(ni, old_data_size);
>> >-			mutex_unlock(&mrec_ni->mrec_lock);
>> >+			/*
>> >+			 * ntfs_attr_truncate() rejects a compressed one.
>> >+			 * Skip it also after a failed initialized size
>> >+			 * rollback: the shrunk data size would stay below
>> >+			 * the initialized size, which the loader rejects.
>> >+			 */
>> >+			if (!NInoCompressed(ni) && !init_err) {
>> >+				mutex_lock(&mrec_ni->mrec_lock);
>> >+				data_err = ntfs_attr_truncate(ni,
>> >+						old_data_size);
>> >+				mutex_unlock(&mrec_ni->mrec_lock);
>> >+			}
>> >+			if (data_err && !undo_err)
>> >+				undo_err = data_err;
>> >+		}
>> >+		if (ni->data_size != old_data_size && NInoCompressed(ni))
>> >+			ntfs_warning(vi->i_sb,
> For compressed-file, the code only emits a warning, which causes the
> VOLUME_IS_DIRTY persistence to be bypassed even though the MFT data size
> remains expanded.
>
> I think that we should report the compressed rollback as an error.
>
> -- Thanks, Hyunchul
   Hi Hyunchul,

   You are right, the warning-only path left the expanded data size with
   no error state.  v3 drops the skip: the data size rollback now goes
   through __ntfs_attr_truncate_vfs(), the same entry ntfs_setattr_size()
   uses, which also supports compressed attributes.  The compressed
   rollback is performed rather than skipped, and a failure of either
   undo sets NVolSetErrors() and calls ntfs_error().

   Thanks,
   Hongling



      reply	other threads:[~2026-10-20  3:24 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-19  6:49 Hongling Zeng
2026-10-20  2:54 ` Hyunchul Lee
2026-10-20  3:22   ` Hongling Zeng [this message]

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=6AD6DE92.9030305@126.com \
    --to=zhongling0719@126.com \
    --cc=hyc.lee@gmail.com \
    --cc=linkinjeon@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ntfs@lists.linux.dev \
    --cc=zenghongling@kylinos.cn \
    /path/to/YOUR_REPLY

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

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

all inboxes | Powered by JetHome®