[3/3] gfs2: Shut down frozen filesystem on last unmount

Commit Message

Andreas Gruenbacher Nov. 29, 2022, 11:07 p.m. UTC

So far, when a frozen filesystem is last unmouted, it turns into a
zombie rather than being shut down; to shut it down, it needs to be
remounted and thawed first.

That's silly for local filesystems, but it's worse for filesystems like
gfs2 which freeze a filesystem cluster-wide when fsfreeze is called on
one of the nodes.  Only the node that initiated a freeze is allowed to
thaw the filesystem it again.  On the other nodes, the only way to shut
down the remotely frozen filesystem is to power off.

Change that on gfs2 so that frozen filesystems are immediately shut down
when they are last unmounted and removed from the filesystem namespace.
This doesn't require writing to the filesystem, so the remaining cluster
nodes remain undisturbed.

Signed-off-by: Andreas Gruenbacher <[e-mail]>
---
 fs/gfs2/glops.c | 17 +++--------------
 fs/gfs2/super.c | 27 +++++++++++++++++++++------
 2 files changed, 24 insertions(+), 20 deletions(-)

Comments

Al Viro Jan. 12, 2023, 7:07 p.m. UTC | #1

On Wed, Nov 30, 2022 at 12:07:35AM +0100, Andreas Gruenbacher wrote:
> So far, when a frozen filesystem is last unmouted, it turns into a
> zombie rather than being shut down; to shut it down, it needs to be
> remounted and thawed first.
> 
> That's silly for local filesystems, but it's worse for filesystems like
> gfs2 which freeze a filesystem cluster-wide when fsfreeze is called on
> one of the nodes.  Only the node that initiated a freeze is allowed to
> thaw the filesystem it again.  On the other nodes, the only way to shut
> down the remotely frozen filesystem is to power off.
> 
> Change that on gfs2 so that frozen filesystems are immediately shut down
> when they are last unmounted and removed from the filesystem namespace.
> This doesn't require writing to the filesystem, so the remaining cluster
> nodes remain undisturbed.

	So what are your preferred rules wrt active references vs. bdev
freeze depth vs. actual frozen state of fs?  The current situation is
already headache-inducing (and I wouldn't bet on correctness - e.g.
FITHAW vs. XFS_IOC_GOINDOWN is interesting).  And the variety of callbacks
(->thaw_super() vs. ->unfreeze_fs(), etc.) also doesn't help, to put it
mildly...

	Sure, gfs2 has unusual needs in that area, but it would be really
nice to get that thing regularized to something that would work for
all users of that machinery.

Patch

diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
index ceadb2d2d948..d73d1944543f 100644
--- a/fs/gfs2/glops.c
+++ b/fs/gfs2/glops.c
@@ -555,25 +555,13 @@  static void inode_go_dump(struct seq_file *seq, struct gfs2_glock *gl,
 static void freeze_go_callback(struct gfs2_glock *gl, bool remote)
 {
 	struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
-	struct super_block *sb = sdp->sd_vfs;
 
 	if (!remote ||
 	    gl->gl_state != LM_ST_SHARED ||
 	    gl->gl_demote_state != LM_ST_UNLOCKED)
 		return;
 
-	/*
-	 * Try to get an active super block reference to prevent racing with
-	 * unmount (see trylock_super()).  But note that unmount isn't the only
-	 * place where a write lock on s_umount is taken, and we can fail here
-	 * because of things like remount as well.
-	 */
-	if (down_read_trylock(&sb->s_umount)) {
-		atomic_inc(&sb->s_active);
-		up_read(&sb->s_umount);
-		if (!queue_work(gfs2_freeze_wq, &sdp->sd_freeze_work))
-			deactivate_super(sb);
-	}
+	queue_work(gfs2_freeze_wq, &sdp->sd_freeze_work);
 }
 
 /**
@@ -588,7 +576,8 @@  static int freeze_go_xmote_bh(struct gfs2_glock *gl)
 	struct gfs2_log_header_host head;
 	int error;
 
-	if (test_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags)) {
+	if (test_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags) &&
+	    !test_bit(SDF_FROZEN, &sdp->sd_flags)) {
 		j_gl->gl_ops->go_inval(j_gl, DIO_METADATA);
 
 		error = gfs2_find_jhead(sdp->sd_jdesc, &head, false);
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index ce0e6d5e0e47..0c6a9e09b5cb 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -533,7 +533,8 @@  static void gfs2_dirty_inode(struct inode *inode, int flags)
 
 void gfs2_make_fs_ro(struct gfs2_sbd *sdp)
 {
-	int log_write_allowed = test_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags);
+	int log_write_allowed = test_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags) &&
+				!test_bit(SDF_FROZEN, &sdp->sd_flags);
 
 	gfs2_flush_delete_work(sdp);
 	if (!log_write_allowed && current == sdp->sd_quotad_process)
@@ -606,6 +607,7 @@  static void gfs2_put_super(struct super_block *sb)
 
 	/*  Release stuff  */
 
+	flush_work(&sdp->sd_freeze_work);
 	gfs2_freeze_unlock(&sdp->sd_freeze_gh);
 
 	iput(sdp->sd_jindex);
@@ -667,7 +669,10 @@  static int gfs2_freeze_locally(struct gfs2_sbd *sdp)
 	struct super_block *sb = sdp->sd_vfs;
 	int error;
 
-	error = freeze_super(sb);
+	if (!activate_super(sb))
+		return -EBUSY;
+	error = freeze_active_super(sb);
+	deactivate_super(sb);
 	if (error)
 		return error;
 
@@ -685,10 +690,22 @@  static int gfs2_enforce_thaw(struct gfs2_sbd *sdp)
 	struct super_block *sb = sdp->sd_vfs;
 	int error;
 
-	error = gfs2_freeze_lock_shared(sdp, 0);
+	error = gfs2_freeze_lock_shared(sdp, GL_ASYNC);
 	if (error)
 		goto fail;
-	error = thaw_super(sb);
+wait_longer:
+	error = gfs2_glock_async_wait(1, &sdp->sd_freeze_gh, 5 * HZ);
+	if (error && error != -ESTALE)
+		goto fail;
+	if (!activate_super(sb))
+		return -EBUSY;
+	if (error != 0) {
+		/* We don't have the lock, yet. */
+		deactivate_super(sb);
+		goto wait_longer;
+	}
+	error = thaw_active_super(sb);
+	deactivate_super(sb);
 	if (!error)
 		return 0;
 
@@ -701,7 +718,6 @@  static int gfs2_enforce_thaw(struct gfs2_sbd *sdp)
 void gfs2_freeze_func(struct work_struct *work)
 {
 	struct gfs2_sbd *sdp = container_of(work, struct gfs2_sbd, sd_freeze_work);
-	struct super_block *sb = sdp->sd_vfs;
 	int error;
 
 	mutex_lock(&sdp->sd_freeze_mutex);
@@ -724,7 +740,6 @@  void gfs2_freeze_func(struct work_struct *work)
 
 out_unlock:
 	mutex_unlock(&sdp->sd_freeze_mutex);
-	deactivate_super(sb);
 out:
 	if (error)
 		fs_info(sdp, "GFS2: couldn't freeze filesystem: %d\n", error);