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


Groups > linux.kernel > #1629786

Re: [PATCH v3 20/20] gfs2: clean up some filemap_* calls

From Jeff Layton <jlayton@redhat.com>
Newsgroups linux.kernel
Subject Re: [PATCH v3 20/20] gfs2: clean up some filemap_* calls
Date 2017-04-24 19:00 +0200
Message-ID <tzPvb-118-11@gated-at.bofh.it> (permalink)
References <tzMdX-7wX-1@gated-at.bofh.it> <tzMdZ-7wX-45@gated-at.bofh.it> <tzN0m-83T-27@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Mon, 2017-04-24 at 10:12 -0400, Bob Peterson wrote:
> ----- Original Message -----
> > In some places, it's trying to reset the mapping error after calling
> > filemap_fdatawait. That's no longer required. Also, turn several
> > filemap_fdatawrite+filemap_fdatawait calls into filemap_write_and_wait.
> > That will at least return writeback errors that occur during the write
> > phase.
> > 
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> >  fs/gfs2/glops.c | 12 ++++--------
> >  fs/gfs2/lops.c  |  4 +---
> >  fs/gfs2/super.c |  6 ++----
> >  3 files changed, 7 insertions(+), 15 deletions(-)
> > 
> > diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
> > index 5db59d444838..7362d19fdc4c 100644
> > --- a/fs/gfs2/glops.c
> > +++ b/fs/gfs2/glops.c
> > @@ -158,9 +158,7 @@ static void rgrp_go_sync(struct gfs2_glock *gl)
> >  	GLOCK_BUG_ON(gl, gl->gl_state != LM_ST_EXCLUSIVE);
> >  
> >  	gfs2_log_flush(sdp, gl, NORMAL_FLUSH);
> > -	filemap_fdatawrite_range(mapping, gl->gl_vm.start, gl->gl_vm.end);
> > -	error = filemap_fdatawait_range(mapping, gl->gl_vm.start, gl->gl_vm.end);
> > -	mapping_set_error(mapping, error);
> > +	filemap_write_and_wait_range(mapping, gl->gl_vm.start, gl->gl_vm.end);
> 
> This should probably have "error = ", no?
> 

This error is discarded in the current code after resetting the error in
the mapping. With the earlier patches in this set we don't need to reset
the error like this anymore.

Now, if this code should doing something else with those errors, then
that's a separate problem.

> >  	gfs2_ail_empty_gl(gl);
> >  
> >  	spin_lock(&gl->gl_lockref.lock);
> > @@ -225,12 +223,10 @@ static void inode_go_sync(struct gfs2_glock *gl)
> >  	filemap_fdatawrite(metamapping);
> >  	if (ip) {
> >  		struct address_space *mapping = ip->i_inode.i_mapping;
> > -		filemap_fdatawrite(mapping);
> > -		error = filemap_fdatawait(mapping);
> > -		mapping_set_error(mapping, error);
> > +		filemap_write_and_wait(mapping);
> > +	} else {
> > +		filemap_fdatawait(metamapping);
> >  	}
> > -	error = filemap_fdatawait(metamapping);
> > -	mapping_set_error(metamapping, error);
> 
> This part doesn't look right at all. There's a big difference in gfs2 between
> mapping and metamapping. We need to wait for metamapping regardless.
> 

...and this should wait. Basically, filemap_write_and_wait does
filemap_fdatawrite and then filemap_fdatawait. This is mostly just
replacing the existing code with a more concise helper.

-- 
Jeff Layton <jlayton@redhat.com>

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


Thread

