Re: [PATCH] mm/filemap: Use filemap_read_page in filemap_fault

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 3 Mar 2021 01:33:13 +0000 Matthew Wilcox <willy@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx> 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?



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux