cluster-devel.redhat.com archive mirror
 help / color / mirror / Atom feed
From: swhiteho@redhat.com <swhiteho@redhat.com>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [PATCH 46/48] [GFS2] Invalidate cache at correct point
Date: Thu, 17 Apr 2008 09:39:22 +0100	[thread overview]
Message-ID: <12084216641014-git-send-email-swhiteho@redhat.com> (raw)
In-Reply-To: <12084216611008-git-send-email-swhiteho@redhat.com>

From: Benjamin Marzinski <bmarzins@redhat.com>

GFS2 wasn't invalidating its cache before it called into the lock manager
with a request that could potentially drop a lock.  This was leaving a
window where the lock could be actually be held by another node, but the
file's page cache would still appear valid, causing coherency problems.
This patch moves the cache invalidation to before the lock manager call
when dropping a lock. It also adds the option to the lock_dlm lock
manager to not use conversion mode deadlock avoidance, which, on a
conversion from shared to exclusive, could internally drop the lock, and
then reacquire in. GFS2 now asks lock_dlm to not do this.  Instead, GFS2
manually drops the lock and reacquires it.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 63981e2..d636b3e 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -764,7 +764,7 @@ static void state_change(struct gfs2_glock *gl, unsigned int new_state)
 static void drop_bh(struct gfs2_glock *gl, unsigned int ret)
 {
 	struct gfs2_sbd *sdp = gl->gl_sbd;
-	const struct gfs2_glock_operations *glops = gl->gl_ops;
+	struct gfs2_holder *gh = gl->gl_req_gh;
 
 	gfs2_assert_warn(sdp, test_bit(GLF_LOCK, &gl->gl_flags));
 	gfs2_assert_warn(sdp, list_empty(&gl->gl_holders));
@@ -772,8 +772,14 @@ static void drop_bh(struct gfs2_glock *gl, unsigned int ret)
 
 	state_change(gl, LM_ST_UNLOCKED);
 
-	if (glops->go_inval)
-		glops->go_inval(gl, DIO_METADATA);
+	if (test_and_clear_bit(GLF_CONV_DEADLK, &gl->gl_flags)) {
+		spin_lock(&gl->gl_spin);
+		gh->gh_error = 0;
+		spin_unlock(&gl->gl_spin);
+		gfs2_glock_xmote_th(gl, gl->gl_req_gh);
+		gfs2_glock_put(gl);
+		return;
+	}
 
 	spin_lock(&gl->gl_spin);
 	gfs2_demote_wake(gl);
@@ -794,7 +800,6 @@ static void xmote_bh(struct gfs2_glock *gl, unsigned int ret)
 	struct gfs2_sbd *sdp = gl->gl_sbd;
 	const struct gfs2_glock_operations *glops = gl->gl_ops;
 	struct gfs2_holder *gh = gl->gl_req_gh;
-	int prev_state = gl->gl_state;
 	int op_done = 1;
 
 	if (!gh && (ret & LM_OUT_ST_MASK) == LM_ST_UNLOCKED) {
@@ -808,16 +813,6 @@ static void xmote_bh(struct gfs2_glock *gl, unsigned int ret)
 
 	state_change(gl, ret & LM_OUT_ST_MASK);
 
-	if (prev_state != LM_ST_UNLOCKED && !(ret & LM_OUT_CACHEABLE)) {
-		if (glops->go_inval)
-			glops->go_inval(gl, DIO_METADATA);
-	} else if (gl->gl_state == LM_ST_DEFERRED) {
-		/* We might not want to do this here.
-		   Look at moving to the inode glops. */
-		if (glops->go_inval)
-			glops->go_inval(gl, 0);
-	}
-
 	/*  Deal with each possible exit condition  */
 
 	if (!gh) {
@@ -837,6 +832,14 @@ static void xmote_bh(struct gfs2_glock *gl, unsigned int ret)
 		}
 	} else {
 		spin_lock(&gl->gl_spin);
+		if (ret & LM_OUT_CONV_DEADLK) {
+			gh->gh_error = 0;
+			set_bit(GLF_CONV_DEADLK, &gl->gl_flags);
+			spin_unlock(&gl->gl_spin);
+			gfs2_glock_drop_th(gl);
+			gfs2_glock_put(gl);
+			return;
+		}
 		list_del_init(&gh->gh_list);
 		gh->gh_error = -EIO;
 		if (unlikely(test_bit(SDF_SHUTDOWN, &sdp->sd_flags))) 
@@ -910,6 +913,8 @@ static void gfs2_glock_xmote_th(struct gfs2_glock *gl, struct gfs2_holder *gh)
 
 	if (glops->go_xmote_th)
 		glops->go_xmote_th(gl);
+	if (state == LM_ST_DEFERRED && glops->go_inval)
+		glops->go_inval(gl, DIO_METADATA);
 
 	gfs2_assert_warn(sdp, test_bit(GLF_LOCK, &gl->gl_flags));
 	gfs2_assert_warn(sdp, list_empty(&gl->gl_holders));
@@ -952,6 +957,8 @@ static void gfs2_glock_drop_th(struct gfs2_glock *gl)
 
 	if (glops->go_xmote_th)
 		glops->go_xmote_th(gl);
+	if (glops->go_inval)
+		glops->go_inval(gl, DIO_METADATA);
 
 	gfs2_assert_warn(sdp, test_bit(GLF_LOCK, &gl->gl_flags));
 	gfs2_assert_warn(sdp, list_empty(&gl->gl_holders));
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index 4ba2ea6..9c2c0b9 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -167,6 +167,7 @@ enum {
 	GLF_DEMOTE_IN_PROGRESS	= 6,
 	GLF_LFLUSH		= 7,
 	GLF_WAITERS2		= 8,
+	GLF_CONV_DEADLK		= 9,
 };
 
 struct gfs2_glock {
diff --git a/fs/gfs2/locking/dlm/lock.c b/fs/gfs2/locking/dlm/lock.c
index 542a797..53a6ab3 100644
--- a/fs/gfs2/locking/dlm/lock.c
+++ b/fs/gfs2/locking/dlm/lock.c
@@ -137,7 +137,8 @@ static inline unsigned int make_flags(struct gdlm_lock *lp,
 
 		/* Conversion deadlock avoidance by DLM */
 
-		if (!test_bit(LFL_FORCE_PROMOTE, &lp->flags) &&
+		if (!(lp->ls->fsflags & LM_MFLAG_CONV_NODROP) &&
+		    !test_bit(LFL_FORCE_PROMOTE, &lp->flags) &&
 		    !(lkf & DLM_LKF_NOQUEUE) &&
 		    cur > DLM_LOCK_NL && req > DLM_LOCK_NL && cur != req)
 			lkf |= DLM_LKF_CONVDEADLK;
diff --git a/fs/gfs2/locking/dlm/thread.c b/fs/gfs2/locking/dlm/thread.c
index 521694f..e53db6f 100644
--- a/fs/gfs2/locking/dlm/thread.c
+++ b/fs/gfs2/locking/dlm/thread.c
@@ -135,7 +135,15 @@ static void process_complete(struct gdlm_lock *lp)
 			 lp->lksb.sb_status, lp->lockname.ln_type,
 			 (unsigned long long)lp->lockname.ln_number,
 			 lp->flags);
-		return;
+		if (lp->lksb.sb_status == -EDEADLOCK &&
+		    lp->ls->fsflags & LM_MFLAG_CONV_NODROP) {
+			lp->req = lp->cur;
+			acb.lc_ret |= LM_OUT_CONV_DEADLK;
+			if (lp->cur == DLM_LOCK_IV)
+				lp->lksb.sb_lkid = 0;
+			goto out;
+		} else
+			return;
 	}
 
 	/*
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 5b518f7..ef9c6c4 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -723,7 +723,7 @@ static int gfs2_lm_mount(struct gfs2_sbd *sdp, int silent)
 {
 	char *proto = sdp->sd_proto_name;
 	char *table = sdp->sd_table_name;
-	int flags = 0;
+	int flags = LM_MFLAG_CONV_NODROP;
 	int error;
 
 	if (sdp->sd_args.ar_spectator)
diff --git a/include/linux/lm_interface.h b/include/linux/lm_interface.h
index 1418fdc..f274997 100644
--- a/include/linux/lm_interface.h
+++ b/include/linux/lm_interface.h
@@ -21,9 +21,15 @@ typedef void (*lm_callback_t) (void *ptr, unsigned int type, void *data);
  * modify the filesystem.  The lock module shouldn't assign a journal to the FS
  * mount.  It shouldn't send recovery callbacks to the FS mount.  If the node
  * dies or withdraws, all locks can be wiped immediately.
+ *
+ * LM_MFLAG_CONV_NODROP
+ * Do not allow the dlm to internally resolve conversion deadlocks by demoting
+ * the lock to unlocked and then reacquiring it in the requested mode. Instead,
+ * it should cancel the request and return LM_OUT_CONV_DEADLK.
  */
 
 #define LM_MFLAG_SPECTATOR	0x00000001
+#define LM_MFLAG_CONV_NODROP	0x00000002
 
 /*
  * lm_lockstruct flags
@@ -110,6 +116,9 @@ typedef void (*lm_callback_t) (void *ptr, unsigned int type, void *data);
  *
  * LM_OUT_ASYNC
  * The result of the request will be returned in an LM_CB_ASYNC callback.
+ *
+ * LM_OUT_CONV_DEADLK
+ * The lock request was canceled do to a conversion deadlock.
  */
 
 #define LM_OUT_ST_MASK		0x00000003
@@ -117,6 +126,7 @@ typedef void (*lm_callback_t) (void *ptr, unsigned int type, void *data);
 #define LM_OUT_CANCELED		0x00000008
 #define LM_OUT_ASYNC		0x00000080
 #define LM_OUT_ERROR		0x00000100
+#define LM_OUT_CONV_DEADLK	0x00000200
 
 /*
  * lm_callback_t types
-- 
1.5.1.2



  reply	other threads:[~2008-04-17  8:39 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-17  8:37 [Cluster-devel] [GFS2] Pre-pull patch posting swhiteho
2008-04-17  8:38 ` [Cluster-devel] [PATCH 01/48] [GFS2] Speed up gfs2_write_alloc_required, deprecate gfs2_extent_map swhiteho
2008-04-17  8:38   ` [Cluster-devel] [PATCH 02/48] [GFS2] Streamline indirect pointer tree height calculation swhiteho
2008-04-17  8:38     ` [Cluster-devel] [PATCH 03/48] [GFS2] Get rid of unneeded parameter in gfs2_rlist_alloc swhiteho
2008-04-17  8:38       ` [Cluster-devel] [PATCH 04/48] [GFS2] Fix debug inode printing swhiteho
2008-04-17  8:38         ` [Cluster-devel] [PATCH 05/48] [GFS2] Only do lo_incore_commit once swhiteho
2008-04-17  8:38           ` [Cluster-devel] [PATCH 06/48] [GFS2] Misc fixups swhiteho
2008-04-17  8:38             ` [Cluster-devel] [PATCH 07/48] [GFS2] Only wake the reclaim daemon if we need to swhiteho
2008-04-17  8:38               ` [Cluster-devel] [PATCH 08/48] [GFS2] make gfs2_glock_hold() static swhiteho
2008-04-17  8:38                 ` [Cluster-devel] [PATCH 09/48] [GFS2] Plug an unlikely leak swhiteho
2008-04-17  8:38                   ` [Cluster-devel] [PATCH 10/48] [GFS2] Allocate gfs2_rgrpd from slab memory swhiteho
2008-04-17  8:38                     ` [Cluster-devel] [PATCH 11/48] [GFS2] Combine rg_flags and rd_flags swhiteho
2008-04-17  8:38                       ` [Cluster-devel] [PATCH 12/48] [GFS2] Get rid of gl_waiters2 swhiteho
2008-04-17  8:38                         ` [Cluster-devel] [PATCH 13/48] [GFS2] Move part of gfs2_block_map into a separate function swhiteho
2008-04-17  8:38                           ` [Cluster-devel] [PATCH 14/48] [GFS2] Introduce array of buffers to struct metapath swhiteho
2008-04-17  8:38                             ` [Cluster-devel] [PATCH 15/48] [GFS2] Add consts to various bits of rgrp.c swhiteho
2008-04-17  8:38                               ` [Cluster-devel] [PATCH 16/48] [GFS2] Eliminate gl_req_bh swhiteho
2008-04-17  8:38                                 ` [Cluster-devel] [PATCH 17/48] [GFS2] Remove lm.[ch] and distribute content swhiteho
2008-04-17  8:38                                   ` [Cluster-devel] [PATCH 18/48] [GFS2] Remove rgrp and glock version numbers swhiteho
2008-04-17  8:38                                     ` [Cluster-devel] [PATCH 19/48] [GFS2] Shrink & rename di_depth swhiteho
2008-04-17  8:38                                       ` [Cluster-devel] [PATCH 20/48] [GFS2] Remove unused counters swhiteho
2008-04-17  8:38                                         ` [Cluster-devel] [PATCH 21/48] [GFS2] Reduce inode size by merging fields swhiteho
2008-04-17  8:38                                           ` [Cluster-devel] [PATCH 22/48] [GFS2] Merge the rd_last_alloc_meta and rd_last_alloc_data fields swhiteho
2008-04-17  8:38                                             ` [Cluster-devel] [PATCH 23/48] [GFS2] Update gfs2_trans_add_unrevoke to accept extents swhiteho
2008-04-17  8:39                                               ` [Cluster-devel] [PATCH 24/48] [GFS2] Merge gfs2_alloc_meta and gfs2_alloc_data swhiteho
2008-04-17  8:39                                                 ` [Cluster-devel] [PATCH 25/48] [GFS2] Add extent allocation to block allocator swhiteho
2008-04-17  8:39                                                   ` [Cluster-devel] [PATCH 26/48] [GFS2] The case of the missing asterisk swhiteho
2008-04-17  8:39                                                     ` [Cluster-devel] [PATCH 27/48] [GFS2] Add a function to interate over an extent swhiteho
2008-04-17  8:39                                                       ` [Cluster-devel] [PATCH 28/48] [GFS2] Eliminate (almost) duplicate field from gfs2_inode swhiteho
2008-04-17  8:39                                                         ` [Cluster-devel] [PATCH 29/48] [GFS2] Get inode buffer only once per block map call swhiteho
2008-04-17  8:39                                                           ` [Cluster-devel] [PATCH 30/48] [GFS2] Fix bug where we called drop_bh incorrectly swhiteho
2008-04-17  8:39                                                             ` [Cluster-devel] [PATCH 31/48] [GFS2] be*_add_cpu conversion swhiteho
2008-04-17  8:39                                                               ` [Cluster-devel] [PATCH 32/48] [GFS2] gfs2/ops_file.c should #include "ops_inode.h" swhiteho
2008-04-17  8:39                                                                 ` [Cluster-devel] [PATCH 33/48] [GFS2] proper extern for gfs2/locking/dlm/mount.c:gdlm_ops swhiteho
2008-04-17  8:39                                                                   ` [Cluster-devel] [PATCH 34/48] [GFS2] Fix a page lock / glock deadlock swhiteho
2008-04-17  8:39                                                                     ` [Cluster-devel] [PATCH 35/48] [GFS2] Allow bmap to allocate extents swhiteho
2008-04-17  8:39                                                                       ` [Cluster-devel] [PATCH 36/48] [GFS2] fix file_system_type leak on gfs2meta mount swhiteho
2008-04-17  8:39                                                                         ` [Cluster-devel] [PATCH 37/48] [GFS2] remove gfs2_dev_iops swhiteho
2008-04-17  8:39                                                                           ` [Cluster-devel] [PATCH 38/48] [GFS2] re-support special inode swhiteho
2008-04-17  8:39                                                                             ` [Cluster-devel] [PATCH 39/48] [GFS2] Need to ensure that sector_t is 64bits for GFS2 swhiteho
2008-04-17  8:39                                                                               ` [Cluster-devel] [PATCH 40/48] [GFS2] possible null pointer dereference fixup swhiteho
2008-04-17  8:39                                                                                 ` [Cluster-devel] [PATCH 41/48] [GFS2] gfs2_adjust_quota has broken unstuffing code swhiteho
2008-04-17  8:39                                                                                   ` [Cluster-devel] [PATCH 42/48] [GFS2] Remove drop of module ref where not needed swhiteho
2008-04-17  8:39                                                                                     ` [Cluster-devel] [PATCH 43/48] [GFS2] Streamline quota lock/check for no-quota case swhiteho
2008-04-17  8:39                                                                                       ` [Cluster-devel] [PATCH 44/48] [GFS2] Faster gfs2_bitfit algorithm swhiteho
2008-04-17  8:39                                                                                         ` [Cluster-devel] [PATCH 45/48] [GFS2] fs/gfs2/recovery.c: suppress warnings swhiteho
2008-04-17  8:39                                                                                           ` swhiteho [this message]
2008-04-17  8:39                                                                                             ` [Cluster-devel] [PATCH 47/48] [GFS2] test for IS_ERR rather than 0 swhiteho
2008-04-17  8:39                                                                                               ` [Cluster-devel] [PATCH 48/48] [GFS2] fix GFP_KERNEL misuses swhiteho
2008-04-17 11:58                                                                         ` [Cluster-devel] Re: [PATCH 36/48] [GFS2] fix file_system_type leak on gfs2meta mount Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=12084216641014-git-send-email-swhiteho@redhat.com \
    --to=swhiteho@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).