On Fri, Apr 21, 2023 at 9:07 PM Bob Peterson <rpete...@redhat.com> wrote:
> Before this patch, function gfs2_ail_empty_gl did not return errors it
> encountered from __gfs2_trans_begin. Those errors usually came from the
> fact that the file system was made read-only, often due to unmount,
> (but theoretically could be due to -o remount,ro) which prevented
> the transaction from starting.
>
> The inability to start a transaction prevented its revokes from being
> properly written to the journal for glocks during unmount (and
> transition to ro).
>
> That meant glocks could be unlocked without the metadata properly
> revoked in the journal. So other nodes could grab the glock thinking
> that their lvb values were correct, but in fact corresponded to the
> glock without its revokes properly synced. That presented as lvb
> mismatch errors.
>
> This patch allows gfs2_ail_empty_gl to return the error properly to
> the caller.
Alright,
> Signed-off-by: Bob Peterson <rpete...@redhat.com>
> ---
> fs/gfs2/glops.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
> index 6e33c8058059..8e245d793e6b 100644
> --- a/fs/gfs2/glops.c
> +++ b/fs/gfs2/glops.c
> @@ -90,7 +90,7 @@ static int gfs2_ail_empty_gl(struct gfs2_glock *gl)
> struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
> struct gfs2_trans tr;
> unsigned int revokes;
> - int ret;
> + int ret = 0;
>
> revokes = atomic_read(&gl->gl_ail_count);
>
> @@ -124,15 +124,15 @@ static int gfs2_ail_empty_gl(struct gfs2_glock *gl)
> memset(&tr, 0, sizeof(tr));
> set_bit(TR_ONSTACK, &tr.tr_flags);
> ret = __gfs2_trans_begin(&tr, sdp, 0, revokes, _RET_IP_);
> - if (ret)
> - goto flush;
> - __gfs2_ail_flush(gl, 0, revokes);
> - gfs2_trans_end(sdp);
> + if (!ret) {
> + __gfs2_ail_flush(gl, 0, revokes);
> + gfs2_trans_end(sdp);
> + }
but the above hunk doesn't help, so let me skip that.
> flush:
> gfs2_log_flush(sdp, NULL, GFS2_LOG_HEAD_FLUSH_NORMAL |
> GFS2_LFC_AIL_EMPTY_GL);
> - return 0;
> + return ret;
> }
>
> void gfs2_ail_flush(struct gfs2_glock *gl, bool fsync)
> @@ -326,7 +326,9 @@ static int inode_go_sync(struct gfs2_glock *gl)
> ret = gfs2_inode_metasync(gl);
> if (!error)
> error = ret;
> - gfs2_ail_empty_gl(gl);
> + ret = gfs2_ail_empty_gl(gl);
> + if (!error)
> + error = ret;
> /*
> * Writeback of the data mapping may cause the dirty flag to be set
> * so we have to clear it again here.
> --
> 2.39.2
>
Thanks,
Andreas
