[PATCH 02/13] gfs2: Retries missing in gfs2_{rename,exchange}

Andreas Gruenbacher <[email protected]>
Newsgroups dev.linux.lists.gfs2
Message-ID <[email protected]>
Fix a bug in gfs2's asynchronous glock handling for rename and exchange
operations.  The original async implementation from commit ad26967b9afa
("gfs2: Use async glocks for rename") mentioned that retries were needed
but never implemented them, causing operations to fail with -ESTALE
instead of retrying on timeout.

Also makes the waiting interruptible.

In addition, the timeouts used were too high for situations in which
timing out is a rare but expected scenario.  Switch to shorter timeouts
with randomization and exponentional backoff.

Fixes: ad26967b9afa ("gfs2: Use async glocks for rename")
Signed-off-by: Andreas Gruenbacher <[email protected]>
---
 fs/gfs2/glock.c | 36 +++++++++++++++++++++++++++---------
 fs/gfs2/glock.h |  3 ++-
 fs/gfs2/inode.c | 18 ++++++++++++++----
 3 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 47e8809575d5..f358448d7b3d 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -1284,31 +1284,45 @@ static int glocks_pending(unsigned int num_gh, struct gfs2_holder *ghs)
  * gfs2_glock_async_wait - wait on multiple asynchronous glock acquisitions
  * @num_gh: the number of holders in the array
  * @ghs: the glock holder array
+ * @retries: number of retries attempted so far
  *
  * Returns: 0 on success, meaning all glocks have been granted and are held.
  *          -ESTALE if the request timed out, meaning all glocks were released,
  *          and the caller should retry the operation.
  */
 
-int gfs2_glock_async_wait(unsigned int num_gh, struct gfs2_holder *ghs)
+int gfs2_glock_async_wait(unsigned int num_gh, struct gfs2_holder *ghs,
+			  unsigned int retries)
 {
 	struct gfs2_sbd *sdp = ghs[0].gh_gl->gl_name.ln_sbd;
-	int i, ret = 0, timeout = 0;
 	unsigned long start_time = jiffies;
+	int i, ret = 0;
+	long timeout;
 
 	might_sleep();
-	/*
-	 * Total up the (minimum hold time * 2) of all glocks and use that to
-	 * determine the max amount of time we should wait.
-	 */
-	for (i = 0; i < num_gh; i++)
-		timeout += ghs[i].gh_gl->gl_hold_time << 1;
 
-	if (!wait_event_timeout(sdp->sd_async_glock_wait,
+	timeout = GL_GLOCK_MIN_HOLD;
+	if (retries) {
+		unsigned int max_shift;
+		long incr;
+
+		/* Add a random delay and increase the timeout exponentially. */
+		max_shift = BITS_PER_LONG - 2 - __fls(GL_GLOCK_HOLD_INCR);
+		incr = min(GL_GLOCK_HOLD_INCR << min(retries - 1, max_shift),
+			   10 * HZ - GL_GLOCK_MIN_HOLD);
+		schedule_timeout_interruptible(get_random_long() % (incr / 3));
+		if (signal_pending(current))
+			goto interrupted;
+		timeout += (incr / 3) + get_random_long() % (incr / 3);
+	}
+
+	if (!wait_event_interruptible_timeout(sdp->sd_async_glock_wait,
 				!glocks_pending(num_gh, ghs), timeout)) {
 		ret = -ESTALE; /* request timed out. */
 		goto out;
 	}
+	if (signal_pending(current))
+		goto interrupted;
 
 	for (i = 0; i < num_gh; i++) {
 		struct gfs2_holder *gh = &ghs[i];
@@ -1332,6 +1346,10 @@ int gfs2_glock_async_wait(unsigned int num_gh, struct gfs2_holder *ghs)
 		}
 	}
 	return ret;
+
+interrupted:
+	ret = -EINTR;
+	goto out;
 }
 
 /**
diff --git a/fs/gfs2/glock.h b/fs/gfs2/glock.h
index 55d5985f32a0..dccbf36b8cb1 100644
--- a/fs/gfs2/glock.h
+++ b/fs/gfs2/glock.h
@@ -204,7 +204,8 @@ int gfs2_glock_poll(struct gfs2_holder *gh);
 int gfs2_instantiate(struct gfs2_holder *gh);
 int gfs2_glock_holder_ready(struct gfs2_holder *gh);
 int gfs2_glock_wait(struct gfs2_holder *gh);
-int gfs2_glock_async_wait(unsigned int num_gh, struct gfs2_holder *ghs);
+int gfs2_glock_async_wait(unsigned int num_gh, struct gfs2_holder *ghs,
+			  unsigned int retries);
 void gfs2_glock_dq(struct gfs2_holder *gh);
 void gfs2_glock_dq_wait(struct gfs2_holder *gh);
 void gfs2_glock_dq_uninit(struct gfs2_holder *gh);
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 36618e353199..b6ed069b3487 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -1495,7 +1495,7 @@ static int gfs2_rename(struct inode *odir, struct dentry *odentry,
 	unsigned int num_gh;
 	int dir_rename = 0;
 	struct gfs2_diradd da = { .nr_blocks = 0, .save_loc = 0, };
-	unsigned int x;
+	unsigned int retries = 0, x;
 	int error;
 
 	gfs2_holder_mark_uninitialized(&r_gh);
@@ -1545,12 +1545,17 @@ static int gfs2_rename(struct inode *odir, struct dentry *odentry,
 		num_gh++;
 	}
 
+again:
 	for (x = 0; x < num_gh; x++) {
 		error = gfs2_glock_nq(ghs + x);
 		if (error)
 			goto out_gunlock;
 	}
-	error = gfs2_glock_async_wait(num_gh, ghs);
+	error = gfs2_glock_async_wait(num_gh, ghs, retries);
+	if (error == -ESTALE) {
+		retries++;
+		goto again;
+	}
 	if (error)
 		goto out_gunlock;
 
@@ -1739,7 +1744,7 @@ static int gfs2_exchange(struct inode *odir, struct dentry *odentry,
 	struct gfs2_sbd *sdp = GFS2_SB(odir);
 	struct gfs2_holder ghs[4], r_gh;
 	unsigned int num_gh;
-	unsigned int x;
+	unsigned int retries = 0, x;
 	umode_t old_mode = oip->i_inode.i_mode;
 	umode_t new_mode = nip->i_inode.i_mode;
 	int error;
@@ -1783,13 +1788,18 @@ static int gfs2_exchange(struct inode *odir, struct dentry *odentry,
 	gfs2_holder_init(nip->i_gl, LM_ST_EXCLUSIVE, GL_ASYNC, ghs + num_gh);
 	num_gh++;
 
+again:
 	for (x = 0; x < num_gh; x++) {
 		error = gfs2_glock_nq(ghs + x);
 		if (error)
 			goto out_gunlock;
 	}
 
-	error = gfs2_glock_async_wait(num_gh, ghs);
+	error = gfs2_glock_async_wait(num_gh, ghs, retries);
+	if (error == -ESTALE) {
+		retries++;
+		goto again;
+	}
 	if (error)
 		goto out_gunlock;
 
-- 
2.52.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.