From 8dc14966ca3eb5bf4e200c50cc73199ee6de2bd7 Mon Sep 17 00:00:00 2001 From: Andrew Price Date: Tue, 14 Mar 2023 13:18:27 +0000 Subject: gfs2: Remove duplicate i_nlink check from gfs2_link() The duplication is: struct gfs2_inode *ip = GFS2_I(inode); [...] error = -ENOENT; if (inode->i_nlink == 0) goto out_gunlock; [...] error = -EINVAL; if (!ip->i_inode.i_nlink) goto out_gunlock; The second check is removed. ENOENT is the correct error code for attempts to link a deleted inode (ref: link(2)). If we support O_TMPFILE in future the check will need to be updated with an exception for inodes flagged I_LINKABLE so sorting out this duplication now will make it a slightly cleaner change. Signed-off-by: Andrew Price Signed-off-by: Andreas Gruenbacher --- fs/gfs2/inode.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'fs') diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c index 1291b5ee3584..79eef9a0ebfc 100644 --- a/fs/gfs2/inode.c +++ b/fs/gfs2/inode.c @@ -992,9 +992,6 @@ static int gfs2_link(struct dentry *old_dentry, struct inode *dir, error = -EPERM; if (IS_IMMUTABLE(inode) || IS_APPEND(inode)) goto out_gunlock; - error = -EINVAL; - if (!ip->i_inode.i_nlink) - goto out_gunlock; error = -EMLINK; if (ip->i_inode.i_nlink == (u32)-1) goto out_gunlock; -- cgit v1.2.3 From 2d08478060438a5af86f44adba2a16727c89ff03 Mon Sep 17 00:00:00 2001 From: Andrew Price Date: Tue, 14 Mar 2023 13:18:28 +0000 Subject: gfs2: Remove ghs[] from gfs2_link Replace the 2-item array with two variables for readability. Signed-off-by: Andrew Price Signed-off-by: Andreas Gruenbacher --- fs/gfs2/inode.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'fs') diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c index 79eef9a0ebfc..9850267b9951 100644 --- a/fs/gfs2/inode.c +++ b/fs/gfs2/inode.c @@ -941,7 +941,7 @@ static int gfs2_link(struct dentry *old_dentry, struct inode *dir, struct gfs2_sbd *sdp = GFS2_SB(dir); struct inode *inode = d_inode(old_dentry); struct gfs2_inode *ip = GFS2_I(inode); - struct gfs2_holder ghs[2]; + struct gfs2_holder d_gh, gh; struct buffer_head *dibh; struct gfs2_diradd da = { .bh = NULL, .save_loc = 1, }; int error; @@ -953,14 +953,14 @@ static int gfs2_link(struct dentry *old_dentry, struct inode *dir, if (error) return error; - gfs2_holder_init(dip->i_gl, LM_ST_EXCLUSIVE, 0, ghs); - gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, ghs + 1); + gfs2_holder_init(dip->i_gl, LM_ST_EXCLUSIVE, 0, &d_gh); + gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, &gh); - error = gfs2_glock_nq(ghs); /* parent */ + error = gfs2_glock_nq(&d_gh); if (error) goto out_parent; - error = gfs2_glock_nq(ghs + 1); /* child */ + error = gfs2_glock_nq(&gh); if (error) goto out_child; @@ -1046,13 +1046,13 @@ out_gunlock_q: gfs2_quota_unlock(dip); out_gunlock: gfs2_dir_no_add(&da); - gfs2_glock_dq(ghs + 1); + gfs2_glock_dq(&gh); out_child: - gfs2_glock_dq(ghs); + gfs2_glock_dq(&d_gh); out_parent: gfs2_qa_put(dip); - gfs2_holder_uninit(ghs); - gfs2_holder_uninit(ghs + 1); + gfs2_holder_uninit(&d_gh); + gfs2_holder_uninit(&gh); return error; } -- cgit v1.2.3 From 14a585177c0f5870c82b32000d79aca1baa94bfc Mon Sep 17 00:00:00 2001 From: Andrew Price Date: Tue, 14 Mar 2023 13:18:29 +0000 Subject: gfs2: Remove ghs[] from gfs2_unlink Replace the 3-item array with three variables for readability. Signed-off-by: Andrew Price Signed-off-by: Andreas Gruenbacher --- fs/gfs2/inode.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) (limited to 'fs') diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c index 9850267b9951..17c994a0c0d0 100644 --- a/fs/gfs2/inode.c +++ b/fs/gfs2/inode.c @@ -1143,7 +1143,7 @@ static int gfs2_unlink(struct inode *dir, struct dentry *dentry) struct gfs2_sbd *sdp = GFS2_SB(dir); struct inode *inode = d_inode(dentry); struct gfs2_inode *ip = GFS2_I(inode); - struct gfs2_holder ghs[3]; + struct gfs2_holder d_gh, r_gh, gh; struct gfs2_rgrpd *rgd; int error; @@ -1153,21 +1153,21 @@ static int gfs2_unlink(struct inode *dir, struct dentry *dentry) error = -EROFS; - gfs2_holder_init(dip->i_gl, LM_ST_EXCLUSIVE, 0, ghs); - gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, ghs + 1); + gfs2_holder_init(dip->i_gl, LM_ST_EXCLUSIVE, 0, &d_gh); + gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, &gh); rgd = gfs2_blk2rgrpd(sdp, ip->i_no_addr, 1); if (!rgd) goto out_inodes; - gfs2_holder_init(rgd->rd_gl, LM_ST_EXCLUSIVE, LM_FLAG_NODE_SCOPE, ghs + 2); + gfs2_holder_init(rgd->rd_gl, LM_ST_EXCLUSIVE, LM_FLAG_NODE_SCOPE, &r_gh); - error = gfs2_glock_nq(ghs); /* parent */ + error = gfs2_glock_nq(&d_gh); if (error) goto out_parent; - error = gfs2_glock_nq(ghs + 1); /* child */ + error = gfs2_glock_nq(&gh); if (error) goto out_child; @@ -1181,7 +1181,7 @@ static int gfs2_unlink(struct inode *dir, struct dentry *dentry) goto out_rgrp; } - error = gfs2_glock_nq(ghs + 2); /* rgrp */ + error = gfs2_glock_nq(&r_gh); /* rgrp */ if (error) goto out_rgrp; @@ -1197,16 +1197,16 @@ static int gfs2_unlink(struct inode *dir, struct dentry *dentry) gfs2_trans_end(sdp); out_gunlock: - gfs2_glock_dq(ghs + 2); + gfs2_glock_dq(&r_gh); out_rgrp: - gfs2_glock_dq(ghs + 1); + gfs2_glock_dq(&gh); out_child: - gfs2_glock_dq(ghs); + gfs2_glock_dq(&d_gh); out_parent: - gfs2_holder_uninit(ghs + 2); + gfs2_holder_uninit(&r_gh); out_inodes: - gfs2_holder_uninit(ghs + 1); - gfs2_holder_uninit(ghs); + gfs2_holder_uninit(&gh); + gfs2_holder_uninit(&d_gh); return error; } -- cgit v1.2.3 From cfcdb5bad34f600aed7613c3c1a5e618111f77b7 Mon Sep 17 00:00:00 2001 From: Andreas Gruenbacher Date: Tue, 28 Mar 2023 00:43:16 +0200 Subject: gfs2: Fix inode height consistency check The maximum allowed height of an inode's metadata tree depends on the filesystem block size; it is lower for bigger-block filesystems. When reading in an inode, make sure that the height doesn't exceed the maximum allowed height. Arrays like sd_heightsize are sized to be big enough for any filesystem block size; they will often be slightly bigger than what's needed for a specific filesystem. Reported-by: syzbot+45d4691b1ed3c48eba05@syzkaller.appspotmail.com Signed-off-by: Andreas Gruenbacher --- fs/gfs2/glops.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c index 4d99cc77a29b..b65950e76be5 100644 --- a/fs/gfs2/glops.c +++ b/fs/gfs2/glops.c @@ -396,6 +396,7 @@ static int inode_go_demote_ok(const struct gfs2_glock *gl) static int gfs2_dinode_in(struct gfs2_inode *ip, const void *buf) { + struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode); const struct gfs2_dinode *str = buf; struct timespec64 atime; u16 height, depth; @@ -442,7 +443,7 @@ static int gfs2_dinode_in(struct gfs2_inode *ip, const void *buf) /* i_diskflags and i_eattr must be set before gfs2_set_inode_flags() */ gfs2_set_inode_flags(inode); height = be16_to_cpu(str->di_height); - if (unlikely(height > GFS2_MAX_META_HEIGHT)) + if (unlikely(height > sdp->sd_max_height)) goto corrupt; ip->i_height = (u8)height; -- cgit v1.2.3 From 7d1b37787fe3997b9612a8ba6766580f44e23796 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Fri, 7 Apr 2023 14:09:51 -0400 Subject: gfs2: Eliminate gfs2_trim_blocks Function gfs2_trim_blocks is not referenced. Eliminate it. Signed-off-by: Bob Peterson Signed-off-by: Andreas Gruenbacher --- fs/gfs2/bmap.c | 8 -------- fs/gfs2/bmap.h | 1 - 2 files changed, 9 deletions(-) (limited to 'fs') diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c index eedf6926c652..c739b258a2d9 100644 --- a/fs/gfs2/bmap.c +++ b/fs/gfs2/bmap.c @@ -2035,14 +2035,6 @@ static int do_shrink(struct inode *inode, u64 newsize) return error; } -void gfs2_trim_blocks(struct inode *inode) -{ - int ret; - - ret = do_shrink(inode, inode->i_size); - WARN_ON(ret != 0); -} - /** * do_grow - Touch and update inode size * @inode: The inode diff --git a/fs/gfs2/bmap.h b/fs/gfs2/bmap.h index 53cce6c08e81..e5b7d17131ed 100644 --- a/fs/gfs2/bmap.h +++ b/fs/gfs2/bmap.h @@ -58,7 +58,6 @@ extern int gfs2_get_extent(struct inode *inode, u64 lblock, u64 *dblock, extern int gfs2_alloc_extent(struct inode *inode, u64 lblock, u64 *dblock, unsigned *extlen, bool *new); extern int gfs2_setattr_size(struct inode *inode, u64 size); -extern void gfs2_trim_blocks(struct inode *inode); extern int gfs2_truncatei_resume(struct gfs2_inode *ip); extern int gfs2_file_dealloc(struct gfs2_inode *ip); extern int gfs2_write_alloc_required(struct gfs2_inode *ip, u64 offset, -- cgit v1.2.3 From 130cf5269cd267a9bf179017666f454d82bc13ef Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Mon, 10 Apr 2023 14:33:41 -0400 Subject: gfs2: Use gfs2_holder_initialized for jindex Before this patch function init_journal() used a local variable jindex to keep track of whether it needed to dequeue the jindex holder when errors were found. It also uselessly set the variable just before returning from the function. This patch simplifies the code by eliminatinng the local variable in favor of using function gfs2_holder_initialized. Signed-off-by: Bob Peterson Signed-off-by: Andreas Gruenbacher --- fs/gfs2/ops_fstype.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'fs') diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c index 6de901c3b89b..9af9ddb61ca0 100644 --- a/fs/gfs2/ops_fstype.c +++ b/fs/gfs2/ops_fstype.c @@ -734,13 +734,11 @@ static int init_journal(struct gfs2_sbd *sdp, int undo) struct inode *master = d_inode(sdp->sd_master_dir); struct gfs2_holder ji_gh; struct gfs2_inode *ip; - int jindex = 1; int error = 0; - if (undo) { - jindex = 0; + gfs2_holder_mark_uninitialized(&ji_gh); + if (undo) goto fail_statfs; - } sdp->sd_jindex = gfs2_lookup_simple(master, "jindex"); if (IS_ERR(sdp->sd_jindex)) { @@ -852,7 +850,6 @@ static int init_journal(struct gfs2_sbd *sdp, int undo) sdp->sd_log_idle = 1; set_bit(SDF_JOURNAL_CHECKED, &sdp->sd_flags); gfs2_glock_dq_uninit(&ji_gh); - jindex = 0; INIT_WORK(&sdp->sd_freeze_work, gfs2_freeze_func); return 0; @@ -869,7 +866,7 @@ fail_journal_gh: gfs2_glock_dq_uninit(&sdp->sd_journal_gh); fail_jindex: gfs2_jindex_free(sdp); - if (jindex) + if (gfs2_holder_initialized(&ji_gh)) gfs2_glock_dq_uninit(&ji_gh); fail: iput(sdp->sd_jindex); -- cgit v1.2.3 From 55534c094fd4e47db5547a013feab3ee88576e73 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Thu, 13 Apr 2023 20:54:30 +0200 Subject: gfs2: Move variable assignment behind a null pointer check in inode_go_dump Since commit 27a2660f1ef9 ("gfs2: Dump nrpages for inodes and their glocks"), inode_go_dump() computes the address of inode within ip before checking if ip is NULL. This isn't a bug by itself, but it can give rise to bugs later. Avoid that by checking if ip is NULL first. Signed-off-by: Markus Elfring Signed-off-by: Andreas Gruenbacher --- fs/gfs2/glops.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c index b65950e76be5..6e33c8058059 100644 --- a/fs/gfs2/glops.c +++ b/fs/gfs2/glops.c @@ -535,12 +535,13 @@ static void inode_go_dump(struct seq_file *seq, struct gfs2_glock *gl, const char *fs_id_buf) { struct gfs2_inode *ip = gl->gl_object; - struct inode *inode = &ip->i_inode; + struct inode *inode; unsigned long nrpages; if (ip == NULL) return; + inode = &ip->i_inode; xa_lock_irq(&inode->i_data.i_pages); nrpages = inode->i_data.nrpages; xa_unlock_irq(&inode->i_data.i_pages); -- cgit v1.2.3 From 24ab15829867a9cc09dfd16f41ed75aa8272a45f Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Fri, 21 Apr 2023 15:07:07 -0400 Subject: gfs2: return errors from gfs2_ail_empty_gl Before this patch, function gfs2_ail_empty_gl did not return errors it encountered from __gfs2_trans_begin. Those errors usually came from the fact that the file system was made read-only, often due to unmount (but theoretically could be due to -o remount,ro), which prevented the transaction from starting. The inability to start a transaction prevented its revokes from being properly written to the journal for glocks during unmount (and transition to ro). That meant glocks could be unlocked without the metadata properly revoked in the journal. So other nodes could grab the glock thinking that their lvb values were correct, but in fact corresponded to the glock without its revokes properly synced. That presented as lvb mismatch errors. This patch allows gfs2_ail_empty_gl to return the error properly to the caller. Signed-off-by: Bob Peterson Signed-off-by: Andreas Gruenbacher --- fs/gfs2/glops.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'fs') diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c index 6e33c8058059..29e0a664efd6 100644 --- a/fs/gfs2/glops.c +++ b/fs/gfs2/glops.c @@ -90,7 +90,7 @@ static int gfs2_ail_empty_gl(struct gfs2_glock *gl) struct gfs2_sbd *sdp = gl->gl_name.ln_sbd; struct gfs2_trans tr; unsigned int revokes; - int ret; + int ret = 0; revokes = atomic_read(&gl->gl_ail_count); @@ -132,7 +132,7 @@ static int gfs2_ail_empty_gl(struct gfs2_glock *gl) flush: gfs2_log_flush(sdp, NULL, GFS2_LOG_HEAD_FLUSH_NORMAL | GFS2_LFC_AIL_EMPTY_GL); - return 0; + return ret; } void gfs2_ail_flush(struct gfs2_glock *gl, bool fsync) @@ -326,7 +326,9 @@ static int inode_go_sync(struct gfs2_glock *gl) ret = gfs2_inode_metasync(gl); if (!error) error = ret; - gfs2_ail_empty_gl(gl); + ret = gfs2_ail_empty_gl(gl); + if (!error) + error = ret; /* * Writeback of the data mapping may cause the dirty flag to be set * so we have to clear it again here. -- cgit v1.2.3 From 68ca088dc1cfc4e366811b11ffe2954c6dcebca1 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Fri, 21 Apr 2023 15:07:08 -0400 Subject: gfs2: Perform second log flush in gfs2_make_fs_ro Before this patch, function gfs2_make_fs_ro called gfs2_log_flush once to finalize the log. However, if there's dirty metadata, log flushes tend to sync the metadata and formulate revokes. Before this patch, those revokes may not be written out to the journal immediately, which meant unresolved glocks could still have revokes in their ail lists. When the glock worker runs, it tries to transition the glock, but the unresolved revokes in the ail still need to be written, so it tries to start a transaction. It's impossible to start a transaction because at that point, the SDF_JOURNAL_LIVE flag has been cleared by gfs2_make_fs_ro. That causes the glock worker to fail, unable to write the revokes. The calling sequence looked something like this: gfs2_make_fs_ro gfs2_log_flush - with GFS2_LOG_HEAD_FLUSH_SHUTDOWN flag set if (flags & GFS2_LOG_HEAD_FLUSH_SHUTDOWN) clear_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags); ...meanwhile... glock_work_func do_xmote rgrp_go_sync (or possibly inode_go_sync) ... gfs2_ail_empty_gl __gfs2_trans_begin if (unlikely(!test_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags))) { ... return -EROFS; The previous patch in the series ("gfs2: return errors from gfs2_ail_empty_gl") now causes the transaction error to no longer be ignored, so it causes a warning from MOST of the xfstests: WARNING: CPU: 11 PID: X at fs/gfs2/super.c:603 gfs2_put_super [gfs2] which corresponds to: WARN_ON(gfs2_withdrawing(sdp)); The withdraw was triggered silently from do_xmote by: if (unlikely(sdp->sd_log_error && !gfs2_withdrawn(sdp))) gfs2_withdraw_delayed(sdp); This patch adds a second log_flush to gfs2_make_fs_ro: one to sync the data and one to sync any outstanding revokes and finalize the journal. Note that both of these log flushes need to be "special," in other words, not GFS2_LOG_HEAD_FLUSH_NORMAL. Signed-off-by: Bob Peterson Signed-off-by: Andreas Gruenbacher --- fs/gfs2/super.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'fs') diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c index a83fa62106f0..5eed8c237500 100644 --- a/fs/gfs2/super.c +++ b/fs/gfs2/super.c @@ -552,6 +552,15 @@ void gfs2_make_fs_ro(struct gfs2_sbd *sdp) gfs2_quota_sync(sdp->sd_vfs, 0); gfs2_statfs_sync(sdp->sd_vfs, 0); + /* We do two log flushes here. The first one commits dirty inodes + * and rgrps to the journal, but queues up revokes to the ail list. + * The second flush writes out and removes the revokes. + * + * The first must be done before the FLUSH_SHUTDOWN code + * clears the LIVE flag, otherwise it will not be able to start + * a transaction to write its revokes, and the error will cause + * a withdraw of the file system. */ + gfs2_log_flush(sdp, NULL, GFS2_LFC_MAKE_FS_RO); gfs2_log_flush(sdp, NULL, GFS2_LOG_HEAD_FLUSH_SHUTDOWN | GFS2_LFC_MAKE_FS_RO); wait_event_timeout(sdp->sd_log_waitq, -- cgit v1.2.3 From b97e583caa25abf95695cd06d7f9512b484c6c01 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Fri, 21 Apr 2023 15:07:09 -0400 Subject: gfs2: Issue message when revokes cannot be written Before this patch, function gfs2_ail_empty_gl would silently return an error to the caller. This would get silently set into sd_log_error which would cause a withdraw, but there was no indication why the file system was withdrawn. This patch adds a fs_err to log the appropriate error message. Signed-off-by: Bob Peterson Signed-off-by: Andreas Gruenbacher --- fs/gfs2/glops.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c index 29e0a664efd6..caef70334af3 100644 --- a/fs/gfs2/glops.c +++ b/fs/gfs2/glops.c @@ -124,8 +124,10 @@ static int gfs2_ail_empty_gl(struct gfs2_glock *gl) memset(&tr, 0, sizeof(tr)); set_bit(TR_ONSTACK, &tr.tr_flags); ret = __gfs2_trans_begin(&tr, sdp, 0, revokes, _RET_IP_); - if (ret) + if (ret) { + fs_err(sdp, "Transaction error %d: Unable to write revokes.", ret); goto flush; + } __gfs2_ail_flush(gl, 0, revokes); gfs2_trans_end(sdp); -- cgit v1.2.3 From 644f6bf762fa903f64c59c2ec0f4d0d753527053 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Fri, 21 Apr 2023 15:07:10 -0400 Subject: gfs2: gfs2_ail_empty_gl no log flush on error Before this patch, function gfs2_ail_empty_gl called gfs2_log_flush even in cases where it encountered an error. It should probably skip the log flush and leave the file system in an inconsistent state, letting a subsequent withdraw force the journal to be replayed to reestablish metadata consistency. Signed-off-by: Bob Peterson Signed-off-by: Andreas Gruenbacher --- fs/gfs2/glops.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'fs') diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c index caef70334af3..01d433ed6ce7 100644 --- a/fs/gfs2/glops.c +++ b/fs/gfs2/glops.c @@ -132,8 +132,9 @@ static int gfs2_ail_empty_gl(struct gfs2_glock *gl) gfs2_trans_end(sdp); flush: - gfs2_log_flush(sdp, NULL, GFS2_LOG_HEAD_FLUSH_NORMAL | - GFS2_LFC_AIL_EMPTY_GL); + if (!ret) + gfs2_log_flush(sdp, NULL, GFS2_LOG_HEAD_FLUSH_NORMAL | + GFS2_LFC_AIL_EMPTY_GL); return ret; } -- cgit v1.2.3