Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1595443

Re: [PATCH v2 6/9] mm: set mapping error when launder_pages fails

From Jeff Layton <jlayton@redhat.com>
Newsgroups linux.kernel
Subject Re: [PATCH v2 6/9] mm: set mapping error when launder_pages fails
Date 2017-03-08 19:50 +0100
Message-ID <tiOOS-1ZF-15@gated-at.bofh.it> (permalink)
References <tiMN3-GJ-3@gated-at.bofh.it> <tiN6r-Q0-37@gated-at.bofh.it> <tiOc9-1K6-11@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Wed, 2017-03-08 at 18:01 +0000, Trond Myklebust wrote:
> On Wed, 2017-03-08 at 11:29 -0500, Jeff Layton wrote:
> > If launder_page fails, then we hit a problem writing back some inode
> > data. Ensure that we communicate that fact in a subsequent fsync
> > since
> > another task could still have it open for write.
> > 
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> >  mm/truncate.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/mm/truncate.c b/mm/truncate.c
> > index 6263affdef88..29ae420a5bf9 100644
> > --- a/mm/truncate.c
> > +++ b/mm/truncate.c
> > @@ -594,11 +594,15 @@ invalidate_complete_page2(struct address_space
> > *mapping, struct page *page)
> >  
> >  static int do_launder_page(struct address_space *mapping, struct
> > page *page)
> >  {
> > +	int ret;
> > +
> >  	if (!PageDirty(page))
> >  		return 0;
> >  	if (page->mapping != mapping || mapping->a_ops->launder_page 
> > == NULL)
> >  		return 0;
> > -	return mapping->a_ops->launder_page(page);
> > +	ret = mapping->a_ops->launder_page(page);
> > +	mapping_set_error(mapping, ret);
> > +	return ret;
> >  }
> >  
> >  /**
> 
> No. At that layer, you don't know that this is a page error. In the NFS
> case, it could, for instance, just as well be a fatal signal.
> 

Ok...don't we have the same problem with writepage then? Most of the
writepage callers will set an error in the mapping if writepage returns
any sort of error? A fatal signal in that codepath could cause the same
problem, it seems. We don't dip into direct reclaim so much anymore, so
maybe signals aren't an issue there?

The alternative here would be to push this down into the callers. I
worry a bit though about getting this right across filesystems though.
It'd be preferable it if we could keep the mapping_set_error call in
generic VFS code instead, but if not then I'll just plan to do that.

Thanks,
-- 
Jeff Layton <jlayton@redhat.com>

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH v2 0/9] mm/fs: get PG_error out of the writeback reporting business Jeff Layton <jlayton@redhat.com> - 2017-03-08 18:00 +0100
  [PATCH v2 9/9] Documentation: document what to do on a writeback error Jeff Layton <jlayton@redhat.com> - 2017-03-08 18:00 +0100
  [PATCH v2 5/9] dax: set error in mapping when writeback fails Jeff Layton <jlayton@redhat.com> - 2017-03-08 18:00 +0100
    Re: [PATCH v2 5/9] dax: set error in mapping when writeback fails Ross Zwisler <ross.zwisler@linux.intel.com> - 2017-03-10 01:30 +0100
  [PATCH v2 6/9] mm: set mapping error when launder_pages fails Jeff Layton <jlayton@redhat.com> - 2017-03-08 18:00 +0100
    Re: [PATCH v2 6/9] mm: set mapping error when launder_pages fails Trond Myklebust <trondmy@primarydata.com> - 2017-03-08 19:10 +0100
      Re: [PATCH v2 6/9] mm: set mapping error when launder_pages fails Jeff Layton <jlayton@redhat.com> - 2017-03-08 19:50 +0100
        Re: [PATCH v2 6/9] mm: set mapping error when launder_pages fails Trond Myklebust <trondmy@primarydata.com> - 2017-03-08 20:30 +0100
      Re: [PATCH v2 6/9] mm: set mapping error when launder_pages fails NeilBrown <neilb@suse.com> - 2017-03-08 23:50 +0100
  [PATCH v2 4/9] nilfs2: set the mapping error when calling SetPageError on writeback Jeff Layton <jlayton@redhat.com> - 2017-03-08 18:00 +0100

csiph-web