All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Wilcox <willy@infradead.org>
To: linux-xfs@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
Subject: Why doesn't XFS need ->launder_folio?
Date: Fri, 8 Sep 2023 15:48:03 +0100	[thread overview]
Message-ID: <ZPs0I9ZTxfAQtyI9@casper.infradead.org> (raw)

I want to remove ->launder_folio.  So I'm looking at commit e3db7691e9f3
which introduced ->launder_page.  The race described there is pretty
clear:

     invalidate_inode_pages2() may find the dirty bit has been set on a page
     owing to the fact that the page may still be mapped after it was locked.
     Only after the call to unmap_mapping_range() are we sure that the page
     can no longer be dirtied.

ie this happens:

Task A				Task B
mmaps a file, writes to page A
				open(O_DIRECT)
				read()
				kiocb_invalidate_pages()
				filemap_write_and_wait_range()
				__filemap_fdatawrite_range()
				filemap_fdatawrite_wbc()
				do_writepages()
				iomap_writepages()
				write_cache_pages()
				page A gets cleaned
writes to page A again
				invalidate_inode_pages2_range()
				folio_mapped() is true, so we unmap it
				folio_launder() returns 0
				invalidate_complete_folio2() returns 0
				ret = -EBUSY
				kiocb_invalidate_pages() returns EBUSY

and the DIO read fails, despite it being totally reasonable to return
the now-stale data on storage.  A DIO write would be a different matter;
we really do need to get page A out of cache.

So would it be reasonable to unmap the pages earlier and rely on
invalidate_lock to prevent page faults making the page writable
between the call to filemap_write_and_wait_range() and the call to
invalidate_complete_folio2() ?  Then we could get rid of ->launder_folio()
as well as making DIO a little more reliable when racing with page faults.

             reply	other threads:[~2023-09-08 14:48 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-08 14:48 Matthew Wilcox [this message]
2023-09-11  0:02 ` Why doesn't XFS need ->launder_folio? Dave Chinner

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=ZPs0I9ZTxfAQtyI9@casper.infradead.org \
    --to=willy@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

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

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