gfs2: Add proper lockspace locking

GFS2 has been calling functions like dlm_lock() even after the lockspace
that these functions operate on has been released with
dlm_release_lockspace().  It has always assumed that those functions
would return -EINVAL in that case, but that was never guaranteed, and it
certainly is no longer the case since commit 4db41bf4f04f ("dlm: remove
ls_local_handle from struct dlm_ls").

To fix that, add proper lockspace locking.

Fixes: 3e11e5304150 ("GFS2: ignore unlock failures after withdraw")
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: Andrew Price <anprice@redhat.com>

diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index 72d95185a39f6192267bd809959bb91bfbc88c2f..bc67fa058c845997588c3db660e9bcd4be3561bf 100644 (file)
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -1442,6 +1442,7 @@ static int gfs2_lock(struct file *file, int cmd, struct file_lock *fl)
        struct gfs2_inode *ip = GFS2_I(file->f_mapping->host);
        struct gfs2_sbd *sdp = GFS2_SB(file->f_mapping->host);
        struct lm_lockstruct *ls = &sdp->sd_lockstruct;
+       int ret;
 
        if (!(fl->c.flc_flags & FL_POSIX))
                return -ENOLCK;
@@ -1450,14 +1451,20 @@ static int gfs2_lock(struct file *file, int cmd, struct file_lock *fl)
                        locks_lock_file_wait(file, fl);
                return -EIO;
        }
-       if (cmd == F_CANCELLK)
-               return dlm_posix_cancel(ls->ls_dlm, ip->i_no_addr, file, fl);
-       else if (IS_GETLK(cmd))
-               return dlm_posix_get(ls->ls_dlm, ip->i_no_addr, file, fl);
-       else if (lock_is_unlock(fl))
-               return dlm_posix_unlock(ls->ls_dlm, ip->i_no_addr, file, fl);
-       else
-               return dlm_posix_lock(ls->ls_dlm, ip->i_no_addr, file, cmd, fl);
+       down_read(&ls->ls_sem);
+       ret = -ENODEV;
+       if (likely(ls->ls_dlm != NULL)) {
+               if (cmd == F_CANCELLK)
+                       ret = dlm_posix_cancel(ls->ls_dlm, ip->i_no_addr, file, fl);
+               else if (IS_GETLK(cmd))
+                       ret = dlm_posix_get(ls->ls_dlm, ip->i_no_addr, file, fl);
+               else if (lock_is_unlock(fl))
+                       ret = dlm_posix_unlock(ls->ls_dlm, ip->i_no_addr, file, fl);
+               else
+                       ret = dlm_posix_lock(ls->ls_dlm, ip->i_no_addr, file, cmd, fl);
+       }
+       up_read(&ls->ls_sem);
+       return ret;
 }
 
 static void __flock_holder_uninit(struct file *file, struct gfs2_holder *fl_gh)

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 747ee7ca44e2fe38961c42f1687f919433b48730..b677c0e6b9ab3092c87017f9e60173d03fe333df 100644 (file)
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -795,9 +795,8 @@ skip_inval:
                }
                clear_bit(GLF_PENDING_REPLY, &gl->gl_flags);
 
