gfs2: Retries missing in gfs2_{rename,exchange}

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 <agruenba@redhat.com>

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 47e8809575d5007fb1b512088376f9b011f69da1..f358448d7b3d7efe59a9122931faad8467654fd6 100644 (file)
--- 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 @@ out:
                }
        }
        return ret;
+
+interrupted:
+       ret = -EINTR;
+       goto out;
 }
 
 /**

diff --git a/fs/gfs2/glock.h b/fs/gfs2/glock.h
index 55d5985f32a0808bbbbb6eb3322ea6e9f6e9f433..dccbf36b8cb108ffeb39b496dbdcafbc2a81f789 100644 (file)
--- 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 36618e35319965bd9aa0c4b4a3387a096de0b187..b6ed069b348721ab8fd5eda377c386535010e8ad 100644 (file)
--- 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;