[Cluster-devel] [GFS2 PATCH 00/15] gfs2: misc. patch collection (V2)

[Cluster-devel] [GFS2 PATCH 00/15] gfs2: misc. patch collection (V2)

[Bob Peterson]

This is version2 of a set of misc. patches from my collection.
As before, they can be added individually or as a set. Changes from V1:

1. I added a wrapper patch Andreas wrote. I'm not sure how serious he is
   about this one.
2. This set omits the patch "New log flush watchdog" due to Steve
   Whitehouse's objections.
3. New patches were added to allow several more xfstests to run.
4. A new patch fixes a recovery error more understandable.
5. Patch "gfs2: fix deadlock in gfs2_ail1_empty withdraw" has been
   reworked somewhat because of problems discovered by HCH.
6. A new patch was added to reduce code redundancy in gfs2_trans_add_*.
7. Two new patches from Andreas were added that allow xfstests
   generic/079 to run on gfs2.

Most of these patches are very safe and well-tested.
I left out some of my more experimental patches.

Andreas Gruenbacher (3):
  gfs2: Add wrapper for iomap_file_buffered_write
  fs: Move notify_change permission checks into may_setattr
  gfs2: Switch to may_setattr in gfs2_setattr

Bob Peterson (12):
  gfs2: Fix glock recursion in freeze_go_xmote_bh
  gfs2: Eliminate go_xmote_bh in favor of go_lock
  gfs2: be more verbose replaying invalid rgrp blocks
  gfs2: trivial clean up of gfs2_ail_error
  gfs2: tiny cleanup in gfs2_log_reserve
  gfs2: init system threads before freeze lock
  gfs2: Don't release and reacquire local statfs bh
  gfs2: fix deadlock in gfs2_ail1_empty withdraw
  gfs2: replace sd_aspace with sd_inode
  gfs2: reduce redundant code in gfs2_trans_add_*
  gfs2: Make recovery error more readable
  gfs2: ignore usr|grp|prjquota mount options

 fs/attr.c            |  50 ++++++++++++--------
 fs/gfs2/aops.c       |   9 +---
 fs/gfs2/file.c       |  20 +++++---
 fs/gfs2/glock.c      |  12 +----
 fs/gfs2/glops.c      |  43 +++++++++--------
 fs/gfs2/incore.h     |   9 +++-
 fs/gfs2/inode.c      |   4 +-
 fs/gfs2/log.c        |  18 +++++---
 fs/gfs2/lops.c       |  44 ++++++++++++------
 fs/gfs2/meta_io.c    |   2 +-
 fs/gfs2/meta_io.h    |   2 -
 fs/gfs2/ops_fstype.c |  84 ++++++++++++++++++++++++++++-----
 fs/gfs2/super.c      | 107 ++++++++-----------------------------------
 fs/gfs2/super.h      |   3 +-
 fs/gfs2/trans.c      |  46 ++++++++-----------
 fs/gfs2/util.c       |   2 +-
 include/linux/fs.h   |   2 +
 17 files changed, 238 insertions(+), 219 deletions(-)

-- 
2.31.1

[Cluster-devel] [GFS2 PATCH 01/15] gfs2: Add wrapper for iomap_file_buffered_write

[Bob Peterson]

From: Andreas Gruenbacher <agruenba@redhat.com>

Add a wrapper around iomap_file_buffered_write.  We'll add code for when
the operation needs to be retried here later.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/file.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index 84ec053d43b4..55ec1cadc9e6 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -876,6 +876,18 @@ static ssize_t gfs2_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
 	return written ? written : ret;
 }
 
+static ssize_t gfs2_file_buffered_write(struct kiocb *iocb, struct iov_iter *from)
+{
+	struct file *file = iocb->ki_filp;
+	struct inode *inode = file_inode(file);
+	ssize_t ret;
+
+	current->backing_dev_info = inode_to_bdi(inode);
+	ret = iomap_file_buffered_write(iocb, from, &gfs2_iomap_ops);
+	current->backing_dev_info = NULL;
+	return ret;
+}
+
 /**
  * gfs2_file_write_iter - Perform a write to a file
  * @iocb: The io context
@@ -927,9 +939,7 @@ static ssize_t gfs2_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 			goto out_unlock;
 
 		iocb->ki_flags |= IOCB_DSYNC;
-		current->backing_dev_info = inode_to_bdi(inode);
-		buffered = iomap_file_buffered_write(iocb, from, &gfs2_iomap_ops);
-		current->backing_dev_info = NULL;
+		buffered = gfs2_file_buffered_write(iocb, from);
 		if (unlikely(buffered <= 0)) {
 			if (!ret)
 				ret = buffered;
@@ -951,9 +961,7 @@ static ssize_t gfs2_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 		if (!ret || ret2 > 0)
 			ret += ret2;
 	} else {
-		current->backing_dev_info = inode_to_bdi(inode);
-		ret = iomap_file_buffered_write(iocb, from, &gfs2_iomap_ops);
-		current->backing_dev_info = NULL;
+		ret = gfs2_file_buffered_write(iocb, from);
 		if (likely(ret > 0)) {
 			iocb->ki_pos += ret;
 			ret = generic_write_sync(iocb, ret);
-- 
2.31.1

[Cluster-devel] [GFS2 PATCH 02/15] gfs2: Fix glock recursion in freeze_go_xmote_bh

[Bob Peterson]

We must not call gfs2_consist (which does a file system withdraw) from
the freeze glock's freeze_go_xmote_bh function because the withdraw
will try to use the freeze glock, thus causing a glock recursion error.

This patch changes freeze_go_xmote_bh to call function
gfs2_assert_withdraw_delayed instead of gfs2_consist to avoid recursion.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/glops.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
index 54d3fbeb3002..4939308d54f3 100644
--- a/fs/gfs2/glops.c
+++ b/fs/gfs2/glops.c
@@ -604,24 +604,24 @@ static int freeze_go_xmote_bh(struct gfs2_glock *gl)
 	struct gfs2_inode *ip = GFS2_I(sdp->sd_jdesc->jd_inode);
 	struct gfs2_glock *j_gl = ip->i_gl;
 	struct gfs2_log_header_host head;
-	int error;
+	int error = 0;
 
 	if (test_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags)) {
 		j_gl->gl_ops->go_inval(j_gl, DIO_METADATA);
 
 		error = gfs2_find_jhead(sdp->sd_jdesc, &head, false);
-		if (error)
-			gfs2_consist(sdp);
-		if (!(head.lh_flags & GFS2_LOG_HEAD_UNMOUNT))
-			gfs2_consist(sdp);
-
-		/*  Initialize some head of the log stuff  */
-		if (!gfs2_withdrawn(sdp)) {
-			sdp->sd_log_sequence = head.lh_sequence + 1;
-			gfs2_log_pointers_init(sdp, head.lh_blkno);
+		if (gfs2_assert_withdraw_delayed(sdp, !error))
+			goto out;
+		if (gfs2_assert_withdraw_delayed(sdp, head.lh_flags &
+						 GFS2_LOG_HEAD_UNMOUNT)) {
+			error = -EIO;
+			goto out;
 		}
+		sdp->sd_log_sequence = head.lh_sequence + 1;
+		gfs2_log_pointers_init(sdp, head.lh_blkno);
 	}
-	return 0;
+out:
+	return error;
 }
 
 /**
-- 
2.31.1

[Cluster-devel] [GFS2 PATCH 03/15] gfs2: Eliminate go_xmote_bh in favor of go_lock

[Bob Peterson]

Before this patch, the freeze glock was the only glock to use the
go_xmote_bh glock op (glop). The go_xmote_bh glop is done when
a glock is locked. But so is go_lock. This patch eliminates the
glop altogether in favor of just using go_lock for the freeze
glock. This is for better consistency, readability, etc.
I also fixed a misleading comment in do_promote.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/glock.c  | 12 +-----------
 fs/gfs2/glops.c  |  7 ++++---
 fs/gfs2/incore.h |  1 -
 3 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 1f3902ecdded..9f94d6b13363 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -384,7 +384,7 @@ static void do_error(struct gfs2_glock *gl, const int ret)
  * do_promote - promote as many requests as possible on the current queue
  * @gl: The glock
  * 
- * Returns: 1 if there is a blocked holder at the head of the list, or 2
+ * Returns: 1 if there is a blocked waiter at the head of the list, or 2
  *          if a type specific operation is underway.
  */
 
