From a6579cbfd7216b071008db13360c322a6b21400b Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Mon, 12 Jul 2021 17:24:30 +0100 Subject: [PATCH 01/15] gfs2: Fix memory leak of object lsi on error return path In the case where IS_ERR(lsi->si_sc_inode) is true the error exit path to free_local does not kfree the allocated object lsi leading to a memory leak. Fix this by kfree'ing lst before taking the error exit path. Addresses-Coverity: ("Resource leak") Fixes: 97fd734ba17e ("gfs2: lookup local statfs inodes prior to journal recovery") Signed-off-by: Colin Ian King Signed-off-by: Andreas Gruenbacher --- fs/gfs2/ops_fstype.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c index 5f4504dd0875..bd3b3be1a473 100644 --- a/fs/gfs2/ops_fstype.c +++ b/fs/gfs2/ops_fstype.c @@ -677,6 +677,7 @@ static int init_statfs(struct gfs2_sbd *sdp) error = PTR_ERR(lsi->si_sc_inode); fs_err(sdp, "can't find local \"sc\" file#%u: %d\n", jd->jd_jid, error); + kfree(lsi); goto free_local; } lsi->si_jid = jd->jd_jid; From 9d9b16054b7d357afde69a027514c695092b0d22 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Tue, 1 Jun 2021 09:41:40 -0500 Subject: [PATCH 02/15] gfs2: Fix glock recursion in freeze_go_xmote_bh 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 --- fs/gfs2/glops.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c index 54d3fbeb3002..384565d63eea 100644 --- a/fs/gfs2/glops.c +++ b/fs/gfs2/glops.c @@ -610,16 +610,13 @@ static int freeze_go_xmote_bh(struct gfs2_glock *gl) 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)) + return error; + if (gfs2_assert_withdraw_delayed(sdp, head.lh_flags & + GFS2_LOG_HEAD_UNMOUNT)) + return -EIO; + sdp->sd_log_sequence = head.lh_sequence + 1; + gfs2_log_pointers_init(sdp, head.lh_blkno); } return 0; } From c37453cb87e38623cb47437fdbf54ffc1262cc45 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Mon, 22 Feb 2021 10:29:46 -0500 Subject: [PATCH 03/15] gfs2: be more verbose replaying invalid rgrp blocks 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 --- fs/gfs2/lops.c | 42 ++++++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 14 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; + if (mh->mh_type == cpu_to_be32(GFS2_METATYPE_RG)) + obsolete_rgrp(jd, bh_log, blkno); - 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); - } - } mark_buffer_dirty(bh_ip); } brelse(bh_log); From 69a61144f32b590650af8b5f1e1262f1a731f9c5 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Mon, 24 May 2021 11:51:26 -0500 Subject: [PATCH 04/15] gfs2: trivial clean up of gfs2_ail_error 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 --- 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 384565d63eea..5fbdc7e9f4ff 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); } /** From dc7674eda002037d7a2d551e272037574507c2db Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Wed, 30 Jun 2021 12:49:59 -0500 Subject: [PATCH 05/15] gfs2: tiny cleanup in gfs2_log_reserve 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 --- 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) { From a28dc123fa66ba7f3eca7cffc4b01d96bfd35c27 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Fri, 14 May 2021 07:42:33 -0500 Subject: [PATCH 06/15] gfs2: init system threads before freeze lock 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: 96b1454f2e8e ("gfs2: move freeze glock outside the make_fs_rw and _ro functions") Signed-off-by: Bob Peterson --- 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; } From 70c11ba8f2dc6ff216477a8dd7ec0ad8568c410e Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Wed, 30 Jun 2021 11:46:17 -0500 Subject: [PATCH 07/15] gfs2: Don't release and reacquire local statfs bh 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 --- 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 e6f820f146cb..397b091cbed1 100644 --- a/fs/gfs2/incore.h +++ b/fs/gfs2/incore.h @@ -768,6 +768,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); + error = gfs2_trans_begin(sdp, 2 * RES_DINODE, 0); if (error) goto out_bh; - error = gfs2_trans_begin(sdp, 2 * RES_DINODE, 0); - if (error) - goto out_bh2; - - 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); From 7392fbb0a402ec4e4342a5e26a4bd6c359e67165 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Mon, 26 Jul 2021 12:53:12 -0500 Subject: [PATCH 08/15] gfs2: Make recovery error more readable 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 --- 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; From a8f1d32d0f04354ee4dddb83072413f2c299a192 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Tue, 27 Jul 2021 13:52:42 -0500 Subject: [PATCH 09/15] gfs2: Eliminate vestigial HIF_FIRST Holder flag HIF_FIRST is no longer used or needed, so remove it. Signed-off-by: Bob Peterson --- fs/gfs2/glock.c | 2 -- fs/gfs2/incore.h | 1 - 2 files changed, 3 deletions(-) diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c index 1f3902ecdded..ff8d6a79fc6e 100644 --- a/fs/gfs2/glock.c +++ b/fs/gfs2/glock.c @@ -2077,8 +2077,6 @@ static const char *hflags2str(char *buf, u16 flags, unsigned long iflags) *p++ = 'H'; if (test_bit(HIF_WAIT, &iflags)) *p++ = 'W'; - if (test_bit(HIF_FIRST, &iflags)) - *p++ = 'F'; *p = 0; return buf; } diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h index 397b091cbed1..0fe49770166e 100644 --- a/fs/gfs2/incore.h +++ b/fs/gfs2/incore.h @@ -253,7 +253,6 @@ struct gfs2_lkstats { enum { /* States */ HIF_HOLDER = 6, /* Set for gh that "holds" the glock */ - HIF_FIRST = 7, HIF_WAIT = 10, }; From ba3ca2bcf4aa20670849f621f059b3657fd7614a Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Thu, 29 Jul 2021 07:34:39 -0500 Subject: [PATCH 10/15] gfs2: nit: gfs2_drop_inode shouldn't return bool Today, gfs2_drop_inode can return "false" for an int value. I'm sure this was just an oversight. Change to int value. Signed-off-by: Bob Peterson --- fs/gfs2/super.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c index 0a5b7dfa7a45..6e00d15ef0a8 100644 --- a/fs/gfs2/super.c +++ b/fs/gfs2/super.c @@ -947,7 +947,7 @@ static int gfs2_drop_inode(struct inode *inode) gfs2_glock_hold(gl); if (!gfs2_queue_delete_work(gl, 0)) gfs2_glock_queue_put(gl); - return false; + return 0; } return generic_drop_inode(inode); From 1b8550b5de7610027609ef605f85dc29f1d9da82 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Fri, 30 Jul 2021 12:40:25 -0500 Subject: [PATCH 11/15] gfs2: Mark journal inodes as "don't cache" Before this patch, journal inodes were considered regular inodes, which meant that instead of evicting them, function iput_final would just put them on the lru for later processing. If the file system withdrew for whatever reason, the withdraw would never be seen until the inode was evicted, which could be indefinitely. This patch marks all journal inodes as "don't cache" which means function iput_final will evict them immediately, allowing us to properly recover the journal on other cluster nodes. Signed-off-by: Bob Peterson --- fs/gfs2/ops_fstype.c | 1 + fs/gfs2/util.c | 1 + 2 files changed, 2 insertions(+) diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c index 5b90d2b7db47..7f8410d8fdc1 100644 --- a/fs/gfs2/ops_fstype.c +++ b/fs/gfs2/ops_fstype.c @@ -614,6 +614,7 @@ static int gfs2_jindex_hold(struct gfs2_sbd *sdp, struct gfs2_holder *ji_gh) break; } + d_mark_dontcache(jd->jd_inode); spin_lock(&sdp->sd_jindex_spin); jd->jd_jid = sdp->sd_journals++; jip = GFS2_I(jd->jd_inode); diff --git a/fs/gfs2/util.c b/fs/gfs2/util.c index 34087bba88ee..cf345a86ef67 100644 --- a/fs/gfs2/util.c +++ b/fs/gfs2/util.c @@ -278,6 +278,7 @@ static void signal_our_withdraw(struct gfs2_sbd *sdp) goto skip_recovery; } sdp->sd_jdesc->jd_inode = inode; + d_mark_dontcache(inode); /* * Now wait until recovery is complete. From 8cc67f704f4b61384343629feb1f1c30d64188c6 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Fri, 30 Jul 2021 12:41:16 -0500 Subject: [PATCH 12/15] gfs2: don't stop reads while withdraw in progress When gfs2 withdraws a file system, it calls signal_our_withdraw which triggers another node to replay the withdrawing node's journal. Then it waits until it knows the journal has been replayed. Part of this wait is to repeatedly call check_journal_clean which calls gfs2_jdesc_check, which checks to see if the journal is sane. As part of its sanity checks it needs to re-read its journal's metadata. But with today's code, any attempt to re-read the metadata results in -EIO because of a check for the file system withdraw in function gfs2_meta_wait. This patch adds an additional check for SDF_WITHDRAW_IN_PROG, to tell if the read is done while the withdraw is in progress. In that case we allow the metadata read to not be rejected. Therefore the metadata check is done properly, so the withdraw sequence can finish normally. Signed-off-by: Bob Peterson --- fs/gfs2/meta_io.c | 7 +++---- fs/gfs2/util.h | 5 +++++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/fs/gfs2/meta_io.c b/fs/gfs2/meta_io.c index 7c9619997355..72d30a682ece 100644 --- a/fs/gfs2/meta_io.c +++ b/fs/gfs2/meta_io.c @@ -258,8 +258,7 @@ int gfs2_meta_read(struct gfs2_glock *gl, u64 blkno, int flags, struct buffer_head *bh, *bhs[2]; int num = 0; - if (unlikely(gfs2_withdrawn(sdp)) && - (!sdp->sd_jdesc || gl != sdp->sd_jinode_gl)) { + if (unlikely(gfs2_withdrawn(sdp)) && !gfs2_withdraw_in_prog(sdp)) { *bhp = NULL; return -EIO; } @@ -317,7 +316,7 @@ int gfs2_meta_read(struct gfs2_glock *gl, u64 blkno, int flags, int gfs2_meta_wait(struct gfs2_sbd *sdp, struct buffer_head *bh) { - if (unlikely(gfs2_withdrawn(sdp))) + if (unlikely(gfs2_withdrawn(sdp)) && !gfs2_withdraw_in_prog(sdp)) return -EIO; wait_on_buffer(bh); @@ -328,7 +327,7 @@ int gfs2_meta_wait(struct gfs2_sbd *sdp, struct buffer_head *bh) gfs2_io_error_bh_wd(sdp, bh); return -EIO; } - if (unlikely(gfs2_withdrawn(sdp))) + if (unlikely(gfs2_withdrawn(sdp)) && !gfs2_withdraw_in_prog(sdp)) return -EIO; return 0; diff --git a/fs/gfs2/util.h b/fs/gfs2/util.h index 69e1a0ae5a4d..78ec190f4155 100644 --- a/fs/gfs2/util.h +++ b/fs/gfs2/util.h @@ -218,6 +218,11 @@ static inline bool gfs2_withdrawing(struct gfs2_sbd *sdp) !test_bit(SDF_WITHDRAWN, &sdp->sd_flags); } +static inline bool gfs2_withdraw_in_prog(struct gfs2_sbd *sdp) +{ + return test_bit(SDF_WITHDRAW_IN_PROG, &sdp->sd_flags); +} + #define gfs2_tune_get(sdp, field) \ gfs2_tune_get_i(&(sdp)->sd_tune, &(sdp)->sd_tune.field) From d1340f80f0b8066321b499a376780da00560e857 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Fri, 30 Jul 2021 12:41:49 -0500 Subject: [PATCH 13/15] gfs2: Don't call dlm after protocol is unmounted In the gfs2 withdraw sequence, the dlm protocol is unmounted with a call to lm_unmount. After a withdraw, users are allowed to unmount the withdrawn file system. But at that point we may still have glocks left over that we need to free via unmount's call to gfs2_gl_hash_clear. These glocks may have never been completed because of whatever problem caused the withdraw (IO errors or whatever). Before this patch, function gdlm_put_lock would still try to call into dlm to unlock these leftover glocks, which resulted in dlm returning -EINVAL because the lock space was abandoned. These glocks were never freed because there was no mechanism after that to free them. This patch adds a check to gdlm_put_lock to see if the locking protocol was inactive (DFL_UNMOUNT flag) and if so, free the glock and not make the invalid call into dlm. I could have combined this "if" with the one that follows, related to leftover glock LVBs, but I felt the code was more readable with its own if clause. Signed-off-by: Bob Peterson --- fs/gfs2/lock_dlm.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/fs/gfs2/lock_dlm.c b/fs/gfs2/lock_dlm.c index dac040162ecc..50578f881e6d 100644 --- a/fs/gfs2/lock_dlm.c +++ b/fs/gfs2/lock_dlm.c @@ -299,6 +299,11 @@ static void gdlm_put_lock(struct gfs2_glock *gl) gfs2_sbstats_inc(gl, GFS2_LKS_DCOUNT); gfs2_update_request_times(gl); + /* don't want to call dlm if we've unmounted the lock protocol */ + if (test_bit(DFL_UNMOUNT, &ls->ls_recover_flags)) { + gfs2_glock_free(gl); + return; + } /* don't want to skip dlm_unlock writing the lvb when lock has one */ if (test_bit(SDF_SKIP_DLM_UNLOCK, &sdp->sd_flags) && From fffe9bee14b0e04ef632b96279fa44cb3df80812 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Fri, 30 Jul 2021 13:23:49 -0500 Subject: [PATCH 14/15] gfs2: Delay withdraw from atomic context Before this patch, if function __gfs2_ail_flush detected an error syncing the ail list, it call gfs2_ail_error which called gfs2_withdraw. Since __gfs2_ail_flush deals with a specific glock, we shouldn't withdraw immediately because the withdraw code (signal_our_withdraw) uses glocks in its processing. This patch changes the call from gfs2_withdraw to gfs2_withdraw_delayed which defers the withdraw until a more appropriate context, such as the logd daemon, discovers the intent to withdraw. Reported-by: Dan Carpenter Signed-off-by: Bob Peterson --- fs/gfs2/glops.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c index 5fbdc7e9f4ff..79c621c7863d 100644 --- a/fs/gfs2/glops.c +++ b/fs/gfs2/glops.c @@ -44,7 +44,7 @@ static void gfs2_ail_error(struct gfs2_glock *gl, const struct buffer_head *bh) gl->gl_name.ln_type, gl->gl_name.ln_number, gfs2_glock2aspace(gl)); gfs2_lm(sdp, "AIL error\n"); - gfs2_withdraw(sdp); + gfs2_withdraw_delayed(sdp); } /** From 08d736667185dca2751cf47eabb0830cecdeb160 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Mon, 2 Aug 2021 09:08:51 -0500 Subject: [PATCH 15/15] gfs2: Remove redundant check from gfs2_glock_dq In function gfs2_glock_dq, it checks to see if this is the fast path. Before this patch, it checked both "find_first_holder(gl) == NULL" and list_empty(&gl->gl_holders), which is redundant. If gl_holders is empty then find_first_holder must return NULL. This patch removes the redundancy. Signed-off-by: Bob Peterson --- fs/gfs2/glock.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c index ff8d6a79fc6e..e0eaa9cf9fb6 100644 --- a/fs/gfs2/glock.c +++ b/fs/gfs2/glock.c @@ -1494,12 +1494,11 @@ void gfs2_glock_dq(struct gfs2_holder *gh) list_del_init(&gh->gh_list); clear_bit(HIF_HOLDER, &gh->gh_iflags); - if (find_first_holder(gl) == NULL) { - if (list_empty(&gl->gl_holders) && - !test_bit(GLF_PENDING_DEMOTE, &gl->gl_flags) && - !test_bit(GLF_DEMOTE, &gl->gl_flags)) - fast_path = 1; - } + if (list_empty(&gl->gl_holders) && + !test_bit(GLF_PENDING_DEMOTE, &gl->gl_flags) && + !test_bit(GLF_DEMOTE, &gl->gl_flags)) + fast_path = 1; + if (!test_bit(GLF_LFLUSH, &gl->gl_flags) && demote_ok(gl)) gfs2_glock_add_to_lru(gl);