From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bob Peterson Date: Mon, 19 Nov 2018 16:26:21 -0500 (EST) Subject: [Cluster-devel] [PATCH 03/13] GFS2: Eliminate a goto in finish_xmote In-Reply-To: References: <20181119132931.15726-1-rpeterso@redhat.com> <20181119132931.15726-4-rpeterso@redhat.com> Message-ID: <663356246.35122165.1542662781583.JavaMail.zimbra@redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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 > > --- > > 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