All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Matthew Wilcox <willy@infradead.org>
Cc: linux-mm@kvack.org, linux-fsdevel@vger.kernel.org,
	Kent Overstreet <kent.overstreet@gmail.com>,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH] mm/filemap: Use filemap_read_page in filemap_fault
Date: Tue, 2 Mar 2021 22:07:35 -0800	[thread overview]
Message-ID: <20210302220735.1f150f28323f676d2955ab49@linux-foundation.org> (raw)
In-Reply-To: <20210303013313.GZ2723601@casper.infradead.org>

On Wed, 3 Mar 2021 01:33:13 +0000 Matthew Wilcox <willy@infradead.org> wrote:

> On Tue, Mar 02, 2021 at 05:30:39PM -0800, Andrew Morton wrote:
> > On Fri, 26 Feb 2021 14:00:11 +0000 "Matthew Wilcox (Oracle)" <willy@infradead.org> wrote:
> > 
> > > After splitting generic_file_buffered_read() into smaller parts, it
> > > turns out we can reuse one of the parts in filemap_fault().  This fixes
> > > an oversight -- waiting for the I/O to complete is now interruptible
> > > by a fatal signal.  And it saves us a few bytes of text in an unlikely
> > > path.
> > 
> > We also handle AOP_TRUNCATED_PAGE which the present code fails to do. 
> > Should this be in the changelog?
> 
> No, the present code does handle AOP_TRUNCATED_PAGE.  It's perhaps not
> the clearest in the diff, but it's there.  Here's git show -U5:
> 
> -       ClearPageError(page);
>         fpin = maybe_unlock_mmap_for_io(vmf, fpin);
> -       error = mapping->a_ops->readpage(file, page);
> -       if (!error) {
> -               wait_on_page_locked(page);
> -               if (!PageUptodate(page))
> -                       error = -EIO;
> -       }
> +       error = filemap_read_page(file, mapping, page);
>         if (fpin)
>                 goto out_retry;
>         put_page(page);
>  
>         if (!error || error == AOP_TRUNCATED_PAGE)
>                 goto retry_find;
>  
> -       shrink_readahead_size_eio(ra);
>         return VM_FAULT_SIGBUS;

But ->readpage() doesn't check for !mapping (does it?).  So the
->readpage() cannot return AOP_TRUNCATED_PAGE.

However filemap_read_page() does check for !mapping.  So current -linus
doesn't check for !mapping, and post-willy does do this?

  reply	other threads:[~2021-03-04  0:27 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-26 14:00 [PATCH] mm/filemap: Use filemap_read_page in filemap_fault Matthew Wilcox (Oracle)
2021-03-03  1:30 ` Andrew Morton
2021-03-03  1:33   ` Matthew Wilcox
2021-03-03  6:07     ` Andrew Morton [this message]
2021-03-03 13:26       ` Matthew Wilcox
2021-03-03 20:12         ` Andrew Morton
2021-03-03 20:57           ` Matthew Wilcox
2021-03-03 21:51             ` Andrew Morton
2021-03-03 22:31               ` 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=20210302220735.1f150f28323f676d2955ab49@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=hch@lst.de \
    --cc=kent.overstreet@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=willy@infradead.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.