[PATCH v3 00/20] fs: introduce new writeback error reporting and convert existing API as a wrapper around it Jeff Layton <jlayton@redhat.com> - 2017-04-24 15:30 +0200
  [PATCH v3 09/20] 9p: set mapping error when writeback fails in launder_page Jeff Layton <jlayton@redhat.com> - 2017-04-24 15:30 +0200
    Re: [PATCH v3 09/20] 9p: set mapping error when writeback fails in  launder_page Christoph Hellwig <hch@infradead.org> - 2017-04-24 17:30 +0200
    Re: [PATCH v3 09/20] 9p: set mapping error when writeback fails in  launder_page Jan Kara <jack@suse.cz> - 2017-04-24 18:00 +0200
  [PATCH v3 08/20] mm: ensure that we set mapping error if writeout() fails Jeff Layton <jlayton@redhat.com> - 2017-04-24 15:30 +0200
    Re: [PATCH v3 08/20] mm: ensure that we set mapping error if  writeout() fails Christoph Hellwig <hch@infradead.org> - 2017-04-24 17:30 +0200
    Re: [PATCH v3 08/20] mm: ensure that we set mapping error if  writeout() fails Jan Kara <jack@suse.cz> - 2017-04-24 18:00 +0200
  [PATCH v3 04/20] fs: check for writeback errors after syncing out buffers in generic_file_fsync Jeff Layton <jlayton@redhat.com> - 2017-04-24 15:30 +0200
    Re: [PATCH v3 04/20] fs: check for writeback errors after syncing  out buffers in generic_file_fsync Christoph Hellwig <hch@infradead.org> - 2017-04-24 17:30 +0200
    Re: [PATCH v3 04/20] fs: check for writeback errors after syncing  out buffers in generic_file_fsync Jan Kara <jack@suse.cz> - 2017-04-24 17:50 +0200
  [PATCH v3 06/20] dax: set errors in mapping when writeback fails Jeff Layton <jlayton@redhat.com> - 2017-04-24 15:30 +0200
    Re: [PATCH v3 06/20] dax: set errors in mapping when writeback fails Christoph Hellwig <hch@infradead.org> - 2017-04-24 17:30 +0200
    Re: [PATCH v3 06/20] dax: set errors in mapping when writeback fails Jan Kara <jack@suse.cz> - 2017-04-24 18:00 +0200
    Re: [PATCH v3 06/20] dax: set errors in mapping when writeback fails Ross Zwisler <ross.zwisler@linux.intel.com> - 2017-04-24 21:20 +0200
  [PATCH v3 12/20] lib: add errseq_t type and infrastructure for handling it Jeff Layton <jlayton@redhat.com> - 2017-04-24 15:30 +0200
  [PATCH v3 03/20] buffer: use mapping_set_error instead of setting the flag Jeff Layton <jlayton@redhat.com> - 2017-04-24 15:30 +0200
  [PATCH v3 10/20] fuse: set mapping error in writepage_locked when it fails Jeff Layton <jlayton@redhat.com> - 2017-04-24 15:30 +0200
    Re: [PATCH v3 10/20] fuse: set mapping error in writepage_locked  when it fails Jan Kara <jack@suse.cz> - 2017-04-24 18:10 +0200
      Re: [PATCH v3 10/20] fuse: set mapping error in writepage_locked  when it fails Jeff Layton <jlayton@redhat.com> - 2017-04-24 19:20 +0200
        Re: [PATCH v3 10/20] fuse: set mapping error in writepage_locked  when it fails Jan Kara <jack@suse.cz> - 2017-04-25 11:50 +0200
          Re: [PATCH v3 10/20] fuse: set mapping error in writepage_locked  when it fails Jeff Layton <jlayton@redhat.com> - 2017-04-25 12:40 +0200
            Re: [PATCH v3 10/20] fuse: set mapping error in writepage_locked  when it fails Jan Kara <jack@suse.cz> - 2017-04-25 13:30 +0200
              Re: [PATCH v3 10/20] fuse: set mapping error in writepage_locked  when it fails Jeff Layton <jlayton@redhat.com> - 2017-04-25 18:50 +0200
  [PATCH v3 01/20] mm: drop "wait" parameter from write_one_page Jeff Layton <jlayton@redhat.com> - 2017-04-24 15:30 +0200
    Re: [PATCH v3 01/20] mm: drop "wait" parameter from write_one_page Christoph Hellwig <hch@infradead.org> - 2017-04-24 17:30 +0200
  [PATCH v3 15/20] mm: remove AS_EIO and AS_ENOSPC flags Jeff Layton <jlayton@redhat.com> - 2017-04-24 15:30 +0200
  [PATCH v3 18/20] mm: clean up error handling in write_one_page Jeff Layton <jlayton@redhat.com> - 2017-04-24 15:30 +0200
  [PATCH v3 16/20] mm: don't TestClearPageError in __filemap_fdatawait_range Jeff Layton <jlayton@redhat.com> - 2017-04-24 15:30 +0200
  [PATCH v3 20/20] gfs2: clean up some filemap_* calls Jeff Layton <jlayton@redhat.com> - 2017-04-24 15:30 +0200
    Re: [PATCH v3 20/20] gfs2: clean up some filemap_* calls Bob Peterson <rpeterso@redhat.com> - 2017-04-24 16:20 +0200
      Re: [PATCH v3 20/20] gfs2: clean up some filemap_* calls Jeff Layton <jlayton@redhat.com> - 2017-04-24 19:00 +0200
        Re: [PATCH v3 20/20] gfs2: clean up some filemap_* calls Bob Peterson <rpeterso@redhat.com> - 2017-04-24 19:50 +0200
          Re: [PATCH v3 20/20] gfs2: clean up some filemap_* calls Jeff Layton <jlayton@redhat.com> - 2017-04-24 20:00 +0200
  [PATCH v3 07/20] nilfs2: set the mapping error when calling SetPageError on writeback Jeff Layton <jlayton@redhat.com> - 2017-04-24 15:30 +0200
    Re: [PATCH v3 07/20] nilfs2: set the mapping error when calling  SetPageError on writeback Christoph Hellwig <hch@infradead.org> - 2017-04-24 17:30 +0200
  [PATCH v3 13/20] fs: new infrastructure for writeback error handling and reporting Jeff Layton <jlayton@redhat.com> - 2017-04-24 15:30 +0200
  [PATCH v3 17/20] cifs: cleanup writeback handling errors and comments Jeff Layton <jlayton@redhat.com> - 2017-04-24 15:30 +0200
  [PATCH v3 19/20] jbd2: don't reset error in journal_finish_inode_data_buffers Jeff Layton <jlayton@redhat.com> - 2017-04-24 15:30 +0200
  [PATCH v3 02/20] mm: fix mapping_set_error call in me_pagecache_dirty Jeff Layton <jlayton@redhat.com> - 2017-04-24 15:40 +0200
  [PATCH v3 05/20] orangefs: don't call filemap_write_and_wait from fsync Jeff Layton <jlayton@redhat.com> - 2017-04-24 15:40 +0200
    Re: [PATCH v3 05/20] orangefs: don't call filemap_write_and_wait  from fsync Christoph Hellwig <hch@infradead.org> - 2017-04-24 17:30 +0200
      Re: [PATCH v3 05/20] orangefs: don't call filemap_write_and_wait from fsync Mike Marshall <hubcap@omnibond.com> - 2017-04-24 20:30 +0200
  [PATCH v3 14/20] fs: retrofit old error reporting API onto new infrastructure Jeff Layton <jlayton@redhat.com> - 2017-04-24 15:40 +0200
  [PATCH v3 11/20] cifs: set mapping error when page writeback fails in writepage or launder_pages Jeff Layton <jlayton@redhat.com> - 2017-04-24 15:40 +0200
    Re: [PATCH v3 11/20] cifs: set mapping error when page writeback  fails in writepage or launder_pages Christoph Hellwig <hch@infradead.org> - 2017-04-24 17:30 +0200
      Re: [PATCH v3 11/20] cifs: set mapping error when page writeback  fails in writepage or launder_pages Jeff Layton <jlayton@redhat.com> - 2017-04-24 19:20 +0200

csiph-web