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

[Bob Peterson]

----- 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@xxxxxxxxxx>
| ---
|  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?

|  	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.

(snip)

Regards,

Bob Peterson
Red Hat File Systems