[Cluster-devel] [GFS2 PATCH 1/3] gfs2: switch go_xmote_bh glop to pass gh not gl

[Andreas Gruenbacher]

On Tue, Aug 24, 2021 at 6:48 PM Bob Peterson <rpeterso at redhat.com> wrote:
> On 8/24/21 11:12 AM, Andreas Gruenbacher wrote:
> > On Tue, Aug 24, 2021 at 4:02 PM Bob Peterson <rpeterso at redhat.com> wrote:
> >> Before this patch, the go_xmote_bh function was passed gl, the glock
> >> pointer. This patch switches it to gh, the holder, which points to the gl.
> >> This facilitates improvements for the next patch.
> >>
> >> Signed-off-by: Bob Peterson <rpeterso at redhat.com>
> >> ---
> >>   fs/gfs2/glock.c  | 4 ++--
> >>   fs/gfs2/glops.c  | 5 +++--
> >>   fs/gfs2/incore.h | 2 +-
> >>   3 files changed, 6 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
> >> index e0eaa9cf9fb6..d43eed1696ab 100644
> >> --- a/fs/gfs2/glock.c
> >> +++ b/fs/gfs2/glock.c
> >> @@ -562,9 +562,9 @@ static void finish_xmote(struct gfs2_glock *gl, unsigned int ret)
> >>          if (test_and_clear_bit(GLF_DEMOTE_IN_PROGRESS, &gl->gl_flags))
> >>                  gfs2_demote_wake(gl);
> >>          if (state != LM_ST_UNLOCKED) {
> >> -               if (glops->go_xmote_bh) {
> >> +               if (gh && glops->go_xmote_bh) {
> >
> > This changes when the callback is called. Please explain why that's okay.
>
> This is okay because patch 3 eliminates go_xmote_bh() completely anyway.
> I just threw the "gh &&" as an abundance of precaution.

I didn't mean this as a joke. This patch changes when the go_xmote_bh
hook is called, and there is no mention why that's a safe thing to do.
Then the go_xmote_bh hook is removed in favor of go_lock, which is yet
called under different circumstances, and again there is no mention
why that doesn't matter and we still end up calling freeze_go_xmote_bh
(now freeze_go_lock) in the right circumstances.

Also, it's never been okay to break things just because a patch
further down the line fixes it again (or breaks it in a different
way). Please explain your changes; this also serves to document that
you haven't accidentally missed a case.

Thanks,
Andreas