linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@redhat.com>
To: Trond Myklebust <trondmy@primarydata.com>,
	"viro@zeniv.linux.org.uk" <viro@zeniv.linux.org.uk>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>
Cc: "linux-nilfs@vger.kernel.org" <linux-nilfs@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"konishi.ryusuke@lab.ntt.co.jp" <konishi.ryusuke@lab.ntt.co.jp>,
	"neilb@suse.com" <neilb@suse.com>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"adilger@dilger.ca" <adilger@dilger.ca>,
	"James.Bottomley@HansenPartnership.com"
	<James.Bottomley@HansenPartnership.com>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"ross.zwisler@linux.intel.com" <ross.zwisler@linux.intel.com>,
	"openosd@gmail.com" <openosd@gmail.com>,
	"jack@suse.cz" <jack@suse.cz>
Subject: Re: [PATCH v2 6/9] mm: set mapping error when launder_pages fails
Date: Wed, 08 Mar 2017 13:38:08 -0500	[thread overview]
Message-ID: <1488998288.2802.25.camel@redhat.com> (raw)
In-Reply-To: <1488996103.3098.4.camel@primarydata.com>

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>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2017-03-08 18:38 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-08 16:29 [PATCH v2 0/9] mm/fs: get PG_error out of the writeback reporting business Jeff Layton
2017-03-08 16:29 ` [PATCH v2 1/9] mm: fix mapping_set_error call in me_pagecache_dirty Jeff Layton
2017-03-10  0:06   ` Ross Zwisler
2017-03-08 16:29 ` [PATCH v2 2/9] mm: drop "wait" parameter from write_one_page Jeff Layton
2017-03-10  0:07   ` Ross Zwisler
2017-03-08 16:29 ` [PATCH v2 3/9] mm: clear any AS_* errors when returning error on any fsync or close Jeff Layton
2017-03-08 21:23   ` NeilBrown
2017-03-09  0:10     ` Jeff Layton
2017-03-10  0:09   ` Ross Zwisler
2017-03-10  3:08     ` Jeff Layton
2017-03-08 16:29 ` [PATCH v2 4/9] nilfs2: set the mapping error when calling SetPageError on writeback Jeff Layton
2017-03-08 16:29 ` [PATCH v2 5/9] dax: set error in mapping when writeback fails Jeff Layton
2017-03-10  0:21   ` Ross Zwisler
2017-03-08 16:29 ` [PATCH v2 6/9] mm: set mapping error when launder_pages fails Jeff Layton
2017-03-08 18:01   ` Trond Myklebust
2017-03-08 18:38     ` Jeff Layton [this message]
2017-03-08 19:16       ` Trond Myklebust
2017-03-08 21:28     ` NeilBrown
2017-03-08 16:29 ` [PATCH v2 7/9] mm: ensure that we set mapping error if writeout() fails Jeff Layton
2017-03-08 16:29 ` [PATCH v2 8/9] mm: don't TestClearPageError in __filemap_fdatawait_range Jeff Layton
2017-03-08 16:29 ` [PATCH v2 9/9] Documentation: document what to do on a writeback error Jeff Layton

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=1488998288.2802.25.camel@redhat.com \
    --to=jlayton@redhat.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=adilger@dilger.ca \
    --cc=akpm@linux-foundation.org \
    --cc=jack@suse.cz \
    --cc=konishi.ryusuke@lab.ntt.co.jp \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-nilfs@vger.kernel.org \
    --cc=neilb@suse.com \
    --cc=openosd@gmail.com \
    --cc=ross.zwisler@linux.intel.com \
    --cc=trondmy@primarydata.com \
    --cc=viro@zeniv.linux.org.uk \
    /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;
as well as URLs for NNTP newsgroup(s).