[Cluster-devel] [PATCH 18/19] gfs2: don't call go_unlock unless demote is close at hand

[Bob Peterson]

The go_unlock glops function is only used when unlocking rgrps.
Before this patch, the glock code did:
 1. spin_unlock(&gl->gl_lockref.lock);
 2. go_unlock
 3. spin_lock(&gl->gl_lockref.lock);
The go_unlock function checked for the GLF_DEMOTE or DEMOTE_PENDING
bits, and if set, called rgrp_go_unlock. But doing it that way
leaves a race: by the time rgrp_go_unlock returns, another process
may have set GLF_DEMOTE, causing the go_unlock to have been skipped.

This patch changes function gfs2_glock_dq so that it does not
release the gl_lockref lock until after the test for DEMOTE is
complete, and we know for sure whether it needs demoting (for this go).
That way, nobody can sneak in and change the flag during the function.

For simplicity's sake, I also changed the go_unlock parameter to
accept the glock rather than the holder.

Signed-off-by: Bob Peterson <rpeterso at redhat.com>
---
 fs/gfs2/glock.c  | 9 ++++++---
 fs/gfs2/incore.h | 2 +-
 fs/gfs2/rgrp.c   | 8 +++-----
 fs/gfs2/rgrp.h   | 2 +-
 4 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 72a7b19c3aef..465de5ef8a5d 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -1173,9 +1173,12 @@ void gfs2_glock_dq(struct gfs2_holder *gh)
 	if (find_first_holder(gl) == NULL) {
 		if (glops->go_unlock) {
 			GLOCK_BUG_ON(gl, test_and_set_bit(GLF_LOCK, &gl->gl_flags));
-			spin_unlock(&gl->gl_lockref.lock);
-			glops->go_unlock(gh);
-			spin_lock(&gl->gl_lockref.lock);
+			if (test_bit(GLF_DEMOTE, &gl->gl_flags) ||
+			    test_bit(GLF_PENDING_DEMOTE, &gl->gl_flags)) {
+				spin_unlock(&gl->gl_lockref.lock);
+				glops->go_unlock(gl);
+				spin_lock(&gl->gl_lockref.lock);
+			}
 			clear_bit(GLF_LOCK, &gl->gl_flags);
 		}
 		if (list_empty(&gl->gl_holders) &&
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index 341590b348a3..94afe91deb73 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -242,7 +242,7 @@ struct gfs2_glock_operations {
 	void (*go_inval) (struct gfs2_glock *gl, int flags);
 	int (*go_demote_ok) (const struct gfs2_glock *gl);
 	int (*go_lock) (struct gfs2_holder *gh);
-	void (*go_unlock) (struct gfs2_holder *gh);
+	void (*go_unlock) (struct gfs2_glock *gl);
 	void (*go_dump)(struct seq_file *seq, struct gfs2_glock *gl);
 	void (*go_callback)(struct gfs2_glock *gl, bool remote);
 	const int go_type;
diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index 52a4f340a867..2a7f47923f88 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -1306,13 +1306,11 @@ void gfs2_rgrp_brelse(struct gfs2_rgrpd *rgd)
  *
  */
 
-void gfs2_rgrp_go_unlock(struct gfs2_holder *gh)
+void gfs2_rgrp_go_unlock(struct gfs2_glock *gl)
 {
-	struct gfs2_rgrpd *rgd = gh->gh_gl->gl_object;
-	int demote_requested = test_bit(GLF_DEMOTE, &gh->gh_gl->gl_flags) |
-		test_bit(GLF_PENDING_DEMOTE, &gh->gh_gl->gl_flags);
+	struct gfs2_rgrpd *rgd = gl->gl_object;
 
-	if (rgd && demote_requested)
+	if (rgd)
 		gfs2_rgrp_brelse(rgd);
 }
 
diff --git a/fs/gfs2/rgrp.h b/fs/gfs2/rgrp.h
index 499079a9dbbe..f2b6665857e1 100644
--- a/fs/gfs2/rgrp.h
+++ b/fs/gfs2/rgrp.h
@@ -36,7 +36,7 @@ extern int gfs2_rindex_update(struct gfs2_sbd *sdp);
 extern void gfs2_free_clones(struct gfs2_rgrpd *rgd);
 extern int gfs2_rgrp_go_lock(struct gfs2_holder *gh);
 extern void gfs2_rgrp_brelse(struct gfs2_rgrpd *rgd);
-extern void gfs2_rgrp_go_unlock(struct gfs2_holder *gh);
+extern void gfs2_rgrp_go_unlock(struct gfs2_glock *gl);
 
 extern struct gfs2_alloc *gfs2_alloc_get(struct gfs2_inode *ip);
 
-- 
2.20.1