@@ -504,7 +504,6 @@ static void gfs2_demote_wake(struct gfs2_glock *gl)
 
 static void finish_xmote(struct gfs2_glock *gl, unsigned int ret)
 {
-	const struct gfs2_glock_operations *glops = gl->gl_ops;
 	struct gfs2_holder *gh;
 	unsigned state = ret & LM_OUT_ST_MASK;
 	int rv;
@@ -562,15 +561,6 @@ static void finish_xmote(struct gfs2_glock *gl, unsigned int ret)
 	if (test_and_clear_bit(GLF_DEMOTE_IN_PROGRESS, &gl->gl_flags))
 		gfs2_demote_wake(gl);
 	if (state != LM_ST_UNLOCKED) {
-		if (glops->go_xmote_bh) {
-			spin_unlock(&gl->gl_lockref.lock);
-			rv = glops->go_xmote_bh(gl);
-			spin_lock(&gl->gl_lockref.lock);
-			if (rv) {
-				do_error(gl, rv);
-				goto out;
-			}
-		}
 		rv = do_promote(gl);
 		if (rv == 2)
 			goto out_locked;
diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
index 4939308d54f3..9f3b68806376 100644
--- a/fs/gfs2/glops.c
+++ b/fs/gfs2/glops.c
@@ -595,11 +595,12 @@ static int freeze_go_sync(struct gfs2_glock *gl)
 }
 
 /**
- * freeze_go_xmote_bh - After promoting/demoting the freeze glock
+ * freeze_go_lock - After promoting/demoting the freeze glock
  * @gl: the glock
  */
-static int freeze_go_xmote_bh(struct gfs2_glock *gl)
+static int freeze_go_lock(struct gfs2_holder *gh)
 {
+	struct gfs2_glock *gl = gh->gh_gl;
 	struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
 	struct gfs2_inode *ip = GFS2_I(sdp->sd_jdesc->jd_inode);
 	struct gfs2_glock *j_gl = ip->i_gl;
@@ -759,7 +760,7 @@ const struct gfs2_glock_operations gfs2_rgrp_glops = {
 
 const struct gfs2_glock_operations gfs2_freeze_glops = {
 	.go_sync = freeze_go_sync,
-	.go_xmote_bh = freeze_go_xmote_bh,
+	.go_lock = freeze_go_lock,
 	.go_demote_ok = freeze_go_demote_ok,
 	.go_type = LM_TYPE_NONDISK,
 	.go_flags = GLOF_NONDISK,
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index e6f820f146cb..d5ea955d6a87 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -217,7 +217,6 @@ struct lm_lockname {
 
 struct gfs2_glock_operations {
 	int (*go_sync) (struct gfs2_glock *gl);
-	int (*go_xmote_bh)(struct gfs2_glock *gl);
 	void (*go_inval) (struct gfs2_glock *gl, int flags);
 	int (*go_demote_ok) (const struct gfs2_glock *gl);
 	int (*go_lock) (struct gfs2_holder *gh);
-- 
2.31.1

[Cluster-devel] [GFS2 PATCH 04/15] gfs2: be more verbose replaying invalid rgrp blocks

[Bob Peterson]

This patch adds some crucial information when journal replay detects a
replay of an obsolete rgrp block. For example, it wasn't printing the
journal id or the generation number played. This just supplements what
is logged in this unusual case.

The function that actually complains about the replaying of an obsolete
rgrp block has been split off to avoid long lines and sparse warnings.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/lops.c | 44 +++++++++++++++++++++++++++++---------------
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
index 8ee05d25dfa6..ca0bb3a73912 100644
--- a/fs/gfs2/lops.c
+++ b/fs/gfs2/lops.c
@@ -761,6 +761,32 @@ static void buf_lo_before_scan(struct gfs2_jdesc *jd,
 	jd->jd_replayed_blocks = 0;
 }
 
+#define obsolete_rgrp_replay \
+"Replaying 0x%llx from jid=%d/0x%llx but we already have a bh!\n"
+#define obsolete_rgrp_replay2 \
+"busy:%d, pinned:%d rg_gen:0x%llx, j_gen:0x%llx\n"
+
+static void obsolete_rgrp(struct gfs2_jdesc *jd, struct buffer_head *bh_log,
+			  u64 blkno)
+{
+	struct gfs2_sbd *sdp = GFS2_SB(jd->jd_inode);
+	struct gfs2_rgrpd *rgd;
+	struct gfs2_rgrp *jrgd = (struct gfs2_rgrp *)bh_log->b_data;
+
+	rgd = gfs2_blk2rgrpd(sdp, blkno, false);
+	if (rgd && rgd->rd_addr == blkno &&
+	    rgd->rd_bits && rgd->rd_bits->bi_bh) {
+		fs_info(sdp, obsolete_rgrp_replay, (unsigned long long)blkno,
+			jd->jd_jid, bh_log->b_blocknr);
+		fs_info(sdp, obsolete_rgrp_replay2,
+			buffer_busy(rgd->rd_bits->bi_bh) ? 1 : 0,
+			buffer_pinned(rgd->rd_bits->bi_bh),
+			rgd->rd_igeneration,
+			be64_to_cpu(jrgd->rg_igeneration));
+		gfs2_dump_glock(NULL, rgd->rd_gl, true);
+	}
+}
+
 static int buf_lo_scan_elements(struct gfs2_jdesc *jd, u32 start,
 				struct gfs2_log_descriptor *ld, __be64 *ptr,
 				int pass)
@@ -799,21 +825,9 @@ static int buf_lo_scan_elements(struct gfs2_jdesc *jd, u32 start,
 			struct gfs2_meta_header *mh =
 				(struct gfs2_meta_header *)bh_ip->b_data;
 
-			if (mh->mh_type == cpu_to_be32(GFS2_METATYPE_RG)) {
-				struct gfs2_rgrpd *rgd;
-
-				rgd = gfs2_blk2rgrpd(sdp, blkno, false);
-				if (rgd && rgd->rd_addr == blkno &&
-				    rgd->rd_bits && rgd->rd_bits->bi_bh) {
-					fs_info(sdp, "Replaying 0x%llx but we "
-						"already have a bh!\n",
-						(unsigned long long)blkno);
-					fs_info(sdp, "busy:%d, pinned:%d\n",
-						buffer_busy(rgd->rd_bits->bi_bh) ? 1 : 0,
-						buffer_pinned(rgd->rd_bits->bi_bh));
-					gfs2_dump_glock(NULL, rgd->rd_gl, true);
-				}
-			}
+			if (mh->mh_type == cpu_to_be32(GFS2_METATYPE_RG))
+				obsolete_rgrp(jd, bh_log, blkno);
+
 			mark_buffer_dirty(bh_ip);
 		}
 		brelse(bh_log);
-- 
2.31.1

[Cluster-devel] [GFS2 PATCH 05/15] gfs2: trivial clean up of gfs2_ail_error

[Bob Peterson]

This patch does not change function. It adds variable sdp to clean up
function gfs2_ail_error and make it more readable.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/glops.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
index 9f3b68806376..744cacd27213 100644
--- a/fs/gfs2/glops.c
+++ b/fs/gfs2/glops.c
@@ -33,16 +33,18 @@ extern struct workqueue_struct *gfs2_control_wq;
 
 static void gfs2_ail_error(struct gfs2_glock *gl, const struct buffer_head *bh)
 {
-	fs_err(gl->gl_name.ln_sbd,
+	struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
+
+	fs_err(sdp,
 	       "AIL buffer %p: blocknr %llu state 0x%08lx mapping %p page "
 	       "state 0x%lx\n",
 	       bh, (unsigned long long)bh->b_blocknr, bh->b_state,
 	       bh->b_page->mapping, bh->b_page->flags);
-	fs_err(gl->gl_name.ln_sbd, "AIL glock %u:%llu mapping %p\n",
+	fs_err(sdp, "AIL glock %u:%llu mapping %p\n",
 	       gl->gl_name.ln_type, gl->gl_name.ln_number,
 	       gfs2_glock2aspace(gl));
-	gfs2_lm(gl->gl_name.ln_sbd, "AIL error\n");
-	gfs2_withdraw(gl->gl_name.ln_sbd);
+	gfs2_lm(sdp, "AIL error\n");
+	gfs2_withdraw(sdp);
 }
 
 /**
-- 
2.31.1

[Cluster-devel] [GFS2 PATCH 06/15] gfs2: tiny cleanup in gfs2_log_reserve

[Bob Peterson]

Function gfs2_log_reserve was setting revoke_blks to 0. There's no
need because it calculates it shortly thereafter. This patch removes
the unnecessary set.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/log.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
index 42c15cfc0821..f0ee3ff6f9a8 100644
--- a/fs/gfs2/log.c
+++ b/fs/gfs2/log.c
@@ -594,7 +594,7 @@ void gfs2_log_reserve(struct gfs2_sbd *sdp, struct gfs2_trans *tr,
 {
 	unsigned int blks = tr->tr_reserved;
 	unsigned int revokes = tr->tr_revokes;
-	unsigned int revoke_blks = 0;
+	unsigned int revoke_blks;
 
 	*extra_revokes = 0;
 	if (revokes) {
-- 
2.31.1

[Cluster-devel] [GFS2 PATCH 07/15] gfs2: init system threads before freeze lock

[Bob Peterson]

Patch 96b1454f2e ("gfs2: move freeze glock outside the make_fs_rw and _ro
functions") changed the gfs2 mount sequence so that it holds the freeze
lock before calling gfs2_make_fs_rw. Before this patch, gfs2_make_fs_rw
called init_threads to initialize the quotad and logd threads. That is a
problem if the system needs to withdraw due to IO errors early in the
mount sequence, for example, while initializing the system statfs inode:

1. An IO error causes the statfs glock to not sync properly after
   recovery, and leaves items on the ail list.
2. The leftover items on the ail list causes its do_xmote call to fail,
   which makes it want to withdraw. But since the glock code cannot
   withdraw (because the withdraw sequence uses glocks) it relies upon
   the logd daemon to initiate the withdraw.`
3. The withdraw can never be performed by the logd daemon because all
   this takes place before the logd daemon is started.

This patch moves function init_threads from super.c to ops_fstype.c
and it changes gfs2_fill_super to start its threads before holding the
freeze lock, and if there's an error, stop its threads after releasing
it. This allows the logd to run unblocked by the freeze lock. Thus,
the logd daemon can perform its withdraw sequence properly.

Fixes: 96b1454f2e ("gfs2: move freeze glock outside the make_fs_rw and _ro
functions")
Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/ops_fstype.c | 42 ++++++++++++++++++++++++++++++
 fs/gfs2/super.c      | 61 +++++---------------------------------------
 2 files changed, 48 insertions(+), 55 deletions(-)

diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index bd3b3be1a473..ca76e3b8792c 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -1089,6 +1089,34 @@ void gfs2_online_uevent(struct gfs2_sbd *sdp)
 	kobject_uevent_env(&sdp->sd_kobj, KOBJ_ONLINE, envp);
 }
 
+static int init_threads(struct gfs2_sbd *sdp)
+{
+	struct task_struct *p;
+	int error = 0;
+
+	p = kthread_run(gfs2_logd, sdp, "gfs2_logd");
+	if (IS_ERR(p)) {
+		error = PTR_ERR(p);
+		fs_err(sdp, "can't start logd thread: %d\n", error);
+		return error;
+	}
+	sdp->sd_logd_process = p;
+
+	p = kthread_run(gfs2_quotad, sdp, "gfs2_quotad");
+	if (IS_ERR(p)) {
+		error = PTR_ERR(p);
+		fs_err(sdp, "can't start quotad thread: %d\n", error);
+		goto fail;
+	}
+	sdp->sd_quotad_process = p;
+	return 0;
+
+fail:
+	kthread_stop(sdp->sd_logd_process);
+	sdp->sd_logd_process = NULL;
+	return error;
+}
+
 /**
  * gfs2_fill_super - Read in superblock
  * @sb: The VFS superblock
@@ -1217,6 +1245,14 @@ static int gfs2_fill_super(struct super_block *sb, struct fs_context *fc)
 		goto fail_per_node;
 	}
 
+	if (!sb_rdonly(sb)) {
+		error = init_threads(sdp);
+		if (error) {
+			gfs2_withdraw_delayed(sdp);
+			goto fail_per_node;
+		}
+	}
+
 	error = gfs2_freeze_lock(sdp, &freeze_gh, 0);
 	if (error)
 		goto fail_per_node;
@@ -1226,6 +1262,12 @@ static int gfs2_fill_super(struct super_block *sb, struct fs_context *fc)
 
 	gfs2_freeze_unlock(&freeze_gh);
 	if (error) {
+		if (sdp->sd_quotad_process)
+			kthread_stop(sdp->sd_quotad_process);
+		sdp->sd_quotad_process = NULL;
+		if (sdp->sd_logd_process)
+			kthread_stop(sdp->sd_logd_process);
+		sdp->sd_logd_process = NULL;
 		fs_err(sdp, "can't make FS RW: %d\n", error);
 		goto fail_per_node;
 	}
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 4d4ceb0b6903..2bdbba5ea8d7 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -119,34 +119,6 @@ int gfs2_jdesc_check(struct gfs2_jdesc *jd)
 	return 0;
 }
 
-static int init_threads(struct gfs2_sbd *sdp)
-{
-	struct task_struct *p;
-	int error = 0;
-
-	p = kthread_run(gfs2_logd, sdp, "gfs2_logd");
-	if (IS_ERR(p)) {
-		error = PTR_ERR(p);
-		fs_err(sdp, "can't start logd thread: %d\n", error);
-		return error;
-	}
-	sdp->sd_logd_process = p;
-
-	p = kthread_run(gfs2_quotad, sdp, "gfs2_quotad");
-	if (IS_ERR(p)) {
-		error = PTR_ERR(p);
-		fs_err(sdp, "can't start quotad thread: %d\n", error);
-		goto fail;
-	}
-	sdp->sd_quotad_process = p;
-	return 0;
-
-fail:
-	kthread_stop(sdp->sd_logd_process);
-	sdp->sd_logd_process = NULL;
-	return error;
-}
-
 /**
  * gfs2_make_fs_rw - Turn a Read-Only FS into a Read-Write one
  * @sdp: the filesystem
@@ -161,26 +133,17 @@ int gfs2_make_fs_rw(struct gfs2_sbd *sdp)
 	struct gfs2_log_header_host head;
 	int error;
 
-	error = init_threads(sdp);
-	if (error) {
-		gfs2_withdraw_delayed(sdp);
-		return error;
-	}
-
 	j_gl->gl_ops->go_inval(j_gl, DIO_METADATA);
-	if (gfs2_withdrawn(sdp)) {
-		error = -EIO;
-		goto fail;
-	}
+	if (gfs2_withdrawn(sdp))
+		return -EIO;
 
 	error = gfs2_find_jhead(sdp->sd_jdesc, &head, false);
 	if (error || gfs2_withdrawn(sdp))
-		goto fail;
+		return error;
 
 	if (!(head.lh_flags & GFS2_LOG_HEAD_UNMOUNT)) {
 		gfs2_consist(sdp);
-		error = -EIO;
-		goto fail;
+		return -EIO;
 	}
 
 	/*  Initialize some head of the log stuff  */
@@ -188,20 +151,8 @@ int gfs2_make_fs_rw(struct gfs2_sbd *sdp)
 	gfs2_log_pointers_init(sdp, head.lh_blkno);
 
 	error = gfs2_quota_init(sdp);
-	if (error || gfs2_withdrawn(sdp))
-		goto fail;
-
-	set_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags);
-
-	return 0;
-
-fail:
-	if (sdp->sd_quotad_process)
-		kthread_stop(sdp->sd_quotad_process);
-	sdp->sd_quotad_process = NULL;
-	if (sdp->sd_logd_process)
-		kthread_stop(sdp->sd_logd_process);
-	sdp->sd_logd_process = NULL;
+	if (!error && !gfs2_withdrawn(sdp))
+		set_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags);
 	return error;
 }
 
-- 
2.31.1

[Cluster-devel] [GFS2 PATCH 08/15] gfs2: Don't release and reacquire local statfs bh

[Bob Peterson]

Before this patch, several functions in gfs2 related to the updating
of the statfs file used a newly acquired/read buffer_head for the
local statfs file. This is completely unnecessary, because other nodes
should never update it. Recreating the buffer is a waste of time.

This patch allows gfs2 to read in the local statefs buffer_head at
mount time and keep it around until unmount time.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/aops.c       |  9 ++-------
 fs/gfs2/incore.h     |  1 +
 fs/gfs2/ops_fstype.c |  9 +++++++++
 fs/gfs2/super.c      | 44 ++++++++++++--------------------------------
 fs/gfs2/super.h      |  3 +--
 5 files changed, 25 insertions(+), 41 deletions(-)

diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 81d8f064126e..005e920f5d4a 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -574,10 +574,9 @@ void adjust_fs_space(struct inode *inode)
 {
 	struct gfs2_sbd *sdp = GFS2_SB(inode);
 	struct gfs2_inode *m_ip = GFS2_I(sdp->sd_statfs_inode);
-	struct gfs2_inode *l_ip = GFS2_I(sdp->sd_sc_inode);
 	struct gfs2_statfs_change_host *m_sc = &sdp->sd_statfs_master;
 	struct gfs2_statfs_change_host *l_sc = &sdp->sd_statfs_local;
-	struct buffer_head *m_bh, *l_bh;
+	struct buffer_head *m_bh;
 	u64 fs_total, new_free;
 
 	if (gfs2_trans_begin(sdp, 2 * RES_STATFS, 0) != 0)
@@ -600,11 +599,7 @@ void adjust_fs_space(struct inode *inode)
 		(unsigned long long)new_free);
 	gfs2_statfs_change(sdp, new_free, new_free, 0);
 
-	if (gfs2_meta_inode_buffer(l_ip, &l_bh) != 0)
-		goto out2;
-	update_statfs(sdp, m_bh, l_bh);
-	brelse(l_bh);
-out2:
+	update_statfs(sdp, m_bh);
 	brelse(m_bh);
 out:
 	sdp->sd_rindex_uptodate = 0;
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index d5ea955d6a87..6f31a067a5f2 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -767,6 +767,7 @@ struct gfs2_sbd {
 	struct gfs2_glock *sd_jinode_gl;
 
 	struct gfs2_holder sd_sc_gh;
+	struct buffer_head *sd_sc_bh;
 	struct gfs2_holder sd_qc_gh;
 
 	struct completion sd_journal_ready;
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index ca76e3b8792c..5b90d2b7db47 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -696,8 +696,16 @@ static int init_statfs(struct gfs2_sbd *sdp)
 		fs_err(sdp, "can't lock local \"sc\" file: %d\n", error);
 		goto free_local;
 	}
+	/* read in the local statfs buffer - other nodes don't change it. */
+	error = gfs2_meta_inode_buffer(ip, &sdp->sd_sc_bh);
+	if (error) {
+		fs_err(sdp, "Cannot read in local statfs: %d\n", error);
+		goto unlock_sd_gh;
+	}
 	return 0;
 
+unlock_sd_gh:
+	gfs2_glock_dq_uninit(&sdp->sd_sc_gh);
 free_local:
 	free_local_statfs_inodes(sdp);
 	iput(pn);
@@ -711,6 +719,7 @@ static int init_statfs(struct gfs2_sbd *sdp)
 static void uninit_statfs(struct gfs2_sbd *sdp)
 {
 	if (!sdp->sd_args.ar_spectator) {
+		brelse(sdp->sd_sc_bh);
 		gfs2_glock_dq_uninit(&sdp->sd_sc_gh);
 		free_local_statfs_inodes(sdp);
 	}
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 2bdbba5ea8d7..0a5b7dfa7a45 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -178,9 +178,8 @@ int gfs2_statfs_init(struct gfs2_sbd *sdp)
 {
 	struct gfs2_inode *m_ip = GFS2_I(sdp->sd_statfs_inode);
 	struct gfs2_statfs_change_host *m_sc = &sdp->sd_statfs_master;
-	struct gfs2_inode *l_ip = GFS2_I(sdp->sd_sc_inode);
 	struct gfs2_statfs_change_host *l_sc = &sdp->sd_statfs_local;
-	struct buffer_head *m_bh, *l_bh;
+	struct buffer_head *m_bh;
 	struct gfs2_holder gh;
 	int error;
 
@@ -199,21 +198,15 @@ int gfs2_statfs_init(struct gfs2_sbd *sdp)
 				      sizeof(struct gfs2_dinode));
 		spin_unlock(&sdp->sd_statfs_spin);
 	} else {
-		error = gfs2_meta_inode_buffer(l_ip, &l_bh);
-		if (error)
-			goto out_m_bh;
-
 		spin_lock(&sdp->sd_statfs_spin);
 		gfs2_statfs_change_in(m_sc, m_bh->b_data +
 				      sizeof(struct gfs2_dinode));
-		gfs2_statfs_change_in(l_sc, l_bh->b_data +
+		gfs2_statfs_change_in(l_sc, sdp->sd_sc_bh->b_data +
 				      sizeof(struct gfs2_dinode));
 		spin_unlock(&sdp->sd_statfs_spin);
 
-		brelse(l_bh);
 	}
 
-out_m_bh:
 	brelse(m_bh);
 out:
 	gfs2_glock_dq_uninit(&gh);
@@ -226,22 +219,17 @@ void gfs2_statfs_change(struct gfs2_sbd *sdp, s64 total, s64 free,
 	struct gfs2_inode *l_ip = GFS2_I(sdp->sd_sc_inode);
 	struct gfs2_statfs_change_host *l_sc = &sdp->sd_statfs_local;
 	struct gfs2_statfs_change_host *m_sc = &sdp->sd_statfs_master;
-	struct buffer_head *l_bh;
 	s64 x, y;
 	int need_sync = 0;
-	int error;
-
-	error = gfs2_meta_inode_buffer(l_ip, &l_bh);
-	if (error)
-		return;
 
-	gfs2_trans_add_meta(l_ip->i_gl, l_bh);
+	gfs2_trans_add_meta(l_ip->i_gl, sdp->sd_sc_bh);
 
 	spin_lock(&sdp->sd_statfs_spin);
 	l_sc->sc_total += total;
 	l_sc->sc_free += free;
 	l_sc->sc_dinodes += dinodes;
-	gfs2_statfs_change_out(l_sc, l_bh->b_data + sizeof(struct gfs2_dinode));
+	gfs2_statfs_change_out(l_sc, sdp->sd_sc_bh->b_data +
+			       sizeof(struct gfs2_dinode));
 	if (sdp->sd_args.ar_statfs_percent) {
 		x = 100 * l_sc->sc_free;
 		y = m_sc->sc_free * sdp->sd_args.ar_statfs_percent;
@@ -250,20 +238,18 @@ void gfs2_statfs_change(struct gfs2_sbd *sdp, s64 total, s64 free,
 	}
 	spin_unlock(&sdp->sd_statfs_spin);
 
-	brelse(l_bh);
 	if (need_sync)
 		gfs2_wake_up_statfs(sdp);
 }
 
-void update_statfs(struct gfs2_sbd *sdp, struct buffer_head *m_bh,
-		   struct buffer_head *l_bh)
+void update_statfs(struct gfs2_sbd *sdp, struct buffer_head *m_bh)
 {
 	struct gfs2_inode *m_ip = GFS2_I(sdp->sd_statfs_inode);
 	struct gfs2_inode *l_ip = GFS2_I(sdp->sd_sc_inode);
 	struct gfs2_statfs_change_host *m_sc = &sdp->sd_statfs_master;
 	struct gfs2_statfs_change_host *l_sc = &sdp->sd_statfs_local;
 
-	gfs2_trans_add_meta(l_ip->i_gl, l_bh);
+	gfs2_trans_add_meta(l_ip->i_gl, sdp->sd_sc_bh);
 	gfs2_trans_add_meta(m_ip->i_gl, m_bh);
 
 	spin_lock(&sdp->sd_statfs_spin);
@@ -271,7 +257,7 @@ void update_statfs(struct gfs2_sbd *sdp, struct buffer_head *m_bh,
 	m_sc->sc_free += l_sc->sc_free;
 	m_sc->sc_dinodes += l_sc->sc_dinodes;
 	memset(l_sc, 0, sizeof(struct gfs2_statfs_change));
-	memset(l_bh->b_data + sizeof(struct gfs2_dinode),
+	memset(sdp->sd_sc_bh->b_data + sizeof(struct gfs2_dinode),
 	       0, sizeof(struct gfs2_statfs_change));
 	gfs2_statfs_change_out(m_sc, m_bh->b_data + sizeof(struct gfs2_dinode));
 	spin_unlock(&sdp->sd_statfs_spin);
@@ -281,11 +267,10 @@ int gfs2_statfs_sync(struct super_block *sb, int type)
 {
 	struct gfs2_sbd *sdp = sb->s_fs_info;
 	struct gfs2_inode *m_ip = GFS2_I(sdp->sd_statfs_inode);
-	struct gfs2_inode *l_ip = GFS2_I(sdp->sd_sc_inode);
 	struct gfs2_statfs_change_host *m_sc = &sdp->sd_statfs_master;
 	struct gfs2_statfs_change_host *l_sc = &sdp->sd_statfs_local;
 	struct gfs2_holder gh;
-	struct buffer_head *m_bh, *l_bh;
+	struct buffer_head *m_bh;
 	int error;
 
 	error = gfs2_glock_nq_init(m_ip->i_gl, LM_ST_EXCLUSIVE, GL_NOCACHE,
@@ -306,21 +291,15 @@ int gfs2_statfs_sync(struct super_block *sb, int type)
 	}
 	spin_unlock(&sdp->sd_statfs_spin);
 
-	error = gfs2_meta_inode_buffer(l_ip, &l_bh);
-	if (error)
-		goto out_bh;
-
 	error = gfs2_trans_begin(sdp, 2 * RES_DINODE, 0);
 	if (error)
-		goto out_bh2;
+		goto out_bh;
 
-	update_statfs(sdp, m_bh, l_bh);
+	update_statfs(sdp, m_bh);
 	sdp->sd_statfs_force_sync = 0;
 
 	gfs2_trans_end(sdp);
 
-out_bh2:
-	brelse(l_bh);
 out_bh:
 	brelse(m_bh);
 out_unlock:
@@ -626,6 +605,7 @@ static void gfs2_put_super(struct super_block *sb)
 			gfs2_glock_dq_uninit(&sdp->sd_journal_gh);
 		if (gfs2_holder_initialized(&sdp->sd_jinode_gh))
 			gfs2_glock_dq_uninit(&sdp->sd_jinode_gh);
+		brelse(sdp->sd_sc_bh);
 		gfs2_glock_dq_uninit(&sdp->sd_sc_gh);
 		gfs2_glock_dq_uninit(&sdp->sd_qc_gh);
 		free_local_statfs_inodes(sdp);
diff --git a/fs/gfs2/super.h b/fs/gfs2/super.h
index ec4affb33ed5..58d13fd77aed 100644
--- a/fs/gfs2/super.h
+++ b/fs/gfs2/super.h
@@ -43,8 +43,7 @@ extern void gfs2_statfs_change_in(struct gfs2_statfs_change_host *sc,
 				  const void *buf);
 extern void gfs2_statfs_change_out(const struct gfs2_statfs_change_host *sc,
 				   void *buf);
-extern void update_statfs(struct gfs2_sbd *sdp, struct buffer_head *m_bh,
-			  struct buffer_head *l_bh);
+extern void update_statfs(struct gfs2_sbd *sdp, struct buffer_head *m_bh);
 extern int gfs2_statfs_sync(struct super_block *sb, int type);
 extern void gfs2_freeze_func(struct work_struct *work);
 
-- 
2.31.1

[Cluster-devel] [GFS2 PATCH 09/15] gfs2: fix deadlock in gfs2_ail1_empty withdraw

[Bob Peterson]

Before this patch, function gfs2_ail1_empty could issue a file system
withdraw when IO errors were discovered. However, there are several
callers, including gfs2_flush_revokes() which holds the gfs2_log_lock
before calling gfs2_ail1_empty. If gfs2_ail1_empty needed to withdraw
it would leave the gfs2_log_lock held, which resulted in a deadlock
due to other processes that needed the log_lock.

Another problem discovered by Christoph Helwig is that we cannot
withdraw from the log_flush process because it may be called from
the glock workqueue, and the withdraw process waits for that very
workqueue to be flushed. So the withdraw must be ignored until it may
be handled by a more appropriate context like the gfs2_logd daemon.

This patch moves the withdraw out of function gfs2_ail1_empty and
makes each of the callers check for a withdraw by calling new function
check_ail1_withdraw. Function gfs2_flush_revokes now does this check
after releasing the gfs2_log_lock to avoid the deadlock.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/log.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
index f0ee3ff6f9a8..c687a8c4e044 100644
--- a/fs/gfs2/log.c
+++ b/fs/gfs2/log.c
@@ -364,11 +364,6 @@ static int gfs2_ail1_empty(struct gfs2_sbd *sdp, int max_revokes)
 	ret = list_empty(&sdp->sd_ail1_list);
 	spin_unlock(&sdp->sd_ail_lock);
 
-	if (test_bit(SDF_WITHDRAWING, &sdp->sd_flags)) {
-		gfs2_lm(sdp, "fatal: I/O error(s)\n");
-		gfs2_withdraw(sdp);
-	}
-
 	return ret;
 }
 
@@ -786,6 +781,15 @@ void gfs2_glock_remove_revoke(struct gfs2_glock *gl)
 	}
 }
 
+static void check_ail1_withdraw(struct gfs2_sbd *sdp)
+{
+	if (!test_bit(SDF_WITHDRAWING, &sdp->sd_flags))
+		return;
+
+	gfs2_lm(sdp, "fatal: I/O error(s)\n");
+	gfs2_withdraw(sdp);
+}
+
 /**
  * gfs2_flush_revokes - Add as many revokes to the system transaction as we can
  * @sdp: The GFS2 superblock
@@ -1317,6 +1321,7 @@ int gfs2_logd(void *data)
 
 		if (gfs2_jrnl_flush_reqd(sdp) || t == 0) {
 			gfs2_ail1_empty(sdp, 0);
+			check_ail1_withdraw(sdp);
 			gfs2_log_flush(sdp, NULL, GFS2_LOG_HEAD_FLUSH_NORMAL |
 						  GFS2_LFC_LOGD_JFLUSH_REQD);
 		}
@@ -1325,6 +1330,7 @@ int gfs2_logd(void *data)
 			gfs2_ail1_start(sdp);
 			gfs2_ail1_wait(sdp);
 			gfs2_ail1_empty(sdp, 0);
+			check_ail1_withdraw(sdp);
 			gfs2_log_flush(sdp, NULL, GFS2_LOG_HEAD_FLUSH_NORMAL |
 						  GFS2_LFC_LOGD_AIL_FLUSH_REQD);
 		}
-- 
2.31.1

[Cluster-devel] [GFS2 PATCH 10/15] gfs2: replace sd_aspace with sd_inode

[Bob Peterson]

Before this patch, gfs2 kept its own address space for rgrps, but this
caused a lockdep problem because vfs assumes a 1:1 relationship between
address spaces and their inode. One problematic area is this:

gfs2_unpin
   mark_buffer_dirty(bh);
      mapping = page_mapping(page);
         __set_page_dirty(page, mapping, memcg, 0);
            xa_lock_irqsave(&mapping->i_pages, flags);
                 ^---locks page->mapping->i_pages
            account_page_dirtied(page, mapping)
	       struct inode *inode = mapping->host;
                 ^---assumes the mapping points to an inode
               inode_to_wb(inode)
                  WARN_ON_ONCE !lockdep_is_held(&inode->i_mapping->
                                                i_pages.xa_lock)

It manifests as a lockdep warning you see in the last line.

This patch removes sd_aspace in favor of an entire inode, sd_inode.
Functions that need to access the address space may use a new function
that follows the inode to its address space. This creates the 1:1 relation
between the inode and its address space, so lockdep doesn't complain.
This is how some other file systems manage their metadata, such as btrfs.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/glops.c      |  4 ++--
 fs/gfs2/incore.h     |  7 ++++++-
 fs/gfs2/meta_io.c    |  2 +-
 fs/gfs2/meta_io.h    |  2 --
 fs/gfs2/ops_fstype.c | 27 ++++++++++++++++-----------
 fs/gfs2/super.c      |  2 +-
 6 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
index 744cacd27213..5d755d30d91c 100644
--- a/fs/gfs2/glops.c
+++ b/fs/gfs2/glops.c
@@ -162,7 +162,7 @@ void gfs2_ail_flush(struct gfs2_glock *gl, bool fsync)
 static int gfs2_rgrp_metasync(struct gfs2_glock *gl)
 {
 	struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
-	struct address_space *metamapping = &sdp->sd_aspace;
+	struct address_space *metamapping = gfs2_aspace(sdp);
 	struct gfs2_rgrpd *rgd = gfs2_glock2rgrp(gl);
 	const unsigned bsize = sdp->sd_sb.sb_bsize;
 	loff_t start = (rgd->rd_addr * bsize) & PAGE_MASK;
@@ -219,7 +219,7 @@ static int rgrp_go_sync(struct gfs2_glock *gl)
 static void rgrp_go_inval(struct gfs2_glock *gl, int flags)
 {
 	struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
-	struct address_space *mapping = &sdp->sd_aspace;
+	struct address_space *mapping = gfs2_aspace(sdp);
 	struct gfs2_rgrpd *rgd = gfs2_glock2rgrp(gl);
 	const unsigned bsize = sdp->sd_sb.sb_bsize;
 	loff_t start = (rgd->rd_addr * bsize) & PAGE_MASK;
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index 6f31a067a5f2..4a03c3031347 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -795,7 +795,7 @@ struct gfs2_sbd {
 
 	/* Log stuff */
 
-	struct address_space sd_aspace;
+	struct inode *sd_inode;
 
 	spinlock_t sd_log_lock;
 
@@ -851,6 +851,11 @@ struct gfs2_sbd {
 	unsigned long sd_glock_dqs_held;
 };
 
+static inline struct address_space *gfs2_aspace(struct gfs2_sbd *sdp)
+{
+	return sdp->sd_inode->i_mapping;
+}
+
 static inline void gfs2_glstats_inc(struct gfs2_glock *gl, int which)
 {
 	gl->gl_stats.stats[which]++;
diff --git a/fs/gfs2/meta_io.c b/fs/gfs2/meta_io.c
index 7c9619997355..0123437d9c12 100644
--- a/fs/gfs2/meta_io.c
+++ b/fs/gfs2/meta_io.c
@@ -120,7 +120,7 @@ struct buffer_head *gfs2_getbuf(struct gfs2_glock *gl, u64 blkno, int create)
 	unsigned int bufnum;
 
 	if (mapping == NULL)
-		mapping = &sdp->sd_aspace;
+		mapping = gfs2_aspace(sdp);
 
 	shift = PAGE_SHIFT - sdp->sd_sb.sb_bsize_shift;
 	index = blkno >> shift;             /* convert block to page */
diff --git a/fs/gfs2/meta_io.h b/fs/gfs2/meta_io.h
index 21880d72081a..70b9c41ecb46 100644
--- a/fs/gfs2/meta_io.h
+++ b/fs/gfs2/meta_io.h
@@ -42,8 +42,6 @@ static inline struct gfs2_sbd *gfs2_mapping2sbd(struct address_space *mapping)
 	struct inode *inode = mapping->host;
 	if (mapping->a_ops == &gfs2_meta_aops)
 		return (((struct gfs2_glock *)mapping) - 1)->gl_name.ln_sbd;
-	else if (mapping->a_ops == &gfs2_rgrp_aops)
-		return container_of(mapping, struct gfs2_sbd, sd_aspace);
 	else
 		return inode->i_sb->s_fs_info;
 }
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 5b90d2b7db47..8051f130cf53 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -72,7 +72,6 @@ void free_sbd(struct gfs2_sbd *sdp)
 static struct gfs2_sbd *init_sbd(struct super_block *sb)
 {
 	struct gfs2_sbd *sdp;
-	struct address_space *mapping;
 
 	sdp = kzalloc(sizeof(struct gfs2_sbd), GFP_KERNEL);
 	if (!sdp)
@@ -112,16 +111,6 @@ static struct gfs2_sbd *init_sbd(struct super_block *sb)
 
 	INIT_LIST_HEAD(&sdp->sd_sc_inodes_list);
 
-	mapping = &sdp->sd_aspace;
-
-	address_space_init_once(mapping);
-	mapping->a_ops = &gfs2_rgrp_aops;
-	mapping->host = sb->s_bdev->bd_inode;
-	mapping->flags = 0;
-	mapping_set_gfp_mask(mapping, GFP_NOFS);
-	mapping->private_data = NULL;
-	mapping->writeback_index = 0;
-
 	spin_lock_init(&sdp->sd_log_lock);
 	atomic_set(&sdp->sd_log_pinned, 0);
 	INIT_LIST_HEAD(&sdp->sd_log_revokes);
@@ -1140,6 +1129,7 @@ static int gfs2_fill_super(struct super_block *sb, struct fs_context *fc)
 	struct gfs2_sbd *sdp;
 	struct gfs2_holder mount_gh;
 	struct gfs2_holder freeze_gh;
+	struct address_space *mapping;
 	int error;
 
 	sdp = init_sbd(sb);
@@ -1161,6 +1151,21 @@ static int gfs2_fill_super(struct super_block *sb, struct fs_context *fc)
 	sb->s_flags |= SB_NOSEC;
 	sb->s_magic = GFS2_MAGIC;
 	sb->s_op = &gfs2_super_ops;
+
+	/* Set up an address space for metadata writes */
+	sdp->sd_inode = new_inode(sb);
+	if (!sdp->sd_inode)
+		goto fail_free;
+	sdp->sd_inode->i_ino = GFS2_MAGIC;
+	set_nlink(sdp->sd_inode, 1);
+	sdp->sd_inode->i_size = i_size_read(sb->s_bdev->bd_inode);
+	insert_inode_hash(sdp->sd_inode);
+
+	mapping = gfs2_aspace(sdp);
+	mapping->a_ops = &gfs2_rgrp_aops;
+	mapping_set_gfp_mask(mapping, GFP_NOFS);
+	mapping->writeback_index = 0;
+
 	sb->s_d_op = &gfs2_dops;
 	sb->s_export_op = &gfs2_export_ops;
 	sb->s_qcop = &gfs2_quotactl_ops;
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 0a5b7dfa7a45..9857d8725b2d 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -617,13 +617,13 @@ static void gfs2_put_super(struct super_block *sb)
 	gfs2_jindex_free(sdp);
 	/*  Take apart glock structures and buffer lists  */
 	gfs2_gl_hash_clear(sdp);
-	truncate_inode_pages_final(&sdp->sd_aspace);
 	gfs2_delete_debugfs_file(sdp);
 	/*  Unmount the locking protocol  */
 	gfs2_lm_unmount(sdp);
 
 	/*  At this point, we're through participating in the lockspace  */
 	gfs2_sys_fs_del(sdp);
+	iput(sdp->sd_inode);
 	free_sbd(sdp);
 }
 
-- 
2.31.1

[Cluster-devel] [GFS2 PATCH 11/15] gfs2: reduce redundant code in gfs2_trans_add_*

[Bob Peterson]

Before this patch, functions gfs2_trans_add_data and gfs2_trans_add_meta
did similar checks to see if the buffer_head had an existing bd element,
and if not, assigned one, temporarily dropping locks to allow for better
simultaneous operations. These checks were identical except that the
meta version held the page lock after unlocking the log_lock and buffer
lock.

This patch consolidates the similar code into the helper function,
gfs2_alloc_bufdata, for consistency and to eliminate code redundancy.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/trans.c | 46 ++++++++++++++++++++--------------------------
 1 file changed, 20 insertions(+), 26 deletions(-)

diff --git a/fs/gfs2/trans.c b/fs/gfs2/trans.c
index 63fec11ef2ce..4f70d381fd67 100644
--- a/fs/gfs2/trans.c
+++ b/fs/gfs2/trans.c
@@ -163,8 +163,20 @@ void gfs2_trans_end(struct gfs2_sbd *sdp)
 static struct gfs2_bufdata *gfs2_alloc_bufdata(struct gfs2_glock *gl,
 					       struct buffer_head *bh)
 {
+	struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
 	struct gfs2_bufdata *bd;
 
+	bd = bh->b_private;
+	if (bd)
+		goto out_check_gl;
+
+	gfs2_log_unlock(sdp);
+	unlock_buffer(bh);
+	lock_page(bh->b_page);
+	bd = bh->b_private;
+	if (bd)
+		goto out_noalloc;
+
 	bd = kmem_cache_zalloc(gfs2_bufdata_cachep, GFP_NOFS | __GFP_NOFAIL);
 	bd->bd_bh = bh;
 	bd->bd_gl = gl;
@@ -172,6 +184,12 @@ static struct gfs2_bufdata *gfs2_alloc_bufdata(struct gfs2_glock *gl,
 	INIT_LIST_HEAD(&bd->bd_ail_st_list);
 	INIT_LIST_HEAD(&bd->bd_ail_gl_list);
 	bh->b_private = bd;
+out_noalloc:
+	unlock_page(bh->b_page);
+	lock_buffer(bh);
+	gfs2_log_lock(sdp);
+out_check_gl:
+	gfs2_assert(sdp, bd->bd_gl == gl);
 	return bd;
 }
 
@@ -201,18 +219,7 @@ void gfs2_trans_add_data(struct gfs2_glock *gl, struct buffer_head *bh)
 		goto out;
 	}
 	gfs2_log_lock(sdp);
-	bd = bh->b_private;
-	if (bd == NULL) {
-		gfs2_log_unlock(sdp);
-		unlock_buffer(bh);
-		if (bh->b_private == NULL)
-			bd = gfs2_alloc_bufdata(gl, bh);
-		else
-			bd = bh->b_private;
-		lock_buffer(bh);
-		gfs2_log_lock(sdp);
-	}
-	gfs2_assert(sdp, bd->bd_gl == gl);
+	bd = gfs2_alloc_bufdata(gl, bh);
 	set_bit(TR_TOUCHED, &tr->tr_flags);
 	if (list_empty(&bd->bd_list)) {
 		set_bit(GLF_LFLUSH, &bd->bd_gl->gl_flags);
@@ -241,20 +248,7 @@ void gfs2_trans_add_meta(struct gfs2_glock *gl, struct buffer_head *bh)
 		goto out;
 	}
 	gfs2_log_lock(sdp);
-	bd = bh->b_private;
-	if (bd == NULL) {
-		gfs2_log_unlock(sdp);
-		unlock_buffer(bh);
-		lock_page(bh->b_page);
-		if (bh->b_private == NULL)
-			bd = gfs2_alloc_bufdata(gl, bh);
-		else
-			bd = bh->b_private;
-		unlock_page(bh->b_page);
-		lock_buffer(bh);
-		gfs2_log_lock(sdp);
-	}
-	gfs2_assert(sdp, bd->bd_gl == gl);
+	bd = gfs2_alloc_bufdata(gl, bh);
 	set_bit(TR_TOUCHED, &tr->tr_flags);
 	if (!list_empty(&bd->bd_list))
 		goto out_unlock;
-- 
2.31.1

[Cluster-devel] [GFS2 PATCH 12/15] gfs2: Make recovery error more readable

[Bob Peterson]

Before this patch, withdraws could cause an error that looked like:
Journal recovery skipped for 0 until next mount.
This patch changes it to a more readable:
Journal recovery skipped for jid 0 until next mount.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/gfs2/util.c b/fs/gfs2/util.c
index f4325b44956d..34087bba88ee 100644
--- a/fs/gfs2/util.c
+++ b/fs/gfs2/util.c
@@ -295,7 +295,7 @@ static void signal_our_withdraw(struct gfs2_sbd *sdp)
 		fs_warn(sdp, "Journal recovery complete for jid %d.\n",
 			sdp->sd_lockstruct.ls_jid);
 	else
-		fs_warn(sdp, "Journal recovery skipped for %d until next "
+		fs_warn(sdp, "Journal recovery skipped for jid %d until next "
 			"mount.\n", sdp->sd_lockstruct.ls_jid);
 	fs_warn(sdp, "Glock dequeues delayed: %lu\n", sdp->sd_glock_dqs_held);
 	sdp->sd_glock_dqs_held = 0;
-- 
2.31.1

[Cluster-devel] [GFS2 PATCH 13/15] gfs2: ignore usr|grp|prjquota mount options

[Bob Peterson]

Before this patch, gfs2 rejected mounts attempted with the usrquota,
grpquota, or prjquota mount options. That caused numerous xfstests tests
to fail. This patch allows gfs2 to accept but ignore those mount options
so the tests may be run.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/ops_fstype.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 8051f130cf53..69bdc2917fb5 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -1373,6 +1373,7 @@ enum gfs2_param {
 	Opt_barrier,
 	Opt_rgrplvb,
 	Opt_loccookie,
+	Opt_ignore,
 };
 
 static const struct constant_table gfs2_param_quota[] = {
@@ -1431,6 +1432,9 @@ static const struct fs_parameter_spec gfs2_fs_parameters[] = {
 	/* quota can be a flag or an enum so it gets special treatment */
 	fsparam_flag_no("quota",	      Opt_quota_flag),
 	fsparam_enum("quota",		      Opt_quota, gfs2_param_quota),
+	fsparam_flag("usrquota",	      Opt_ignore),
+	fsparam_flag("grpquota",	      Opt_ignore),
+	fsparam_flag("prjquota",	      Opt_ignore),
 	{}
 };
 
@@ -1532,6 +1536,8 @@ static int gfs2_parse_param(struct fs_context *fc, struct fs_parameter *param)
 	case Opt_loccookie:
 		args->ar_loccookie = result.boolean;
 		break;
+	case Opt_ignore:
+		break;
 	default:
 		return invalfc(fc, "invalid mount option: %s", param->key);
 	}
-- 
2.31.1

[Cluster-devel] [GFS2 PATCH 14/15] fs: Move notify_change permission checks into may_setattr

[Bob Peterson]

From: Andreas Gruenbacher <agruenba@redhat.com>

Move the permission checks in notify_change into a separate function to
make them available to filesystems.

When notify_change is called, the vfs performs those checks before
calling into iop->setattr.  However, a filesystem like gfs2 can only
lock and revalidate the inode inside ->setattr, and it must then repeat
those checks to err on the safe side.

It would be nice to get rid of the double checking, but moving the
permission check into iop->setattr altogether isn't really an option.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/attr.c          | 50 ++++++++++++++++++++++++++++------------------
 include/linux/fs.h |  2 ++
 2 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/fs/attr.c b/fs/attr.c
index 87ef39db1c34..473d21b3a86d 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -249,6 +249,34 @@ void setattr_copy(struct user_namespace *mnt_userns, struct inode *inode,
 }
 EXPORT_SYMBOL(setattr_copy);
 
+int may_setattr(struct user_namespace *mnt_userns, struct inode *inode,
+		unsigned int ia_valid)
+{
+	int error;
+
+	if (ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID | ATTR_TIMES_SET)) {
+		if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
+			return -EPERM;
+	}
+
+	/*
+	 * If utimes(2) and friends are called with times == NULL (or both
+	 * times are UTIME_NOW), then we need to check for write permission
+	 */
+	if (ia_valid & ATTR_TOUCH) {
+		if (IS_IMMUTABLE(inode))
+			return -EPERM;
+
+		if (!inode_owner_or_capable(mnt_userns, inode)) {
+			error = inode_permission(mnt_userns, inode, MAY_WRITE);
+			if (error)
+				return error;
+		}
+	}
+	return 0;
+}
+EXPORT_SYMBOL(may_setattr);
+
 /**
  * notify_change - modify attributes of a filesytem object
  * @mnt_userns:	user namespace of the mount the inode was found from
@@ -290,25 +318,9 @@ int notify_change(struct user_namespace *mnt_userns, struct dentry *dentry,
 
 	WARN_ON_ONCE(!inode_is_locked(inode));
 
-	if (ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID | ATTR_TIMES_SET)) {
-		if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
-			return -EPERM;
-	}
-
-	/*
-	 * If utimes(2) and friends are called with times == NULL (or both
-	 * times are UTIME_NOW), then we need to check for write permission
-	 */
-	if (ia_valid & ATTR_TOUCH) {
-		if (IS_IMMUTABLE(inode))
-			return -EPERM;
-
-		if (!inode_owner_or_capable(mnt_userns, inode)) {
-			error = inode_permission(mnt_userns, inode, MAY_WRITE);
-			if (error)
-				return error;
-		}
-	}
+	error = may_setattr(mnt_userns, inode, ia_valid);
+	if (error)
+		return error;
 
 	if ((ia_valid & ATTR_MODE)) {
 		umode_t amode = attr->ia_mode;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 640574294216..50192964bf6b 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3469,6 +3469,8 @@ extern int buffer_migrate_page_norefs(struct address_space *,
 #define buffer_migrate_page_norefs NULL
 #endif
 
+int may_setattr(struct user_namespace *mnt_userns, struct inode *inode,
+		unsigned int ia_valid);
 int setattr_prepare(struct user_namespace *, struct dentry *, struct iattr *);
 extern int inode_newsize_ok(const struct inode *, loff_t offset);
 void setattr_copy(struct user_namespace *, struct inode *inode,
-- 
2.31.1

[Cluster-devel] [GFS2 PATCH 15/15] gfs2: Switch to may_setattr in gfs2_setattr

[Bob Peterson]

From: Andreas Gruenbacher <agruenba@redhat.com>

The permission check in gfs2_setattr is an old and outdated version of
may_setattr().  Switch to the updated version.

Fixes fstest generic/079.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/inode.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 6e15434b23ac..3130f85d2b3f 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -1985,8 +1985,8 @@ static int gfs2_setattr(struct user_namespace *mnt_userns,
 	if (error)
 		goto out;
 
-	error = -EPERM;
-	if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
+	error = may_setattr(&init_user_ns, inode, attr->ia_valid);
+	if (error)
 		goto error;
 
 	error = setattr_prepare(&init_user_ns, dentry, attr);
-- 
2.31.1

[Cluster-devel] [GFS2 PATCH 00/15] gfs2: misc. patch collection (V2)

[Andreas Gruenbacher]

On Tue, Jul 27, 2021 at 7:37 PM Bob Peterson <rpeterso@redhat.com> wrote:
> This is version2 of a set of misc. patches from my collection.
> As before, they can be added individually or as a set. Changes from V1:
>
> 1. I added a wrapper patch Andreas wrote. I'm not sure how serious he is
>    about this one.

This is from the mmap-fault patch queue; it starts to make sense once
you add the retry code.

> 2. This set omits the patch "New log flush watchdog" due to Steve
>    Whitehouse's objections.
> 3. New patches were added to allow several more xfstests to run.
> 4. A new patch fixes a recovery error more understandable.
> 5. Patch "gfs2: fix deadlock in gfs2_ail1_empty withdraw" has been
>    reworked somewhat because of problems discovered by HCH.
> 6. A new patch was added to reduce code redundancy in gfs2_trans_add_*.
> 7. Two new patches from Andreas were added that allow xfstests
>    generic/079 to run on gfs2.
>
> Most of these patches are very safe and well-tested.
> I left out some of my more experimental patches.
>
> Andreas Gruenbacher (3):
>   gfs2: Add wrapper for iomap_file_buffered_write
>   fs: Move notify_change permission checks into may_setattr
>   gfs2: Switch to may_setattr in gfs2_setattr
>
> Bob Peterson (12):
>   gfs2: Fix glock recursion in freeze_go_xmote_bh
>   gfs2: Eliminate go_xmote_bh in favor of go_lock
>   gfs2: be more verbose replaying invalid rgrp blocks
>   gfs2: trivial clean up of gfs2_ail_error
>   gfs2: tiny cleanup in gfs2_log_reserve
>   gfs2: init system threads before freeze lock
>   gfs2: Don't release and reacquire local statfs bh
>   gfs2: fix deadlock in gfs2_ail1_empty withdraw
>   gfs2: replace sd_aspace with sd_inode
>   gfs2: reduce redundant code in gfs2_trans_add_*
>   gfs2: Make recovery error more readable
>   gfs2: ignore usr|grp|prjquota mount options
>
>  fs/attr.c            |  50 ++++++++++++--------
>  fs/gfs2/aops.c       |   9 +---
>  fs/gfs2/file.c       |  20 +++++---
>  fs/gfs2/glock.c      |  12 +----
>  fs/gfs2/glops.c      |  43 +++++++++--------
>  fs/gfs2/incore.h     |   9 +++-
>  fs/gfs2/inode.c      |   4 +-
>  fs/gfs2/log.c        |  18 +++++---
>  fs/gfs2/lops.c       |  44 ++++++++++++------
>  fs/gfs2/meta_io.c    |   2 +-
>  fs/gfs2/meta_io.h    |   2 -
>  fs/gfs2/ops_fstype.c |  84 ++++++++++++++++++++++++++++-----
>  fs/gfs2/super.c      | 107 ++++++++-----------------------------------
>  fs/gfs2/super.h      |   3 +-
>  fs/gfs2/trans.c      |  46 ++++++++-----------
>  fs/gfs2/util.c       |   2 +-
>  include/linux/fs.h   |   2 +
>  17 files changed, 238 insertions(+), 219 deletions(-)
>
> --
> 2.31.1
>

Thanks,
Andreas

[Cluster-devel] [GFS2 PATCH 09/15] gfs2: fix deadlock in gfs2_ail1_empty withdraw

[Andreas Gruenbacher]

Hi Bob,

On Tue, Jul 27, 2021 at 7:37 PM Bob Peterson <rpeterso@redhat.com> wrote:
> Before this patch, function gfs2_ail1_empty could issue a file system
> withdraw when IO errors were discovered. However, there are several
> callers, including gfs2_flush_revokes() which holds the gfs2_log_lock
> before calling gfs2_ail1_empty. If gfs2_ail1_empty needed to withdraw
> it would leave the gfs2_log_lock held, which resulted in a deadlock
> due to other processes that needed the log_lock.
>
> Another problem discovered by Christoph Helwig is that we cannot
> withdraw from the log_flush process because it may be called from
> the glock workqueue, and the withdraw process waits for that very
> workqueue to be flushed. So the withdraw must be ignored until it may
> be handled by a more appropriate context like the gfs2_logd daemon.
>
> This patch moves the withdraw out of function gfs2_ail1_empty and
> makes each of the callers check for a withdraw by calling new function
> check_ail1_withdraw.

> Function gfs2_flush_revokes now does this check
> after releasing the gfs2_log_lock to avoid the deadlock.

I don't see that in the code.

>
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> ---
>  fs/gfs2/log.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
> index f0ee3ff6f9a8..c687a8c4e044 100644
> --- a/fs/gfs2/log.c
> +++ b/fs/gfs2/log.c
> @@ -364,11 +364,6 @@ static int gfs2_ail1_empty(struct gfs2_sbd *sdp, int max_revokes)
>         ret = list_empty(&sdp->sd_ail1_list);
>         spin_unlock(&sdp->sd_ail_lock);
>
> -       if (test_bit(SDF_WITHDRAWING, &sdp->sd_flags)) {
> -               gfs2_lm(sdp, "fatal: I/O error(s)\n");
> -               gfs2_withdraw(sdp);
> -       }
> -
>         return ret;
>  }
>
> @@ -786,6 +781,15 @@ void gfs2_glock_remove_revoke(struct gfs2_glock *gl)
>         }
>  }
>
> +static void check_ail1_withdraw(struct gfs2_sbd *sdp)
> +{
> +       if (!test_bit(SDF_WITHDRAWING, &sdp->sd_flags))
> +               return;
> +
> +       gfs2_lm(sdp, "fatal: I/O error(s)\n");
> +       gfs2_withdraw(sdp);
> +}
> +
>  /**
>   * gfs2_flush_revokes - Add as many revokes to the system transaction as we can
>   * @sdp: The GFS2 superblock
> @@ -1317,6 +1321,7 @@ int gfs2_logd(void *data)
>
>                 if (gfs2_jrnl_flush_reqd(sdp) || t == 0) {
>                         gfs2_ail1_empty(sdp, 0);
> +                       check_ail1_withdraw(sdp);
>                         gfs2_log_flush(sdp, NULL, GFS2_LOG_HEAD_FLUSH_NORMAL |
>                                                   GFS2_LFC_LOGD_JFLUSH_REQD);
>                 }
> @@ -1325,6 +1330,7 @@ int gfs2_logd(void *data)
>                         gfs2_ail1_start(sdp);
>                         gfs2_ail1_wait(sdp);
>                         gfs2_ail1_empty(sdp, 0);
> +                       check_ail1_withdraw(sdp);
>                         gfs2_log_flush(sdp, NULL, GFS2_LOG_HEAD_FLUSH_NORMAL |
>                                                   GFS2_LFC_LOGD_AIL_FLUSH_REQD);
>                 }
> --
> 2.31.1
>

Thanks,
Andreas

[Cluster-devel] [GFS2 PATCH 09/15] gfs2: fix deadlock in gfs2_ail1_empty withdraw

[Bob Peterson]

On 7/28/21 12:38 AM, Andreas Gruenbacher wrote:
> Hi Bob,
> 
> On Tue, Jul 27, 2021 at 7:37 PM Bob Peterson <rpeterso@redhat.com> wrote:
>> Before this patch, function gfs2_ail1_empty could issue a file system
>> withdraw when IO errors were discovered. However, there are several
>> callers, including gfs2_flush_revokes() which holds the gfs2_log_lock
>> before calling gfs2_ail1_empty. If gfs2_ail1_empty needed to withdraw
>> it would leave the gfs2_log_lock held, which resulted in a deadlock
>> due to other processes that needed the log_lock.
>>
>> Another problem discovered by Christoph Helwig is that we cannot
>> withdraw from the log_flush process because it may be called from
>> the glock workqueue, and the withdraw process waits for that very
>> workqueue to be flushed. So the withdraw must be ignored until it may
>> be handled by a more appropriate context like the gfs2_logd daemon.
>>
>> This patch moves the withdraw out of function gfs2_ail1_empty and
>> makes each of the callers check for a withdraw by calling new function
>> check_ail1_withdraw.
> 
>> Function gfs2_flush_revokes now does this check
>> after releasing the gfs2_log_lock to avoid the deadlock.
> 
> I don't see that in the code.

Yeah, the comment was wrong. I noticed the problem and already removed 
the paragraph after the patch set was sent out.

Bob

[Cluster-devel] [GFS2 PATCH 13/15] gfs2: ignore usr|grp|prjquota mount options

[Andreas Gruenbacher]

On Tue, Jul 27, 2021 at 7:37 PM Bob Peterson <rpeterso@redhat.com> wrote:
> Before this patch, gfs2 rejected mounts attempted with the usrquota,
> grpquota, or prjquota mount options. That caused numerous xfstests tests
> to fail. This patch allows gfs2 to accept but ignore those mount options
> so the tests may be run.

We can't just fake this up. Instead, the fstests need to be fixed to
skip tests that don't run on particular filesystems.

> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> ---
>  fs/gfs2/ops_fstype.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
> index 8051f130cf53..69bdc2917fb5 100644
> --- a/fs/gfs2/ops_fstype.c
> +++ b/fs/gfs2/ops_fstype.c
> @@ -1373,6 +1373,7 @@ enum gfs2_param {
>         Opt_barrier,
>         Opt_rgrplvb,
>         Opt_loccookie,
> +       Opt_ignore,
>  };
>
>  static const struct constant_table gfs2_param_quota[] = {
> @@ -1431,6 +1432,9 @@ static const struct fs_parameter_spec gfs2_fs_parameters[] = {
>         /* quota can be a flag or an enum so it gets special treatment */
>         fsparam_flag_no("quota",              Opt_quota_flag),
>         fsparam_enum("quota",                 Opt_quota, gfs2_param_quota),
> +       fsparam_flag("usrquota",              Opt_ignore),
> +       fsparam_flag("grpquota",              Opt_ignore),
> +       fsparam_flag("prjquota",              Opt_ignore),
>         {}
>  };
>
> @@ -1532,6 +1536,8 @@ static int gfs2_parse_param(struct fs_context *fc, struct fs_parameter *param)
>         case Opt_loccookie:
>                 args->ar_loccookie = result.boolean;
>                 break;
> +       case Opt_ignore:
> +               break;
>         default:
>                 return invalfc(fc, "invalid mount option: %s", param->key);
>         }
> --
> 2.31.1
>

Andreas

[Cluster-devel] [GFS2 PATCH 13/15] gfs2: ignore usr|grp|prjquota mount options

[Bob Peterson]

On 7/28/21 1:28 PM, Andreas Gruenbacher wrote:
> On Tue, Jul 27, 2021 at 7:37 PM Bob Peterson <rpeterso@redhat.com> wrote:
>> Before this patch, gfs2 rejected mounts attempted with the usrquota,
>> grpquota, or prjquota mount options. That caused numerous xfstests tests
>> to fail. This patch allows gfs2 to accept but ignore those mount options
>> so the tests may be run.
> 
> We can't just fake this up. Instead, the fstests need to be fixed to
> skip tests that don't run on particular filesystems.
Hi Andreas,

I see your point, and sure, the maintainers of xfstests can improve the 
tests to make them check whether the options are supported for gfs2 as
they do with other some other options.

Still, if you look at the man page for "mount(8)" under "Mount options 
for affs" you see:

"grpquota|noquota|quota|usrquota
These options are accepted but ignored.  (However, quota utilities may 
react to such strings in /etc/fstab.)"

If you look at "Mount options for jfs" you similarly see:

"noquota|quota|usrquota|grpquota
These options are accepted but ignored."

So if affs and jfs can ignore these mount options, why can't gfs2?
Are we planning to support them ever?

Bob Peterson

[Cluster-devel] [GFS2 PATCH 13/15] gfs2: ignore usr|grp|prjquota mount options

[Andreas Gruenbacher]

On Wed, Jul 28, 2021 at 10:33 PM Bob Peterson <rpeterso@redhat.com> wrote:
> On 7/28/21 1:28 PM, Andreas Gruenbacher wrote:
> > On Tue, Jul 27, 2021 at 7:37 PM Bob Peterson <rpeterso@redhat.com> wrote:
> >> Before this patch, gfs2 rejected mounts attempted with the usrquota,
> >> grpquota, or prjquota mount options. That caused numerous xfstests tests
> >> to fail. This patch allows gfs2 to accept but ignore those mount options
> >> so the tests may be run.
> >
> > We can't just fake this up. Instead, the fstests need to be fixed to
> > skip tests that don't run on particular filesystems.
> Hi Andreas,
>
> I see your point, and sure, the maintainers of xfstests can improve the
> tests to make them check whether the options are supported for gfs2 as
> they do with other some other options.
>
> Still, if you look at the man page for "mount(8)" under "Mount options
> for affs" you see:
>
> "grpquota|noquota|quota|usrquota
> These options are accepted but ignored.  (However, quota utilities may
> react to such strings in /etc/fstab.)"
>
> If you look at "Mount options for jfs" you similarly see:
>
> "noquota|quota|usrquota|grpquota
> These options are accepted but ignored."
>
> So if affs and jfs can ignore these mount options, why can't gfs2?
> Are we planning to support them ever?

Hmm, that's interesting. I don't know what to think about that.

Andreas

[Cluster-devel] [GFS2 PATCH 13/15] gfs2: ignore usr|grp|prjquota mount options

[Andrew Price]

On 28/07/2021 21:32, Bob Peterson wrote:
> On 7/28/21 1:28 PM, Andreas Gruenbacher wrote:
>> On Tue, Jul 27, 2021 at 7:37 PM Bob Peterson <rpeterso@redhat.com> wrote:
>>> Before this patch, gfs2 rejected mounts attempted with the usrquota,
>>> grpquota, or prjquota mount options. That caused numerous xfstests tests
>>> to fail. This patch allows gfs2 to accept but ignore those mount options
>>> so the tests may be run.
>>
>> We can't just fake this up. Instead, the fstests need to be fixed to
>> skip tests that don't run on particular filesystems.
> Hi Andreas,
> 
> I see your point, and sure, the maintainers of xfstests can improve the 
> tests to make them check whether the options are supported for gfs2 as
> they do with other some other options.
> 
> Still, if you look at the man page for "mount(8)" under "Mount options 
> for affs" you see:
> 
> "grpquota|noquota|quota|usrquota
> These options are accepted but ignored.? (However, quota utilities may 
> react to such strings in /etc/fstab.)"
> 
> If you look at "Mount options for jfs" you similarly see:
> 
> "noquota|quota|usrquota|grpquota
> These options are accepted but ignored."
> 
> So if affs and jfs can ignore these mount options, why can't gfs2?
> Are we planning to support them ever?
FWIW I agree with Andreas, this is definitely a bug in the test suite. 
If we fix fstests then we can use fstests for bisecting older gfs2 
issues, but if we no-op incompatible mount options that make their way 
into the generic tests then we're only fixing the tests for future gfs2 
versions and not older ones.

Andy