public inbox for linux-ext4@vger.kernel.org
 help / color / mirror / Atom feed
From: "Theodore Tso" <tytso@mit.edu>
To: Matthew Wilcox <willy@infradead.org>
Cc: Deepanshu Kartikey <kartikey406@gmail.com>,
	Zhang Yi <yi.zhang@huaweicloud.com>,
	linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org,
	syzbot+b0a0670332b6b3230a0a@syzkaller.appspotmail.com,
	adilger.kernel@dilger.ca, djwong@kernel.org
Subject: Re: [PATCH v2] ext4: check folio uptodate state in ext4_page_mkwrite()
Date: Fri, 5 Dec 2025 08:37:39 -0500	[thread overview]
Message-ID: <20251205133739.GA19558@macsyma.lan> (raw)
In-Reply-To: <aTJSglQznqeph5lM@casper.infradead.org>

On Fri, Dec 05, 2025 at 03:33:22AM +0000, Matthew Wilcox wrote:
> It sounds like I was confused -- I thought the folios being
> invalidated in mpage_release_unused_pages() belonged to the block
> device, but from what you're saying, they belong to a user-visible
> file?

Yes, correct.  I'm guessing that we were marking the page !uptodate
back when that was the only way to indicate that there had been any
kind of I/O error (either on the read or write side).  Obviously we
have much better ways of doing it in the 21st century.  :-)

> Now, is the folio necessarily dirty at this point?  I guess so if
> we're in the writeback path.  Darrick got rid of similar code in
> iomap a few years ago; see commit e9c3a8e820ed.  So it'd probably be
> good to have ext4 behave the same way.

Hmm, yes.   Agreed.

    commit e9c3a8e820ed0eeb2be05072f29f80d1b79f053b
    Author: Darrick J. Wong <djwong@kernel.org>
    Date:   Mon May 16 15:27:38 2022 -0700

    iomap: don't invalidate folios after writeback errors
    
    XFS has the unique behavior (as compared to the other Linux
    filesystems) that on writeback errors it will completely
    invalidate the affected folio and force the page cache to reread
    the contents from disk.  All other filesystems leave the page
    mapped and up to date.
    
    This is a rude awakening for user programs, since (in the case
    where write fails but reread doesn't) file contents will appear to
    revert old disk contents with no notification other than an EIO on
    fsync.  This might have been annoying back in the days when iomap
    dealt with one page at a time, but with multipage folios, we can
    now throw away *megabytes* worth of data for a single write error...

As Darrick pointed out we could potentially append a *single* byte to
a file, and if there was some kind of writeback error, we could
potentially throw away *vast* amounts of data for no good reason.

     	  	     	       	     - Ted

  reply	other threads:[~2025-12-05 13:39 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-22  1:57 [PATCH v2] ext4: check folio uptodate state in ext4_page_mkwrite() Deepanshu Kartikey
2025-11-30  2:06 ` Deepanshu Kartikey
2025-12-02 12:24   ` Zhang Yi
2025-12-03  1:37     ` Deepanshu Kartikey
2025-12-03  6:52       ` Zhang Yi
2025-12-03  7:43         ` Deepanshu Kartikey
2025-12-03 15:46           ` Theodore Tso
2025-12-03 17:04             ` Deepanshu Kartikey
2025-12-03 18:40             ` Theodore Tso
2025-12-03 21:35             ` Matthew Wilcox
2025-12-03 22:33               ` Theodore Tso
2025-12-04  9:54                 ` Deepanshu Kartikey
2025-12-05  2:18                   ` Theodore Tso
2025-12-05  3:31                     ` Deepanshu Kartikey
2025-12-05  3:33                     ` Matthew Wilcox
2025-12-05 13:37                       ` Theodore Tso [this message]
2025-12-05 14:28                         ` Deepanshu Kartikey
2026-01-05  2:08                           ` Deepanshu Kartikey
2025-12-03 21:54             ` Matthew Wilcox

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=20251205133739.GA19558@macsyma.lan \
    --to=tytso@mit.edu \
    --cc=adilger.kernel@dilger.ca \
    --cc=djwong@kernel.org \
    --cc=kartikey406@gmail.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=syzbot+b0a0670332b6b3230a0a@syzkaller.appspotmail.com \
    --cc=willy@infradead.org \
    --cc=yi.zhang@huaweicloud.com \
    /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