Hi,
----- Original Message -----
>
>
> On 19/11/18 13:29, Bob Peterson wrote:
> > This is another baby step toward a better glock state machine.
> > This patch eliminates a goto in function finish_xmote so we can
> > begin unraveling the cryptic logic with later patches.
> >
> > Signed-off-by: Bob Peterson <rpete...@redhat.com>
> > ---
> > fs/gfs2/glock.c | 11 +++++------
> > 1 file changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
> > index 5f2156f15f05..6e9d53583b73 100644
> > --- a/fs/gfs2/glock.c
> > +++ b/fs/gfs2/glock.c
> > @@ -472,11 +472,11 @@ static void finish_xmote(struct gfs2_glock *gl,
> > unsigned int ret)
> > list_move_tail(&gh->gh_list,
> > &gl->gl_holders);
> > gh = find_first_waiter(gl);
> > gl->gl_target = gh->gh_state;
> > - goto retry;
> > - }
> > - /* Some error or failed "try lock" - report it */
> > - if ((ret & LM_OUT_ERROR) ||
> > - (gh->gh_flags & (LM_FLAG_TRY | LM_FLAG_TRY_1CB))) {
> > + state = LM_ST_UNLOCKED;
> I'm not sure what you are trying to achieve here, but setting the state
> to LM_ST_UNLOCKED when it is quite possible that it is not that state,
> doesn't really seem to improve anything. Indeed, it looks more confusing
> to me, at least it was fairly clear before that the intent was to retry
> the operation which has been canceled.
When finish_xmote hits this affected section of code, it's because the dlm
returned a state different from the intended state. Before this patch, it
did "goto retry" which jumps to the label inside the switch state that
handles LM_ST_UNLOCKED, after which it simply unlocks and returns.
Changing local variable "state" merely forces the code to take the same
codepath in which it calls do_xmote, unlocking and returning as it does today,
but without the goto. This makes the function more suitable to the new
autonomous state machine, which is added in a later patch.
The addition of "else if" is needed so it doesn't go down the wrong code path
at the comment: /* Some error or failed "try lock" - report it */
The logic is a bit tricky here, but is preserved from the original.
Most of the subsequent patches aren't quite as mind-bending, I promise. :)
Regards,
Bob Peterson
