Commit 96b1454f authored by Bob Peterson's avatar Bob Peterson Committed by Andreas Gruenbacher
Browse files

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: default avatarBob Peterson <rpeterso@redhat.com>
Signed-off-by: default avatarAndreas Gruenbacher <agruenba@redhat.com>
parent c77b52c0

fs/gfs2/ops_fstype.c

+17 −14
Original line number Diff line number Diff line
@@ -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) {

			fs_err(sdp, "can't make FS RO: %d\n", error);

	if (error)

		goto fail_per_node;

		}

		gfs2_freeze_unlock(&freeze_gh);

	} else {



	if (!sb_rdonly(sb))

		error = gfs2_make_fs_rw(sdp);



	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;

fs/gfs2/super.c

+0 −23
Original line number Diff line number Diff line
@@ -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 @@ static void gfs2_dirty_inode(struct inode *inode, int flags) 


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)

fs/gfs2/util.c

+16 −2
Original line number Diff line number Diff line
@@ -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))

	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)