gfs2: move freeze glock outside the make_fs_rw and _ro functions

Before this patch, sister functions gfs2_make_fs_rw and gfs2_make_fs_ro locked
(held) the freeze glock by calling gfs2_freeze_lock and gfs2_freeze_unlock.
The problem is, not all the callers of gfs2_make_fs_ro should be doing this.
The three callers of gfs2_make_fs_ro are: remount (gfs2_reconfigure),
signal_our_withdraw, and unmount (gfs2_put_super). But when unmounting the
file system we can get into the following circular lock dependency:

deactivate_super
   down_write(&s->s_umount); <-------------------------------------- s_umount
   deactivate_locked_super
      gfs2_kill_sb
         kill_block_super
            generic_shutdown_super
               gfs2_put_super
                  gfs2_make_fs_ro
                     gfs2_glock_nq_init sd_freeze_gl
                        freeze_go_sync
                           if (freeze glock in SH)
                              freeze_super (vfs)
                                 down_write(&sb->s_umount); <------- s_umount

This patch moves the hold of the freeze glock outside the two sister rw/ro
functions to their callers, but it doesn't request the glock from
gfs2_put_super, thus eliminating the circular dependency.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>

diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 4ee56f5..f2c6bbe 100644 (file)
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -1084,6 +1084,7 @@ static int gfs2_fill_super(struct super_block *sb, struct fs_context *fc)
        int silent = fc->sb_flags & SB_SILENT;
        struct gfs2_sbd *sdp;
        struct gfs2_holder mount_gh;
+       struct gfs2_holder freeze_gh;
        int error;
 
        sdp = init_sbd(sb);
@@ -1195,23 +1196,18 @@ static int gfs2_fill_super(struct super_block *sb, struct fs_context *fc)
                goto fail_per_node;
        }
 
-       if (sb_rdonly(sb)) {
-               struct gfs2_holder freeze_gh;
+       error = gfs2_freeze_lock(sdp, &freeze_gh, 0);
+       if (error)
+               goto fail_per_node;
 
-               error = gfs2_freeze_lock(sdp, &freeze_gh, 0);
-               if (error) {
-                       fs_err(sdp, "can't make FS RO: %d\n", error);
-                       goto fail_per_node;
-               }
-               gfs2_freeze_unlock(&freeze_gh);
-       } else {
+       if (!sb_rdonly(sb))
                error = gfs2_make_fs_rw(sdp);
-               if (error) {
-                       fs_err(sdp, "can't make FS RW: %d\n", error);
-                       goto fail_per_node;
-               }
-       }
 
+       gfs2_freeze_unlock(&freeze_gh);
+       if (error) {
+               fs_err(sdp, "can't make FS RW: %d\n", error);
+               goto fail_per_node;
+       }
        gfs2_glock_dq_uninit(&mount_gh);
        gfs2_online_uevent(sdp);
        return 0;
@@ -1512,6 +1508,12 @@ static int gfs2_reconfigure(struct fs_context *fc)
                fc->sb_flags |= SB_RDONLY;
 
        if ((sb->s_flags ^ fc->sb_flags) & SB_RDONLY) {
+               struct gfs2_holder freeze_gh;
+
+               error = gfs2_freeze_lock(sdp, &freeze_gh, 0);
+               if (error)
+                       return -EINVAL;
+
                if (fc->sb_flags & SB_RDONLY) {
                        error = gfs2_make_fs_ro(sdp);
                        if (error)
@@ -1521,6 +1523,7 @@ static int gfs2_reconfigure(struct fs_context *fc)
                        if (error)
                                errorfc(fc, "unable to remount read-write");
                }
+               gfs2_freeze_unlock(&freeze_gh);
        }
        sdp->sd_args = *newargs;
 

diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index ea312a9..754ea2a 100644 (file)
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -165,7 +165,6 @@ int gfs2_make_fs_rw(struct gfs2_sbd *sdp)
 {
        struct gfs2_inode *ip = GFS2_I(sdp->sd_jdesc->jd_inode);
        struct gfs2_glock *j_gl = ip->i_gl;
-       struct gfs2_holder freeze_gh;
        struct gfs2_log_header_host head;
        int error;
 
@@ -173,10 +172,6 @@ int gfs2_make_fs_rw(struct gfs2_sbd *sdp)
        if (error)
                return error;
 
-       error = gfs2_freeze_lock(sdp, &freeze_gh, 0);
-       if (error)
-               goto fail_threads;
-
        j_gl->gl_ops->go_inval(j_gl, DIO_METADATA);
        if (gfs2_withdrawn(sdp)) {
                error = -EIO;
@@ -203,13 +198,9 @@ int gfs2_make_fs_rw(struct gfs2_sbd *sdp)
 
        set_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags);
 
-       gfs2_freeze_unlock(&freeze_gh);
-
        return 0;
 
 fail:
-       gfs2_freeze_unlock(&freeze_gh);
-fail_threads:
        if (sdp->sd_quotad_process)
                kthread_stop(sdp->sd_quotad_process);
        sdp->sd_quotad_process = NULL;
@@ -607,21 +598,9 @@ out:
 
 int gfs2_make_fs_ro(struct gfs2_sbd *sdp)
 {
-       struct gfs2_holder freeze_gh;
        int error = 0;
        int log_write_allowed = test_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags);
 
-       gfs2_holder_mark_uninitialized(&freeze_gh);
-       if (sdp->sd_freeze_gl &&
-           !gfs2_glock_is_locked_by_me(sdp->sd_freeze_gl)) {
-               error = gfs2_freeze_lock(sdp, &freeze_gh,
-                                        log_write_allowed ? 0 : LM_FLAG_TRY);
-               if (error == GLR_TRYFAILED)
-                       error = 0;
-               if (error && !gfs2_withdrawn(sdp))
-                       return error;
-       }
-
        gfs2_flush_delete_work(sdp);
        if (!log_write_allowed && current == sdp->sd_quotad_process)
                fs_warn(sdp, "The quotad daemon is withdrawing.\n");
@@ -650,8 +629,6 @@ int gfs2_make_fs_ro(struct gfs2_sbd *sdp)
                                   atomic_read(&sdp->sd_reserving_log) == 0,
                                   HZ * 5);
        }
-       gfs2_freeze_unlock(&freeze_gh);
-
        gfs2_quota_cleanup(sdp);
 
        if (!log_write_allowed)

diff --git a/fs/gfs2/util.c b/fs/gfs2/util.c
index a115c44..97b1bdc 100644 (file)
--- a/fs/gfs2/util.c
+++ b/fs/gfs2/util.c
@@ -122,6 +122,7 @@ static void signal_our_withdraw(struct gfs2_sbd *sdp)
        struct inode *inode = sdp->sd_jdesc->jd_inode;
        struct gfs2_inode *ip = GFS2_I(inode);
        u64 no_formal_ino = ip->i_no_formal_ino;
+       int log_write_allowed = test_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags);
        int ret = 0;
        int tries;
 
@@ -142,8 +143,21 @@ static void signal_our_withdraw(struct gfs2_sbd *sdp)
         * therefore we need to clear SDF_JOURNAL_LIVE manually.
         */
        clear_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags);
-       if (!sb_rdonly(sdp->sd_vfs))
-               ret = gfs2_make_fs_ro(sdp);
+       if (!sb_rdonly(sdp->sd_vfs)) {
+               struct gfs2_holder freeze_gh;
+
+               gfs2_holder_mark_uninitialized(&freeze_gh);
+               if (sdp->sd_freeze_gl &&
+                   !gfs2_glock_is_locked_by_me(sdp->sd_freeze_gl)) {
+                       ret = gfs2_freeze_lock(sdp, &freeze_gh,
+                                      log_write_allowed ? 0 : LM_FLAG_TRY);
+                       if (ret == GLR_TRYFAILED)
+                               ret = 0;
+               }
+               if (!ret)
+                       ret = gfs2_make_fs_ro(sdp);
+               gfs2_freeze_unlock(&freeze_gh);
+       }
 
        if (sdp->sd_lockstruct.ls_ops->lm_lock == NULL) { /* lock_nolock */
                if (!ret)