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

Bob Peterson
2017-04-24T14:13:07Z

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

| 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