-               if (ret == -EINVAL && gl->gl_target == LM_ST_UNLOCKED &&
-                   target == LM_ST_UNLOCKED &&
-                   test_bit(DFL_UNMOUNT, &ls->ls_recover_flags)) {
+               if (ret == -ENODEV && gl->gl_target == LM_ST_UNLOCKED &&
+                   target == LM_ST_UNLOCKED) {
                        /*
                         * The lockspace has been released and the lock has
                         * been unlocked implicitly.

diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index cd8a3f469291ffcff612da84b105e1ba48496efa..5a0ea416cfdae9c64952d4ef9d0b036ef5a03b20 100644 (file)
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -656,6 +656,8 @@ struct lm_lockstruct {
        struct completion ls_sync_wait; /* {control,mounted}_{lock,unlock} */
        char *ls_lvb_bits;
 
+       struct rw_semaphore ls_sem;
+
        spinlock_t ls_recover_spin; /* protects following fields */
        unsigned long ls_recover_flags; /* DFL_ */
        uint32_t ls_recover_mount; /* gen in first recover_done cb */

diff --git a/fs/gfs2/lock_dlm.c b/fs/gfs2/lock_dlm.c
index 427150a206cfe37f2b21c8f8a5eadaa91c0e52bd..3c7db20ce5642902f9be38a289b330ef5fb9b98a 100644 (file)
--- a/fs/gfs2/lock_dlm.c
+++ b/fs/gfs2/lock_dlm.c
@@ -312,8 +312,13 @@ static int gdlm_lock(struct gfs2_glock *gl, unsigned int req_state,
         */
 
 again:
-       error = dlm_lock(ls->ls_dlm, req, &gl->gl_lksb, lkf, strname,
-                       GDLM_STRNAME_BYTES - 1, 0, gdlm_ast, gl, gdlm_bast);
+       down_read(&ls->ls_sem);
+       error = -ENODEV;
+       if (likely(ls->ls_dlm != NULL)) {
+               error = dlm_lock(ls->ls_dlm, req, &gl->gl_lksb, lkf, strname,
+                               GDLM_STRNAME_BYTES - 1, 0, gdlm_ast, gl, gdlm_bast);
+       }
+       up_read(&ls->ls_sem);
        if (error == -EBUSY) {
                msleep(20);
                goto again;
@@ -362,8 +367,13 @@ static void gdlm_put_lock(struct gfs2_glock *gl)
                flags |= DLM_LKF_VALBLK;
 
 again:
-       error = dlm_unlock(ls->ls_dlm, gl->gl_lksb.sb_lkid, flags,
-                          NULL, gl);
+       down_read(&ls->ls_sem);
+       error = -ENODEV;
+       if (likely(ls->ls_dlm != NULL)) {
+               error = dlm_unlock(ls->ls_dlm, gl->gl_lksb.sb_lkid, flags,
+                                  NULL, gl);
+       }
+       up_read(&ls->ls_sem);
        if (error == -EBUSY) {
                msleep(20);
                goto again;
@@ -379,7 +389,12 @@ again:
 static void gdlm_cancel(struct gfs2_glock *gl)
 {
        struct lm_lockstruct *ls = &gl->gl_name.ln_sbd->sd_lockstruct;
-       dlm_unlock(ls->ls_dlm, gl->gl_lksb.sb_lkid, DLM_LKF_CANCEL, NULL, gl);
+
+       down_read(&ls->ls_sem);
+       if (likely(ls->ls_dlm != NULL)) {
+               dlm_unlock(ls->ls_dlm, gl->gl_lksb.sb_lkid, DLM_LKF_CANCEL, NULL, gl);
+       }
+       up_read(&ls->ls_sem);
 }
 
 /*
@@ -560,7 +575,11 @@ static int sync_unlock(struct gfs2_sbd *sdp, struct dlm_lksb *lksb, char *name)
        struct lm_lockstruct *ls = &sdp->sd_lockstruct;
        int error;
 
-       error = dlm_unlock(ls->ls_dlm, lksb->sb_lkid, 0, lksb, ls);
+       down_read(&ls->ls_sem);
+       error = -ENODEV;
+       if (likely(ls->ls_dlm != NULL))
+               error = dlm_unlock(ls->ls_dlm, lksb->sb_lkid, 0, lksb, ls);
+       up_read(&ls->ls_sem);
        if (error) {
                fs_err(sdp, "%s lkid %x error %d\n",
                       name, lksb->sb_lkid, error);
@@ -587,9 +606,14 @@ static int sync_lock(struct gfs2_sbd *sdp, int mode, uint32_t flags,
        memset(strname, 0, GDLM_STRNAME_BYTES);
        snprintf(strname, GDLM_STRNAME_BYTES, "%8x%16x", LM_TYPE_NONDISK, num);
 
-       error = dlm_lock(ls->ls_dlm, mode, lksb, flags,
-                        strname, GDLM_STRNAME_BYTES - 1,
-                        0, sync_wait_cb, ls, NULL);
+       down_read(&ls->ls_sem);
+       error = -ENODEV;
+       if (likely(ls->ls_dlm != NULL)) {
+               error = dlm_lock(ls->ls_dlm, mode, lksb, flags,
+                                strname, GDLM_STRNAME_BYTES - 1,
+                                0, sync_wait_cb, ls, NULL);
+       }
+       up_read(&ls->ls_sem);
        if (error) {
                fs_err(sdp, "%s lkid %x flags %x mode %d error %d\n",
                       name, lksb->sb_lkid, flags, mode, error);
@@ -1316,6 +1340,7 @@ static int gdlm_mount(struct gfs2_sbd *sdp, const char *table)
         */
 
        INIT_DELAYED_WORK(&sdp->sd_control_work, gfs2_control_func);
+       ls->ls_dlm = NULL;
        spin_lock_init(&ls->ls_recover_spin);
        ls->ls_recover_flags = 0;
        ls->ls_recover_mount = 0;
@@ -1350,6 +1375,7 @@ static int gdlm_mount(struct gfs2_sbd *sdp, const char *table)
         * create/join lockspace
         */
 
+       init_rwsem(&ls->ls_sem);
        error = dlm_new_lockspace(fsname, cluster, flags, GDLM_LVB_SIZE,
                                  &gdlm_lockspace_ops, sdp, &ops_result,
                                  &ls->ls_dlm);
@@ -1429,10 +1455,12 @@ static void gdlm_unmount(struct gfs2_sbd *sdp)
 
        /* mounted_lock and control_lock will be purged in dlm recovery */
 release:
+       down_write(&ls->ls_sem);
        if (ls->ls_dlm) {
                dlm_release_lockspace(ls->ls_dlm, 2);
                ls->ls_dlm = NULL;
        }
+       up_write(&ls->ls_sem);
 
        free_recover_size(ls);
 }