From 6edb6ba333d31bb42d9ab8b1b6ff1c5ecbc3813c Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Mon, 13 Sep 2021 12:34:38 -0500 Subject: gfs2: remove redundant check in gfs2_rgrp_go_lock Before this patch, function gfs2_rgrp_go_lock checked if GL_SKIP and ar_rgrplvb were both true. However, GL_SKIP is only set for rgrps if ar_rgrplvb is true (see gfs2_inplace_reserve). This patch simply removes the redundant check. Signed-off-by: Bob Peterson Signed-off-by: Andreas Gruenbacher --- fs/gfs2/rgrp.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'fs/gfs2') diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c index c3b00ba92ed2..7a13a687e4f2 100644 --- a/fs/gfs2/rgrp.c +++ b/fs/gfs2/rgrp.c @@ -1291,9 +1291,8 @@ static int update_rgrp_lvb(struct gfs2_rgrpd *rgd) int gfs2_rgrp_go_lock(struct gfs2_holder *gh) { struct gfs2_rgrpd *rgd = gh->gh_gl->gl_object; - struct gfs2_sbd *sdp = rgd->rd_sbd; - if (gh->gh_flags & GL_SKIP && sdp->sd_args.ar_rgrplvb) + if (gh->gh_flags & GL_SKIP) return 0; return gfs2_rgrp_bh_get(rgd); } -- cgit v1.2.3 From 4c69038d9087109e4dcf559cd0ecb44875af3089 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Mon, 13 Sep 2021 12:40:09 -0500 Subject: gfs2: Add GL_SKIP holder flag to dump_holder Somehow, the GL_SKIP flag was missed when dumping glock holders. This patch adds it to function hflags2str. I added it at the end because I wanted Holder and Skip flags together to read "Hs" rather than "sH" to avoid confusion with "Shared" ("SH") holder state. Signed-off-by: Bob Peterson Signed-off-by: Andreas Gruenbacher --- fs/gfs2/glock.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'fs/gfs2') diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c index f686083d0250..ff1227287284 100644 --- a/fs/gfs2/glock.c +++ b/fs/gfs2/glock.c @@ -2238,6 +2238,8 @@ static const char *hflags2str(char *buf, u16 flags, unsigned long iflags) *p++ = 'W'; if (test_bit(HIF_MAY_DEMOTE, &iflags)) *p++ = 'D'; + if (flags & GL_SKIP) + *p++ = 's'; *p = 0; return buf; } -- cgit v1.2.3 From c1442f6b53d887d08a098ec23056be7cfea1e3c7 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Mon, 13 Sep 2021 12:54:16 -0500 Subject: gfs2: move GL_SKIP check from glops to do_promote Before this patch, each individual "go_lock" glock operation (glop) checked the GL_SKIP flag, and if set, would skip further processing. This patch changes the logic so the go_lock caller, function go_promote, checks the GL_SKIP flag before calling the go_lock op in the first place. This avoids having to unnecessarily unlock gl_lockref.lock only to re-lock it again. Signed-off-by: Bob Peterson Signed-off-by: Andreas Gruenbacher --- fs/gfs2/glock.c | 26 ++++++++++++++------------ fs/gfs2/glops.c | 2 +- fs/gfs2/rgrp.c | 2 -- 3 files changed, 15 insertions(+), 15 deletions(-) (limited to 'fs/gfs2') diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c index ff1227287284..5d6b253030a1 100644 --- a/fs/gfs2/glock.c +++ b/fs/gfs2/glock.c @@ -502,18 +502,20 @@ restart: } if (gh->gh_list.prev == &gl->gl_holders && glops->go_lock) { - spin_unlock(&gl->gl_lockref.lock); - /* FIXME: eliminate this eventually */ - ret = glops->go_lock(gh); - spin_lock(&gl->gl_lockref.lock); - if (ret) { - if (ret == 1) - return 2; - gh->gh_error = ret; - list_del_init(&gh->gh_list); - trace_gfs2_glock_queue(gh, 0); - gfs2_holder_wake(gh); - goto restart; + if (!(gh->gh_flags & GL_SKIP)) { + spin_unlock(&gl->gl_lockref.lock); + /* FIXME: eliminate this eventually */ + ret = glops->go_lock(gh); + spin_lock(&gl->gl_lockref.lock); + if (ret) { + if (ret == 1) + return 2; + gh->gh_error = ret; + list_del_init(&gh->gh_list); + trace_gfs2_glock_queue(gh, 0); + gfs2_holder_wake(gh); + goto restart; + } } set_bit(HIF_HOLDER, &gh->gh_iflags); trace_gfs2_promote(gh, 1); diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c index 79c621c7863d..4b19f513570f 100644 --- a/fs/gfs2/glops.c +++ b/fs/gfs2/glops.c @@ -495,7 +495,7 @@ static int inode_go_lock(struct gfs2_holder *gh) struct gfs2_inode *ip = gl->gl_object; int error = 0; - if (!ip || (gh->gh_flags & GL_SKIP)) + if (!ip) return 0; if (test_bit(GIF_INVALID, &ip->i_flags)) { diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c index 7a13a687e4f2..1fb66f6e6a0c 100644 --- a/fs/gfs2/rgrp.c +++ b/fs/gfs2/rgrp.c @@ -1292,8 +1292,6 @@ int gfs2_rgrp_go_lock(struct gfs2_holder *gh) { struct gfs2_rgrpd *rgd = gh->gh_gl->gl_object; - if (gh->gh_flags & GL_SKIP) - return 0; return gfs2_rgrp_bh_get(rgd); } -- cgit v1.2.3 From c98c2ca5eae988c47991074643e5543df60ada35 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Tue, 14 Sep 2021 08:16:58 -0500 Subject: gfs2: Switch some BUG_ON to GLOCK_BUG_ON for debug In rgrp.c, there are several places where it does BUG_ON. This tells us the call stack but nothing more, which is not very helpful. This patch switches them to GLOCK_BUG_ON which also prints the glock, its holders, and many of the rgrp values, which will help us debug problems in the future. Signed-off-by: Bob Peterson Signed-off-by: Andreas Gruenbacher --- fs/gfs2/rgrp.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'fs/gfs2') diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c index 1fb66f6e6a0c..96b2fbed6bf1 100644 --- a/fs/gfs2/rgrp.c +++ b/fs/gfs2/rgrp.c @@ -1230,7 +1230,7 @@ static int gfs2_rgrp_bh_get(struct gfs2_rgrpd *rgd) rgrp_set_bitmap_flags(rgd); rgd->rd_flags |= (GFS2_RDF_UPTODATE | GFS2_RDF_CHECK); rgd->rd_free_clone = rgd->rd_free; - BUG_ON(rgd->rd_reserved); + GLOCK_BUG_ON(rgd->rd_gl, rgd->rd_reserved); /* max out the rgrp allocation failure point */ rgd->rd_extfail_pt = rgd->rd_free; } @@ -1280,7 +1280,7 @@ static int update_rgrp_lvb(struct gfs2_rgrpd *rgd) rgd->rd_free = be32_to_cpu(rgd->rd_rgl->rl_free); rgrp_set_bitmap_flags(rgd); rgd->rd_free_clone = rgd->rd_free; - BUG_ON(rgd->rd_reserved); + GLOCK_BUG_ON(rgd->rd_gl, rgd->rd_reserved); /* max out the rgrp allocation failure point */ rgd->rd_extfail_pt = rgd->rd_free; rgd->rd_dinodes = be32_to_cpu(rgd->rd_rgl->rl_dinodes); @@ -2212,7 +2212,7 @@ void gfs2_inplace_release(struct gfs2_inode *ip) struct gfs2_rgrpd *rgd = rs->rs_rgd; spin_lock(&rgd->rd_rsspin); - BUG_ON(rgd->rd_reserved < rs->rs_reserved); + GLOCK_BUG_ON(rgd->rd_gl, rgd->rd_reserved < rs->rs_reserved); rgd->rd_reserved -= rs->rs_reserved; spin_unlock(&rgd->rd_rsspin); rs->rs_reserved = 0; @@ -2473,9 +2473,9 @@ int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *nblocks, spin_unlock(&rbm.rgd->rd_rsspin); goto rgrp_error; } - BUG_ON(rbm.rgd->rd_reserved < *nblocks); - BUG_ON(rbm.rgd->rd_free_clone < *nblocks); - BUG_ON(rbm.rgd->rd_free < *nblocks); + GLOCK_BUG_ON(rbm.rgd->rd_gl, rbm.rgd->rd_reserved < *nblocks); + GLOCK_BUG_ON(rbm.rgd->rd_gl, rbm.rgd->rd_free_clone < *nblocks); + GLOCK_BUG_ON(rbm.rgd->rd_gl, rbm.rgd->rd_free < *nblocks); rbm.rgd->rd_reserved -= *nblocks; rbm.rgd->rd_free_clone -= *nblocks; rbm.rgd->rd_free -= *nblocks; @@ -2762,8 +2762,8 @@ void gfs2_rlist_free(struct gfs2_rgrp_list *rlist) void rgrp_lock_local(struct gfs2_rgrpd *rgd) { - BUG_ON(!gfs2_glock_is_held_excl(rgd->rd_gl) && - !test_bit(SDF_NORECOVERY, &rgd->rd_sbd->sd_flags)); + GLOCK_BUG_ON(rgd->rd_gl, !gfs2_glock_is_held_excl(rgd->rd_gl) && + !test_bit(SDF_NORECOVERY, &rgd->rd_sbd->sd_flags)); mutex_lock(&rgd->rd_mutex); } -- cgit v1.2.3 From a500bd3155f24398be968fa1fdba37308063f059 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Wed, 25 Aug 2021 12:57:24 -0500 Subject: gfs2: Allow append and immutable bits to coexist Before this patch, function do_gfs2_set_flags checked if the append and immutable flags were being set while already set. If so, error -EPERM was given. There's no reason why these two flags should be mutually exclusive, and if you set them separately, you will, in essence, set one while it is already set. For example: chattr +a /mnt/gfs2/file1 chattr +i /mnt/gfs2/file1 The first command sets the append-only flag. Since they are additive, the second command sets the immutable flag AND append-only flag, since they both coexist in i_diskflags. So the second command should not return an error. This bug caused xfstests generic/545 to fail. This patch simply removes the invalid checks. I also eliminated an unused parm from do_gfs2_set_flags. Signed-off-by: Bob Peterson Signed-off-by: Andreas Gruenbacher --- fs/gfs2/file.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) (limited to 'fs/gfs2') diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c index 40e6501c02e5..0f54d870e402 100644 --- a/fs/gfs2/file.c +++ b/fs/gfs2/file.c @@ -213,11 +213,9 @@ void gfs2_set_inode_flags(struct inode *inode) * @inode: The inode * @reqflags: The flags to set * @mask: Indicates which flags are valid - * @fsflags: The FS_* inode flags passed in * */ -static int do_gfs2_set_flags(struct inode *inode, u32 reqflags, u32 mask, - const u32 fsflags) +static int do_gfs2_set_flags(struct inode *inode, u32 reqflags, u32 mask) { struct gfs2_inode *ip = GFS2_I(inode); struct gfs2_sbd *sdp = GFS2_SB(inode); @@ -237,10 +235,6 @@ static int do_gfs2_set_flags(struct inode *inode, u32 reqflags, u32 mask, goto out; error = -EPERM; - if (IS_IMMUTABLE(inode) && (new_flags & GFS2_DIF_IMMUTABLE)) - goto out; - if (IS_APPEND(inode) && (new_flags & GFS2_DIF_APPENDONLY)) - goto out; if (!IS_IMMUTABLE(inode)) { error = gfs2_permission(&init_user_ns, inode, MAY_WRITE); if (error) @@ -313,7 +307,7 @@ int gfs2_fileattr_set(struct user_namespace *mnt_userns, mask &= ~(GFS2_DIF_TOPDIR | GFS2_DIF_INHERIT_JDATA); } - return do_gfs2_set_flags(inode, gfsflags, mask, fsflags); + return do_gfs2_set_flags(inode, gfsflags, mask); } static int gfs2_getlabel(struct file *filp, char __user *label) -- cgit v1.2.3 From b016d9a84abdd2efaa273814eaeb59e112ecffbd Mon Sep 17 00:00:00 2001 From: Andreas Gruenbacher Date: Thu, 30 Sep 2021 13:49:36 -0500 Subject: gfs2: Save ip from gfs2_glock_nq_init Before this patch, when a glock was locked by function gfs2_glock_nq_init, it initialized the holder gh_ip (return address) as gfs2_glock_nq_init. That made it extremely difficult to track down problems because many functions call gfs2_glock_nq_init. This patch changes the function so that it saves gh_ip from the caller of gfs2_glock_nq_init, which makes it easy to backtrack which holder took the lock. Signed-off-by: Andreas Gruenbacher Signed-off-by: Bob Peterson --- fs/gfs2/glock.c | 8 ++++---- fs/gfs2/glock.h | 13 ++++++++++--- 2 files changed, 14 insertions(+), 7 deletions(-) (limited to 'fs/gfs2') diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c index 5d6b253030a1..54a1ed373ff3 100644 --- a/fs/gfs2/glock.c +++ b/fs/gfs2/glock.c @@ -911,7 +911,7 @@ static void gfs2_glock_poke(struct gfs2_glock *gl) struct gfs2_holder gh; int error; - gfs2_holder_init(gl, LM_ST_SHARED, flags, &gh); + __gfs2_holder_init(gl, LM_ST_SHARED, flags, &gh, _RET_IP_); error = gfs2_glock_nq(&gh); if (!error) gfs2_glock_dq(&gh); @@ -1208,12 +1208,12 @@ out: * */ -void gfs2_holder_init(struct gfs2_glock *gl, unsigned int state, u16 flags, - struct gfs2_holder *gh) +void __gfs2_holder_init(struct gfs2_glock *gl, unsigned int state, u16 flags, + struct gfs2_holder *gh, unsigned long ip) { INIT_LIST_HEAD(&gh->gh_list); gh->gh_gl = gl; - gh->gh_ip = _RET_IP_; + gh->gh_ip = ip; gh->gh_owner_pid = get_pid(task_pid(current)); gh->gh_state = state; gh->gh_flags = flags; diff --git a/fs/gfs2/glock.h b/fs/gfs2/glock.h index 9012487da4c6..a36104229d0d 100644 --- a/fs/gfs2/glock.h +++ b/fs/gfs2/glock.h @@ -190,8 +190,15 @@ extern int gfs2_glock_get(struct gfs2_sbd *sdp, u64 number, extern void gfs2_glock_hold(struct gfs2_glock *gl); extern void gfs2_glock_put(struct gfs2_glock *gl); extern void gfs2_glock_queue_put(struct gfs2_glock *gl); -extern void gfs2_holder_init(struct gfs2_glock *gl, unsigned int state, - u16 flags, struct gfs2_holder *gh); + +extern void __gfs2_holder_init(struct gfs2_glock *gl, unsigned int state, + u16 flags, struct gfs2_holder *gh, + unsigned long ip); +static inline void gfs2_holder_init(struct gfs2_glock *gl, unsigned int state, + u16 flags, struct gfs2_holder *gh) { + __gfs2_holder_init(gl, state, flags, gh, _RET_IP_); +} + extern void gfs2_holder_reinit(unsigned int state, u16 flags, struct gfs2_holder *gh); extern void gfs2_holder_uninit(struct gfs2_holder *gh); @@ -241,7 +248,7 @@ static inline int gfs2_glock_nq_init(struct gfs2_glock *gl, { int error; - gfs2_holder_init(gl, state, flags, gh); + __gfs2_holder_init(gl, state, flags, gh, _RET_IP_); error = gfs2_glock_nq(gh); if (error) -- cgit v1.2.3 From 763766c0571ea14e9790242201b7199b395b8bf5 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Wed, 29 Sep 2021 14:42:38 -0500 Subject: gfs2: dequeue iopen holder in gfs2_inode_lookup error Before this patch, if function gfs2_inode_lookup encountered an error after it had locked the iopen glock, it never unlocked it, relying on the evict code to do the cleanup. The evict code then took the inode glock while holding the iopen glock, which violates the locking order. For example, (1) node A does a gfs2_inode_lookup that fails, leaving the iopen glock locked. (2) node B calls delete_work_func -> gfs2_lookup_by_inum -> gfs2_inode_lookup. It locks the inode glock and blocks trying to lock the iopen glock, which is held by node A. (3) node A eventually calls gfs2_evict_inode -> evict_should_delete. It blocks trying to lock the inode glock, which is now held by node B. This patch introduces error handling to function gfs2_inode_lookup so it properly dequeues held iopen glocks on errors. Signed-off-by: Bob Peterson Signed-off-by: Andreas Gruenbacher --- fs/gfs2/inode.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'fs/gfs2') diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c index 3130f85d2b3f..d8cd835c560a 100644 --- a/fs/gfs2/inode.c +++ b/fs/gfs2/inode.c @@ -225,6 +225,10 @@ struct inode *gfs2_inode_lookup(struct super_block *sb, unsigned int type, return inode; fail: + if (gfs2_holder_initialized(&ip->i_iopen_gh)) { + glock_clear_object(ip->i_iopen_gh.gh_gl, ip); + gfs2_glock_dq_uninit(&ip->i_iopen_gh); + } if (io_gl) gfs2_glock_put(io_gl); if (gfs2_holder_initialized(&i_gh)) -- cgit v1.2.3 From a739765cd8e69560d61d512e6847f6e24f8aea99 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Thu, 30 Sep 2021 07:48:29 -0500 Subject: gfs2: dump glocks from gfs2_consist_OBJ_i Before this patch, failed consistency checks printed out the object that failed, but not the object's glock. This patch makes it also print out the object glock so we can see the glock's holders and flags to aid with debugging. Signed-off-by: Bob Peterson Signed-off-by: Andreas Gruenbacher --- fs/gfs2/util.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'fs/gfs2') diff --git a/fs/gfs2/util.c b/fs/gfs2/util.c index cf345a86ef67..8241029a2a5d 100644 --- a/fs/gfs2/util.c +++ b/fs/gfs2/util.c @@ -454,6 +454,7 @@ void gfs2_consist_inode_i(struct gfs2_inode *ip, (unsigned long long)ip->i_no_formal_ino, (unsigned long long)ip->i_no_addr, function, file, line); + gfs2_dump_glock(NULL, ip->i_gl, 1); gfs2_withdraw(sdp); } @@ -475,6 +476,7 @@ void gfs2_consist_rgrpd_i(struct gfs2_rgrpd *rgd, " function = %s, file = %s, line = %u\n", (unsigned long long)rgd->rd_addr, function, file, line); + gfs2_dump_glock(NULL, rgd->rd_gl, 1); gfs2_withdraw(sdp); } -- cgit v1.2.3 From 3278b977c9c4c51a4e5e04fb40a991fb28edc2b7 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Wed, 29 Sep 2021 15:06:21 -0500 Subject: gfs2: change go_lock to go_instantiate Before this patch, the go_lock glock operations (glops) did not do any actual locking. They were used to instantiate objects, like reading in dinodes and rgrps from the media. This patch renames the functions to go_instantiate for clarity. Signed-off-by: Bob Peterson Signed-off-by: Andreas Gruenbacher --- fs/gfs2/glock.c | 4 ++-- fs/gfs2/glops.c | 8 ++++---- fs/gfs2/incore.h | 2 +- fs/gfs2/rgrp.c | 2 +- fs/gfs2/rgrp.h | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) (limited to 'fs/gfs2') diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c index 54a1ed373ff3..7053628d129e 100644 --- a/fs/gfs2/glock.c +++ b/fs/gfs2/glock.c @@ -501,11 +501,11 @@ restart: first_gh = gh; } if (gh->gh_list.prev == &gl->gl_holders && - glops->go_lock) { + glops->go_instantiate) { if (!(gh->gh_flags & GL_SKIP)) { spin_unlock(&gl->gl_lockref.lock); /* FIXME: eliminate this eventually */ - ret = glops->go_lock(gh); + ret = glops->go_instantiate(gh); spin_lock(&gl->gl_lockref.lock); if (ret) { if (ret == 1) diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c index 4b19f513570f..8452a83bd55a 100644 --- a/fs/gfs2/glops.c +++ b/fs/gfs2/glops.c @@ -482,13 +482,13 @@ int gfs2_inode_refresh(struct gfs2_inode *ip) } /** - * inode_go_lock - operation done after an inode lock is locked by a process + * inode_go_instantiate - read in an inode if necessary * @gh: The glock holder * * Returns: errno */ -static int inode_go_lock(struct gfs2_holder *gh) +static int inode_go_instantiate(struct gfs2_holder *gh) { struct gfs2_glock *gl = gh->gh_gl; struct gfs2_sbd *sdp = gl->gl_name.ln_sbd; @@ -740,7 +740,7 @@ const struct gfs2_glock_operations gfs2_inode_glops = { .go_sync = inode_go_sync, .go_inval = inode_go_inval, .go_demote_ok = inode_go_demote_ok, - .go_lock = inode_go_lock, + .go_instantiate = inode_go_instantiate, .go_dump = inode_go_dump, .go_type = LM_TYPE_INODE, .go_flags = GLOF_ASPACE | GLOF_LRU | GLOF_LVB, @@ -750,7 +750,7 @@ const struct gfs2_glock_operations gfs2_inode_glops = { const struct gfs2_glock_operations gfs2_rgrp_glops = { .go_sync = rgrp_go_sync, .go_inval = rgrp_go_inval, - .go_lock = gfs2_rgrp_go_lock, + .go_instantiate = gfs2_rgrp_go_instantiate, .go_dump = gfs2_rgrp_go_dump, .go_type = LM_TYPE_RGRP, .go_flags = GLOF_LVB, diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h index ca42d310fd4d..af632dc65231 100644 --- a/fs/gfs2/incore.h +++ b/fs/gfs2/incore.h @@ -220,7 +220,7 @@ struct gfs2_glock_operations { 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); + int (*go_instantiate) (struct gfs2_holder *gh); void (*go_dump)(struct seq_file *seq, struct gfs2_glock *gl, const char *fs_id_buf); void (*go_callback)(struct gfs2_glock *gl, bool remote); diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c index 96b2fbed6bf1..cdcbc822064d 100644 --- a/fs/gfs2/rgrp.c +++ b/fs/gfs2/rgrp.c @@ -1288,7 +1288,7 @@ static int update_rgrp_lvb(struct gfs2_rgrpd *rgd) return 0; } -int gfs2_rgrp_go_lock(struct gfs2_holder *gh) +int gfs2_rgrp_go_instantiate(struct gfs2_holder *gh) { struct gfs2_rgrpd *rgd = gh->gh_gl->gl_object; diff --git a/fs/gfs2/rgrp.h b/fs/gfs2/rgrp.h index a6855fd796e0..3e2ca1fb4305 100644 --- a/fs/gfs2/rgrp.h +++ b/fs/gfs2/rgrp.h @@ -31,7 +31,7 @@ extern struct gfs2_rgrpd *gfs2_rgrpd_get_next(struct gfs2_rgrpd *rgd); extern void gfs2_clear_rgrpd(struct gfs2_sbd *sdp); extern int gfs2_rindex_update(struct gfs2_sbd *sdp); extern void gfs2_free_clones(struct gfs2_rgrpd *rgd); -extern int gfs2_rgrp_go_lock(struct gfs2_holder *gh); +extern int gfs2_rgrp_go_instantiate(struct gfs2_holder *gh); extern void gfs2_rgrp_brelse(struct gfs2_rgrpd *rgd); extern struct gfs2_alloc *gfs2_alloc_get(struct gfs2_inode *ip); -- cgit v1.2.3 From d74d0ce5bcd61a1cadb8e6ae3ec23c29b2288b37 Mon Sep 17 00:00:00 2001 From: Andreas Gruenbacher Date: Tue, 5 Oct 2021 18:24:36 +0200 Subject: gfs2: Remove 'first' trace_gfs2_promote argument Remove the 'first' argument of trace_gfs2_promote: with GL_SKIP, the 'first' holder isn't the one that instantiates the glock (gl_instantiate), which is what the 'first' flag was apparently supposed to indicate. Signed-off-by: Andreas Gruenbacher --- fs/gfs2/glock.c | 4 ++-- fs/gfs2/trace_gfs2.h | 9 +++------ 2 files changed, 5 insertions(+), 8 deletions(-) (limited to 'fs/gfs2') diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c index 7053628d129e..a5e197423460 100644 --- a/fs/gfs2/glock.c +++ b/fs/gfs2/glock.c @@ -518,12 +518,12 @@ restart: } } set_bit(HIF_HOLDER, &gh->gh_iflags); - trace_gfs2_promote(gh, 1); + trace_gfs2_promote(gh); gfs2_holder_wake(gh); goto restart; } set_bit(HIF_HOLDER, &gh->gh_iflags); - trace_gfs2_promote(gh, 0); + trace_gfs2_promote(gh); gfs2_holder_wake(gh); continue; } diff --git a/fs/gfs2/trace_gfs2.h b/fs/gfs2/trace_gfs2.h index bd6c8e9e49db..a5deb9f86831 100644 --- a/fs/gfs2/trace_gfs2.h +++ b/fs/gfs2/trace_gfs2.h @@ -197,15 +197,14 @@ TRACE_EVENT(gfs2_demote_rq, /* Promotion/grant of a glock */ TRACE_EVENT(gfs2_promote, - TP_PROTO(const struct gfs2_holder *gh, int first), + TP_PROTO(const struct gfs2_holder *gh), - TP_ARGS(gh, first), + TP_ARGS(gh), TP_STRUCT__entry( __field( dev_t, dev ) __field( u64, glnum ) __field( u32, gltype ) - __field( int, first ) __field( u8, state ) ), @@ -213,14 +212,12 @@ TRACE_EVENT(gfs2_promote, __entry->dev = gh->gh_gl->gl_name.ln_sbd->sd_vfs->s_dev; __entry->glnum = gh->gh_gl->gl_name.ln_number; __entry->gltype = gh->gh_gl->gl_name.ln_type; - __entry->first = first; __entry->state = glock_trace_state(gh->gh_state); ), - TP_printk("%u,%u glock %u:%llu promote %s %s", + TP_printk("%u,%u glock %u:%llu promote %s", MAJOR(__entry->dev), MINOR(__entry->dev), __entry->gltype, (unsigned long long)__entry->glnum, - __entry->first ? "first": "other", glock_trace_name(__entry->state)) ); -- cgit v1.2.3 From 17a6eceeb1c54a0a4af5b03ccc1de7ab824408bc Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Wed, 6 Oct 2021 07:35:04 -0500 Subject: gfs2: re-factor function do_promote This patch simply re-factors function do_promote to reduce the indents. The logic should be unchanged. This makes future patches more readable. Signed-off-by: Bob Peterson Signed-off-by: Andreas Gruenbacher --- fs/gfs2/glock.c | 71 ++++++++++++++++++++++++++++----------------------------- 1 file changed, 35 insertions(+), 36 deletions(-) (limited to 'fs/gfs2') diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c index a5e197423460..f9aebeba7454 100644 --- a/fs/gfs2/glock.c +++ b/fs/gfs2/glock.c @@ -494,48 +494,47 @@ restart: list_for_each_entry_safe(gh, tmp, &gl->gl_holders, gh_list) { if (!test_bit(HIF_WAIT, &gh->gh_iflags)) continue; - if (may_grant(gl, first_gh, gh)) { - if (!incompat_holders_demoted) { - demote_incompat_holders(gl, first_gh); - incompat_holders_demoted = true; - first_gh = gh; - } - if (gh->gh_list.prev == &gl->gl_holders && - glops->go_instantiate) { - if (!(gh->gh_flags & GL_SKIP)) { - spin_unlock(&gl->gl_lockref.lock); - /* FIXME: eliminate this eventually */ - ret = glops->go_instantiate(gh); - spin_lock(&gl->gl_lockref.lock); - if (ret) { - if (ret == 1) - return 2; - gh->gh_error = ret; - list_del_init(&gh->gh_list); - trace_gfs2_glock_queue(gh, 0); - gfs2_holder_wake(gh); - goto restart; - } + if (!may_grant(gl, first_gh, gh)) { + /* + * If we get here, it means we may not grant this holder for + * some reason. If this holder is the head of the list, it + * means we have a blocked holder at the head, so return 1. + */ + if (gh->gh_list.prev == &gl->gl_holders) + return 1; + do_error(gl, 0); + break; + } + if (!incompat_holders_demoted) { + demote_incompat_holders(gl, first_gh); + incompat_holders_demoted = true; + first_gh = gh; + } + if (gh->gh_list.prev == &gl->gl_holders && + glops->go_instantiate) { + if (!(gh->gh_flags & GL_SKIP)) { + spin_unlock(&gl->gl_lockref.lock); + /* FIXME: eliminate this eventually */ + ret = glops->go_instantiate(gh); + spin_lock(&gl->gl_lockref.lock); + if (ret) { + if (ret == 1) + return 2; + gh->gh_error = ret; + list_del_init(&gh->gh_list); + trace_gfs2_glock_queue(gh, 0); + gfs2_holder_wake(gh); + goto restart; } - set_bit(HIF_HOLDER, &gh->gh_iflags); - trace_gfs2_promote(gh); - gfs2_holder_wake(gh); - goto restart; } set_bit(HIF_HOLDER, &gh->gh_iflags); trace_gfs2_promote(gh); gfs2_holder_wake(gh); - continue; + goto restart; } - /* - * If we get here, it means we may not grant this holder for - * some reason. If this holder is the head of the list, it - * means we have a blocked holder at the head, so return 1. - */ - if (gh->gh_list.prev == &gl->gl_holders) - return 1; - do_error(gl, 0); - break; + set_bit(HIF_HOLDER, &gh->gh_iflags); + trace_gfs2_promote(gh); + gfs2_holder_wake(gh); } return 0; } -- cgit v1.2.3 From 60d8bae9d16a52346ba738518bd43702d15eb834 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Wed, 6 Oct 2021 07:59:59 -0500 Subject: gfs2: further simplify do_promote This patch further simplifies function do_promote by eliminating some redundant code in favor of using a lock_released flag. This is just prep work for a future patch. Signed-off-by: Bob Peterson Signed-off-by: Andreas Gruenbacher --- fs/gfs2/glock.c | 43 +++++++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 20 deletions(-) (limited to 'fs/gfs2') diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c index f9aebeba7454..db213f0ef960 100644 --- a/fs/gfs2/glock.c +++ b/fs/gfs2/glock.c @@ -487,12 +487,14 @@ __acquires(&gl->gl_lockref.lock) const struct gfs2_glock_operations *glops = gl->gl_ops; struct gfs2_holder *gh, *tmp, *first_gh; bool incompat_holders_demoted = false; + bool lock_released; int ret; restart: first_gh = find_first_strong_holder(gl); list_for_each_entry_safe(gh, tmp, &gl->gl_holders, gh_list) { - if (!test_bit(HIF_WAIT, &gh->gh_iflags)) + lock_released = false; + if (test_bit(HIF_HOLDER, &gh->gh_iflags)) continue; if (!may_grant(gl, first_gh, gh)) { /* @@ -511,30 +513,31 @@ restart: first_gh = gh; } if (gh->gh_list.prev == &gl->gl_holders && - glops->go_instantiate) { - if (!(gh->gh_flags & GL_SKIP)) { - spin_unlock(&gl->gl_lockref.lock); - /* FIXME: eliminate this eventually */ - ret = glops->go_instantiate(gh); - spin_lock(&gl->gl_lockref.lock); - if (ret) { - if (ret == 1) - return 2; - gh->gh_error = ret; - list_del_init(&gh->gh_list); - trace_gfs2_glock_queue(gh, 0); - gfs2_holder_wake(gh); - goto restart; - } + !(gh->gh_flags & GL_SKIP) && glops->go_instantiate) { + lock_released = true; + spin_unlock(&gl->gl_lockref.lock); + ret = glops->go_instantiate(gh); + spin_lock(&gl->gl_lockref.lock); + if (ret) { + if (ret == 1) + return 2; + gh->gh_error = ret; + list_del_init(&gh->gh_list); + trace_gfs2_glock_queue(gh, 0); + gfs2_holder_wake(gh); + goto restart; } - set_bit(HIF_HOLDER, &gh->gh_iflags); - trace_gfs2_promote(gh); - gfs2_holder_wake(gh); - goto restart; } set_bit(HIF_HOLDER, &gh->gh_iflags); trace_gfs2_promote(gh); gfs2_holder_wake(gh); + /* + * If we released the gl_lockref.lock the holders list may have + * changed. For that reason, we start again at the start of + * the holders queue. + */ + if (lock_released) + goto restart; } return 0; } -- cgit v1.2.3 From e6f856008d2364a16610d6269b6b38503d5e41a4 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Wed, 6 Oct 2021 08:59:52 -0500 Subject: gfs2: split glock instantiation off from do_promote Before this patch, function do_promote had a section of code that did the actual instantiation. This patch splits that off into its own function, gfs2_instantiate, which prepares us for the next patch that will use that function. Signed-off-by: Bob Peterson Signed-off-by: Andreas Gruenbacher --- fs/gfs2/glock.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) (limited to 'fs/gfs2') diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c index db213f0ef960..75d54ed7e54e 100644 --- a/fs/gfs2/glock.c +++ b/fs/gfs2/glock.c @@ -472,6 +472,21 @@ find_first_strong_holder(struct gfs2_glock *gl) return NULL; } +/* + * gfs2_instantiate - Call the glops instantiate function + * @gl: The glock + * + * Returns: 0 if instantiate was successful, 2 if type specific operation is + * underway, or error. + */ +static int gfs2_instantiate(struct gfs2_holder *gh) +{ + struct gfs2_glock *gl = gh->gh_gl; + const struct gfs2_glock_operations *glops = gl->gl_ops; + + return glops->go_instantiate(gh); +} + /** * do_promote - promote as many requests as possible on the current queue * @gl: The glock @@ -484,7 +499,6 @@ static int do_promote(struct gfs2_glock *gl) __releases(&gl->gl_lockref.lock) __acquires(&gl->gl_lockref.lock) { - const struct gfs2_glock_operations *glops = gl->gl_ops; struct gfs2_holder *gh, *tmp, *first_gh; bool incompat_holders_demoted = false; bool lock_released; @@ -513,10 +527,10 @@ restart: first_gh = gh; } if (gh->gh_list.prev == &gl->gl_holders && - !(gh->gh_flags & GL_SKIP) && glops->go_instantiate) { + !(gh->gh_flags & GL_SKIP) && gl->gl_ops->go_instantiate) { lock_released = true; spin_unlock(&gl->gl_lockref.lock); - ret = glops->go_instantiate(gh); + ret = gfs2_instantiate(gh); spin_lock(&gl->gl_lockref.lock); if (ret) { if (ret == 1) -- cgit v1.2.3 From f2e70d8f2fdff0707b3f4de4ef87f93e4396320c Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Wed, 6 Oct 2021 09:29:18 -0500 Subject: gfs2: fix GL_SKIP node_scope problems Before this patch, when a glock was locked, the very first holder on the queue would unlock the lockref and call the go_instantiate glops function (if one existed), unless GL_SKIP was specified. When we introduced the new node-scope concept, we allowed multiple holders to lock glocks in EX mode and share the lock. But node-scope introduced a new problem: if the first holder has GL_SKIP and the next one does NOT, since it is not the first holder on the queue, the go_instantiate op was not called. Eventually the GL_SKIP holder may call the instantiate sub-function (e.g. gfs2_rgrp_bh_get) but there was still a window of time in which another non-GL_SKIP holder assumes the instantiate function had been called by the first holder. In the case of rgrp glocks, this led to a NULL pointer dereference on the buffer_heads. This patch tries to fix the problem by introducing two new glock flags: GLF_INSTANTIATE_NEEDED, which keeps track of when the instantiate function needs to be called to "fill in" or "read in" the object before it is referenced. GLF_INSTANTIATE_IN_PROG which is used to determine when a process is in the process of reading in the object. Whenever a function needs to reference the object, it checks the GLF_INSTANTIATE_NEEDED flag, and if set, it sets GLF_INSTANTIATE_IN_PROG and calls the glops "go_instantiate" function. As before, the gl_lockref spin_lock is unlocked during the IO operation, which may take a relatively long amount of time to complete. While unlocked, if another process determines go_instantiate is still needed, it sees GLF_INSTANTIATE_IN_PROG is set, and waits for the go_instantiate glop operation to be completed. Once GLF_INSTANTIATE_IN_PROG is cleared, it needs to check GLF_INSTANTIATE_NEEDED again because the other process's go_instantiate operation may not have been successful. Functions that previously called the instantiate sub-functions now call directly into gfs2_instantiate so the new bits are managed properly. Signed-off-by: Bob Peterson Signed-off-by: Andreas Gruenbacher --- fs/gfs2/glock.c | 42 ++++++++++++++++++++++++++++++++++++++---- fs/gfs2/glock.h | 1 + fs/gfs2/glops.c | 10 ++++++---- fs/gfs2/incore.h | 2 ++ fs/gfs2/inode.c | 3 ++- fs/gfs2/rgrp.c | 21 +++++++++++---------- fs/gfs2/super.c | 2 +- 7 files changed, 61 insertions(+), 20 deletions(-) (limited to 'fs/gfs2') diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c index 75d54ed7e54e..08073dcaf30e 100644 --- a/fs/gfs2/glock.c +++ b/fs/gfs2/glock.c @@ -479,12 +479,42 @@ find_first_strong_holder(struct gfs2_glock *gl) * Returns: 0 if instantiate was successful, 2 if type specific operation is * underway, or error. */ -static int gfs2_instantiate(struct gfs2_holder *gh) +int gfs2_instantiate(struct gfs2_holder *gh) { struct gfs2_glock *gl = gh->gh_gl; const struct gfs2_glock_operations *glops = gl->gl_ops; + int ret; + +again: + if (!test_bit(GLF_INSTANTIATE_NEEDED, &gl->gl_flags)) + return 0; - return glops->go_instantiate(gh); + /* + * Since we unlock the lockref lock, we set a flag to indicate + * instantiate is in progress. + */ + if (test_bit(GLF_INSTANTIATE_IN_PROG, &gl->gl_flags)) { + wait_on_bit(&gl->gl_flags, GLF_INSTANTIATE_IN_PROG, + TASK_UNINTERRUPTIBLE); + /* + * Here we just waited for a different instantiate to finish. + * But that may not have been successful, as when a process + * locks an inode glock _before_ it has an actual inode to + * instantiate into. So we check again. This process might + * have an inode to instantiate, so might be successful. + */ + goto again; + } + + set_bit(GLF_INSTANTIATE_IN_PROG, &gl->gl_flags); + + ret = glops->go_instantiate(gh); + if (!ret) + clear_bit(GLF_INSTANTIATE_NEEDED, &gl->gl_flags); + clear_bit(GLF_INSTANTIATE_IN_PROG, &gl->gl_flags); + smp_mb__after_atomic(); + wake_up_bit(&gl->gl_flags, GLF_INSTANTIATE_IN_PROG); + return ret; } /** @@ -526,7 +556,7 @@ restart: incompat_holders_demoted = true; first_gh = gh; } - if (gh->gh_list.prev == &gl->gl_holders && + if (test_bit(GLF_INSTANTIATE_NEEDED, &gl->gl_flags) && !(gh->gh_flags & GL_SKIP) && gl->gl_ops->go_instantiate) { lock_released = true; spin_unlock(&gl->gl_lockref.lock); @@ -1162,7 +1192,7 @@ int gfs2_glock_get(struct gfs2_sbd *sdp, u64 number, atomic_inc(&sdp->sd_glock_disposal); gl->gl_node.next = NULL; - gl->gl_flags = 0; + gl->gl_flags = glops->go_instantiate ? BIT(GLF_INSTANTIATE_NEEDED) : 0; gl->gl_name = name; lockdep_set_subclass(&gl->gl_lockref.lock, glops->go_subclass); gl->gl_lockref.count = 1; @@ -2326,6 +2356,10 @@ static const char *gflags2str(char *buf, const struct gfs2_glock *gl) *p++ = 'P'; if (test_bit(GLF_FREEING, gflags)) *p++ = 'x'; + if (test_bit(GLF_INSTANTIATE_NEEDED, gflags)) + *p++ = 'n'; + if (test_bit(GLF_INSTANTIATE_IN_PROG, gflags)) + *p++ = 'N'; *p = 0; return buf; } diff --git a/fs/gfs2/glock.h b/fs/gfs2/glock.h index a36104229d0d..4f8642301801 100644 --- a/fs/gfs2/glock.h +++ b/fs/gfs2/glock.h @@ -204,6 +204,7 @@ extern void gfs2_holder_reinit(unsigned int state, u16 flags, extern void gfs2_holder_uninit(struct gfs2_holder *gh); extern int gfs2_glock_nq(struct gfs2_holder *gh); extern int gfs2_glock_poll(struct gfs2_holder *gh); +extern int gfs2_instantiate(struct gfs2_holder *gh); extern int gfs2_glock_wait(struct gfs2_holder *gh); extern int gfs2_glock_async_wait(unsigned int num_gh, struct gfs2_holder *ghs); extern void gfs2_glock_dq(struct gfs2_holder *gh); diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c index 8452a83bd55a..e2656baf38a3 100644 --- a/fs/gfs2/glops.c +++ b/fs/gfs2/glops.c @@ -357,6 +357,7 @@ static void inode_go_inval(struct gfs2_glock *gl, int flags) truncate_inode_pages(mapping, 0); if (ip) { set_bit(GIF_INVALID, &ip->i_flags); + set_bit(GLF_INSTANTIATE_NEEDED, &gl->gl_flags); forget_all_cached_acls(&ip->i_inode); security_inode_invalidate_secctx(&ip->i_inode); gfs2_dir_hash_inval(ip); @@ -495,13 +496,13 @@ static int inode_go_instantiate(struct gfs2_holder *gh) struct gfs2_inode *ip = gl->gl_object; int error = 0; - if (!ip) - return 0; + if (!ip) /* no inode to populate - read it in later */ + goto out; if (test_bit(GIF_INVALID, &ip->i_flags)) { error = gfs2_inode_refresh(ip); if (error) - return error; + goto out; } if (gh->gh_state != LM_ST_DEFERRED) @@ -515,9 +516,10 @@ static int inode_go_instantiate(struct gfs2_holder *gh) list_add(&ip->i_trunc_list, &sdp->sd_trunc_list); spin_unlock(&sdp->sd_trunc_lock); wake_up(&sdp->sd_quota_wait); - return 1; + error = 1; } +out: return error; } diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h index af632dc65231..19a4c6132c67 100644 --- a/fs/gfs2/incore.h +++ b/fs/gfs2/incore.h @@ -316,6 +316,7 @@ struct gfs2_alloc_parms { enum { GLF_LOCK = 1, + GLF_INSTANTIATE_NEEDED = 2, /* needs instantiate */ GLF_DEMOTE = 3, GLF_PENDING_DEMOTE = 4, GLF_DEMOTE_IN_PROGRESS = 5, @@ -325,6 +326,7 @@ enum { GLF_REPLY_PENDING = 9, GLF_INITIAL = 10, GLF_FROZEN = 11, + GLF_INSTANTIATE_IN_PROG = 12, /* instantiate happening now */ GLF_LRU = 13, GLF_OBJECT = 14, /* Used only for tracing */ GLF_BLOCKING = 15, diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c index d8cd835c560a..940d3e37250d 100644 --- a/fs/gfs2/inode.c +++ b/fs/gfs2/inode.c @@ -183,6 +183,7 @@ struct inode *gfs2_inode_lookup(struct super_block *sb, unsigned int type, glock_set_object(ip->i_gl, ip); set_bit(GIF_INVALID, &ip->i_flags); + set_bit(GLF_INSTANTIATE_NEEDED, &ip->i_gl->gl_flags); error = gfs2_glock_nq_init(io_gl, LM_ST_SHARED, GL_EXACT, &ip->i_iopen_gh); if (unlikely(error)) goto fail; @@ -196,7 +197,7 @@ struct inode *gfs2_inode_lookup(struct super_block *sb, unsigned int type, if (type == DT_UNKNOWN) { /* Inode glock must be locked already */ - error = gfs2_inode_refresh(GFS2_I(inode)); + error = gfs2_instantiate(&i_gh); if (error) goto fail; } else { diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c index cdcbc822064d..5ee7da3a450e 100644 --- a/fs/gfs2/rgrp.c +++ b/fs/gfs2/rgrp.c @@ -1238,8 +1238,7 @@ static int gfs2_rgrp_bh_get(struct gfs2_rgrpd *rgd) rgd->rd_rgl->rl_unlinked = cpu_to_be32(count_unlinked(rgd)); gfs2_rgrp_ondisk2lvb(rgd->rd_rgl, rgd->rd_bits[0].bi_bh->b_data); - } - else if (sdp->sd_args.ar_rgrplvb) { + } else if (sdp->sd_args.ar_rgrplvb) { if (!gfs2_rgrp_lvb_valid(rgd)){ gfs2_consist_rgrpd(rgd); error = -EIO; @@ -1257,11 +1256,10 @@ fail: bi->bi_bh = NULL; gfs2_assert_warn(sdp, !bi->bi_clone); } - return error; } -static int update_rgrp_lvb(struct gfs2_rgrpd *rgd) +static int update_rgrp_lvb(struct gfs2_rgrpd *rgd, struct gfs2_holder *gh) { u32 rl_flags; @@ -1269,7 +1267,7 @@ static int update_rgrp_lvb(struct gfs2_rgrpd *rgd) return 0; if (cpu_to_be32(GFS2_MAGIC) != rgd->rd_rgl->rl_magic) - return gfs2_rgrp_bh_get(rgd); + return gfs2_instantiate(gh); rl_flags = be32_to_cpu(rgd->rd_rgl->rl_flags); rl_flags &= ~GFS2_RDF_MASK; @@ -1312,6 +1310,7 @@ void gfs2_rgrp_brelse(struct gfs2_rgrpd *rgd) bi->bi_bh = NULL; } } + set_bit(GLF_INSTANTIATE_NEEDED, &rgd->rd_gl->gl_flags); } int gfs2_rgrp_send_discards(struct gfs2_sbd *sdp, u64 offset, @@ -2110,7 +2109,8 @@ int gfs2_inplace_reserve(struct gfs2_inode *ip, struct gfs2_alloc_parms *ap) gfs2_rgrp_congested(rs->rs_rgd, loops)) goto skip_rgrp; if (sdp->sd_args.ar_rgrplvb) { - error = update_rgrp_lvb(rs->rs_rgd); + error = update_rgrp_lvb(rs->rs_rgd, + &ip->i_rgd_gh); if (unlikely(error)) { rgrp_unlock_local(rs->rs_rgd); gfs2_glock_dq_uninit(&ip->i_rgd_gh); @@ -2125,8 +2125,11 @@ int gfs2_inplace_reserve(struct gfs2_inode *ip, struct gfs2_alloc_parms *ap) (loops == 0 && target > rs->rs_rgd->rd_extfail_pt)) goto skip_rgrp; - if (sdp->sd_args.ar_rgrplvb) - gfs2_rgrp_bh_get(rs->rs_rgd); + if (sdp->sd_args.ar_rgrplvb) { + error = gfs2_instantiate(&ip->i_rgd_gh); + if (error) + goto skip_rgrp; + } /* Get a reservation if we don't already have one */ if (!gfs2_rs_active(rs)) @@ -2762,8 +2765,6 @@ void gfs2_rlist_free(struct gfs2_rgrp_list *rlist) void rgrp_lock_local(struct gfs2_rgrpd *rgd) { - GLOCK_BUG_ON(rgd->rd_gl, !gfs2_glock_is_held_excl(rgd->rd_gl) && - !test_bit(SDF_NORECOVERY, &rgd->rd_sbd->sd_flags)); mutex_lock(&rgd->rd_mutex); } diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c index 6e00d15ef0a8..26c726580041 100644 --- a/fs/gfs2/super.c +++ b/fs/gfs2/super.c @@ -1245,7 +1245,7 @@ static enum dinode_demise evict_should_delete(struct inode *inode, return SHOULD_NOT_DELETE_DINODE; if (test_bit(GIF_INVALID, &ip->i_flags)) { - ret = gfs2_inode_refresh(ip); + ret = gfs2_instantiate(gh); if (ret) return SHOULD_NOT_DELETE_DINODE; } -- cgit v1.2.3 From ec1d398dd780d42f84d58acdc6cca599d4a12c16 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Tue, 5 Oct 2021 09:10:51 -0500 Subject: gfs2: Eliminate GIF_INVALID flag With the addition of the new GLF_INSTANTIATE_NEEDED flag, the GIF_INVALID flag is now redundant. This patch removes it. Since inode_instantiate is only called when instantiation is needed, the check in inode_instantiate is removed too. Signed-off-by: Bob Peterson Signed-off-by: Andreas Gruenbacher --- fs/gfs2/glops.c | 11 +++-------- fs/gfs2/incore.h | 1 - fs/gfs2/inode.c | 1 - fs/gfs2/super.c | 2 +- 4 files changed, 4 insertions(+), 11 deletions(-) (limited to 'fs/gfs2') diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c index e2656baf38a3..0b6a59f71eef 100644 --- a/fs/gfs2/glops.c +++ b/fs/gfs2/glops.c @@ -356,7 +356,6 @@ static void inode_go_inval(struct gfs2_glock *gl, int flags) struct address_space *mapping = gfs2_glock2aspace(gl); truncate_inode_pages(mapping, 0); if (ip) { - set_bit(GIF_INVALID, &ip->i_flags); set_bit(GLF_INSTANTIATE_NEEDED, &gl->gl_flags); forget_all_cached_acls(&ip->i_inode); security_inode_invalidate_secctx(&ip->i_inode); @@ -477,8 +476,6 @@ int gfs2_inode_refresh(struct gfs2_inode *ip) error = gfs2_dinode_in(ip, dibh->b_data); brelse(dibh); - clear_bit(GIF_INVALID, &ip->i_flags); - return error; } @@ -499,11 +496,9 @@ static int inode_go_instantiate(struct gfs2_holder *gh) if (!ip) /* no inode to populate - read it in later */ goto out; - if (test_bit(GIF_INVALID, &ip->i_flags)) { - error = gfs2_inode_refresh(ip); - if (error) - goto out; - } + error = gfs2_inode_refresh(ip); + if (error) + goto out; if (gh->gh_state != LM_ST_DEFERRED) inode_dio_wait(&ip->i_inode); diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h index 19a4c6132c67..d5edc27d88b2 100644 --- a/fs/gfs2/incore.h +++ b/fs/gfs2/incore.h @@ -373,7 +373,6 @@ struct gfs2_glock { }; enum { - GIF_INVALID = 0, GIF_QD_LOCKED = 1, GIF_ALLOC_FAILED = 2, GIF_SW_PAGED = 3, diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c index 940d3e37250d..9c28b7090895 100644 --- a/fs/gfs2/inode.c +++ b/fs/gfs2/inode.c @@ -182,7 +182,6 @@ struct inode *gfs2_inode_lookup(struct super_block *sb, unsigned int type, } glock_set_object(ip->i_gl, ip); - set_bit(GIF_INVALID, &ip->i_flags); set_bit(GLF_INSTANTIATE_NEEDED, &ip->i_gl->gl_flags); error = gfs2_glock_nq_init(io_gl, LM_ST_SHARED, GL_EXACT, &ip->i_iopen_gh); if (unlikely(error)) diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c index 26c726580041..5b121371508a 100644 --- a/fs/gfs2/super.c +++ b/fs/gfs2/super.c @@ -1244,7 +1244,7 @@ static enum dinode_demise evict_should_delete(struct inode *inode, if (ret) return SHOULD_NOT_DELETE_DINODE; - if (test_bit(GIF_INVALID, &ip->i_flags)) { + if (test_bit(GLF_INSTANTIATE_NEEDED, &ip->i_gl->gl_flags)) { ret = gfs2_instantiate(gh); if (ret) return SHOULD_NOT_DELETE_DINODE; -- cgit v1.2.3 From 4b3113a2573168e382fb64397ba1481f3b2a1ad7 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Tue, 5 Oct 2021 09:28:17 -0500 Subject: gfs2: remove RDF_UPTODATE flag The new GLF_INSTANTIATE_NEEDED flag obsoletes the old rgrp flag GFS2_RDF_UPTODATE, so this patch replaces it like we did with inodes. Signed-off-by: Bob Peterson Signed-off-by: Andreas Gruenbacher --- fs/gfs2/glops.c | 2 +- fs/gfs2/incore.h | 1 - fs/gfs2/rgrp.c | 36 ++++++++++++++---------------------- 3 files changed, 15 insertions(+), 24 deletions(-) (limited to 'fs/gfs2') diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c index 0b6a59f71eef..650ad77c4d0b 100644 --- a/fs/gfs2/glops.c +++ b/fs/gfs2/glops.c @@ -228,7 +228,7 @@ static void rgrp_go_inval(struct gfs2_glock *gl, int flags) gfs2_rgrp_brelse(rgd); WARN_ON_ONCE(!(flags & DIO_METADATA)); truncate_inode_pages_range(mapping, start, end); - rgd->rd_flags &= ~GFS2_RDF_UPTODATE; + set_bit(GLF_INSTANTIATE_NEEDED, &gl->gl_flags); } static void gfs2_rgrp_go_dump(struct seq_file *seq, struct gfs2_glock *gl, diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h index d5edc27d88b2..8c00fb389ae5 100644 --- a/fs/gfs2/incore.h +++ b/fs/gfs2/incore.h @@ -119,7 +119,6 @@ struct gfs2_rgrpd { u32 rd_flags; u32 rd_extfail_pt; /* extent failure point */ #define GFS2_RDF_CHECK 0x10000000 /* check for unlinked inodes */ -#define GFS2_RDF_UPTODATE 0x20000000 /* rg is up to date */ #define GFS2_RDF_ERROR 0x40000000 /* error in rg */ #define GFS2_RDF_PREFERRED 0x80000000 /* This rgrp is preferred */ #define GFS2_RDF_MASK 0xf0000000 /* mask for internal flags */ diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c index 5ee7da3a450e..0fb3c01bc557 100644 --- a/fs/gfs2/rgrp.c +++ b/fs/gfs2/rgrp.c @@ -932,7 +932,7 @@ static int read_rindex_entry(struct gfs2_inode *ip) goto fail; rgd->rd_rgl = (struct gfs2_rgrp_lvb *)rgd->rd_gl->gl_lksb.sb_lvbptr; - rgd->rd_flags &= ~(GFS2_RDF_UPTODATE | GFS2_RDF_PREFERRED); + rgd->rd_flags &= ~GFS2_RDF_PREFERRED; if (rgd->rd_data > sdp->sd_max_rg_data) sdp->sd_max_rg_data = rgd->rd_data; spin_lock(&sdp->sd_rindex_spin); @@ -1185,8 +1185,8 @@ static void rgrp_set_bitmap_flags(struct gfs2_rgrpd *rgd) } /** - * gfs2_rgrp_bh_get - Read in a RG's header and bitmaps - * @rgd: the struct gfs2_rgrpd describing the RG to read in + * gfs2_rgrp_go_instantiate - Read in a RG's header and bitmaps + * @gh: the glock holder representing the rgrpd to read in * * Read in all of a Resource Group's header and bitmap blocks. * Caller must eventually call gfs2_rgrp_brelse() to free the bitmaps. @@ -1194,10 +1194,11 @@ static void rgrp_set_bitmap_flags(struct gfs2_rgrpd *rgd) * Returns: errno */ -static int gfs2_rgrp_bh_get(struct gfs2_rgrpd *rgd) +int gfs2_rgrp_go_instantiate(struct gfs2_holder *gh) { + struct gfs2_glock *gl = gh->gh_gl; + struct gfs2_rgrpd *rgd = gl->gl_object; struct gfs2_sbd *sdp = rgd->rd_sbd; - struct gfs2_glock *gl = rgd->rd_gl; unsigned int length = rgd->rd_length; struct gfs2_bitmap *bi; unsigned int x, y; @@ -1225,15 +1226,13 @@ static int gfs2_rgrp_bh_get(struct gfs2_rgrpd *rgd) } } - if (!(rgd->rd_flags & GFS2_RDF_UPTODATE)) { - gfs2_rgrp_in(rgd, (rgd->rd_bits[0].bi_bh)->b_data); - rgrp_set_bitmap_flags(rgd); - rgd->rd_flags |= (GFS2_RDF_UPTODATE | GFS2_RDF_CHECK); - rgd->rd_free_clone = rgd->rd_free; - GLOCK_BUG_ON(rgd->rd_gl, rgd->rd_reserved); - /* max out the rgrp allocation failure point */ - rgd->rd_extfail_pt = rgd->rd_free; - } + gfs2_rgrp_in(rgd, (rgd->rd_bits[0].bi_bh)->b_data); + rgrp_set_bitmap_flags(rgd); + rgd->rd_flags |= GFS2_RDF_CHECK; + rgd->rd_free_clone = rgd->rd_free; + GLOCK_BUG_ON(rgd->rd_gl, rgd->rd_reserved); + /* max out the rgrp allocation failure point */ + rgd->rd_extfail_pt = rgd->rd_free; if (cpu_to_be32(GFS2_MAGIC) != rgd->rd_rgl->rl_magic) { rgd->rd_rgl->rl_unlinked = cpu_to_be32(count_unlinked(rgd)); gfs2_rgrp_ondisk2lvb(rgd->rd_rgl, @@ -1263,7 +1262,7 @@ static int update_rgrp_lvb(struct gfs2_rgrpd *rgd, struct gfs2_holder *gh) { u32 rl_flags; - if (rgd->rd_flags & GFS2_RDF_UPTODATE) + if (!test_bit(GLF_INSTANTIATE_NEEDED, &gh->gh_gl->gl_flags)) return 0; if (cpu_to_be32(GFS2_MAGIC) != rgd->rd_rgl->rl_magic) @@ -1286,13 +1285,6 @@ static int update_rgrp_lvb(struct gfs2_rgrpd *rgd, struct gfs2_holder *gh) return 0; } -int gfs2_rgrp_go_instantiate(struct gfs2_holder *gh) -{ - struct gfs2_rgrpd *rgd = gh->gh_gl->gl_object; - - return gfs2_rgrp_bh_get(rgd); -} - /** * gfs2_rgrp_brelse - Release RG bitmaps read in with gfs2_rgrp_bh_get() * @rgd: The resource group -- cgit v1.2.3 From 8793e149859ab44d702ff72ed5e50645cec55966 Mon Sep 17 00:00:00 2001 From: Bob Peterson Date: Fri, 1 Oct 2021 13:00:21 -0500 Subject: gfs2: set glock object after nq Before this patch, function gfs2_create_inode called glock_set_object to set the gl_object for inode and iopen glocks before the glock was locked. That's wrong because other competing processes like evict may be blocked waiting for the glock and still have gl_object set before the actual eviction can take place. This patch moves the call to glock_set_object until after the glock is acquire in function gfs2_create_inode, so it waits for possibly competing evicts to finish their processing first. Signed-off-by: Bob Peterson Signed-off-by: Andreas Gruenbacher --- fs/gfs2/inode.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'fs/gfs2') diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c index 9c28b7090895..6424b903e885 100644 --- a/fs/gfs2/inode.c +++ b/fs/gfs2/inode.c @@ -731,18 +731,17 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry, if (error) goto fail_free_inode; flush_delayed_work(&ip->i_gl->gl_work); - glock_set_object(ip->i_gl, ip); error = gfs2_glock_get(sdp, ip->i_no_addr, &gfs2_iopen_glops, CREATE, &io_gl); if (error) goto fail_free_inode; gfs2_cancel_delete_work(io_gl); - glock_set_object(io_gl, ip); error = gfs2_glock_nq_init(ip->i_gl, LM_ST_EXCLUSIVE, GL_SKIP, ghs + 1); if (error) goto fail_gunlock2; + glock_set_object(ip->i_gl, ip); error = gfs2_trans_begin(sdp, blocks, 0); if (error) goto fail_gunlock2; @@ -758,6 +757,7 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry, if (error) goto fail_gunlock2; + glock_set_object(io_gl, ip); gfs2_set_iop(inode); insert_inode_hash(inode); -- cgit v1.2.3 From 486408d690e130c3adacf816754b97558d715f46 Mon Sep 17 00:00:00 2001 From: Andreas Gruenbacher Date: Mon, 11 Oct 2021 20:53:02 +0200 Subject: gfs2: Cancel remote delete work asynchronously In gfs2_inode_lookup and gfs2_create_inode, we're calling gfs2_cancel_delete_work which currently cancels any remote delete work (delete_work_func) synchronously. This means that if the work is currently running, it will wait for it to finish. We're doing this to pevent a previous instance of an inode from having any influence on the next instance. However, delete_work_func uses gfs2_inode_lookup internally, and we can end up in a deadlock when delete_work_func gets interrupted at the wrong time. For example, (1) An inode's iopen glock has delete work queued, but the inode itself has been evicted from the inode cache. (2) The delete work is preempted before reaching gfs2_inode_lookup. (3) Another process recreates the inode (gfs2_create_inode). It tries to cancel any outstanding delete work, which blocks waiting for the ongoing delete work to finish. (4) The delete work calls gfs2_inode_lookup, which blocks waiting for gfs2_create_inode to instantiate and unlock the new inode => deadlock. It turns out that when the delete work notices that its inode has been re-instantiated, it will do nothing. This means that it's safe to cancel the delete work asynchronously. This prevents the kind of deadlock described above. Signed-off-by: Andreas Gruenbacher Signed-off-by: Bob Peterson --- fs/gfs2/glock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/gfs2') diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c index 08073dcaf30e..190d7eb2d0ea 100644 --- a/fs/gfs2/glock.c +++ b/fs/gfs2/glock.c @@ -2127,7 +2127,7 @@ bool gfs2_queue_delete_work(struct gfs2_glock *gl, unsigned long delay) void gfs2_cancel_delete_work(struct gfs2_glock *gl) { - if (cancel_delayed_work_sync(&gl->gl_delete)) { + if (cancel_delayed_work(&gl->gl_delete)) { clear_bit(GLF_PENDING_DELETE, &gl->gl_flags); gfs2_glock_put(gl); } -- cgit v1.2.3 From 7427f3bb49d81525b7dd1d0f7c5f6bbc752e6f0e Mon Sep 17 00:00:00 2001 From: Andreas Gruenbacher Date: Thu, 7 Oct 2021 15:57:44 +0200 Subject: gfs2: Fix glock_hash_walk bugs So far, glock_hash_walk took a reference on each glock it iterated over, and it was the examiner's responsibility to drop those references. Dropping the final reference to a glock can sleep and the examiners are called in a RCU critical section with spin locks held, so examiners that didn't need the extra reference had to drop it asynchronously via gfs2_glock_queue_put or similar. This wasn't done correctly in thaw_glock which did call gfs2_glock_put, and not at all in dump_glock_func. Change glock_hash_walk to not take glock references at all. That way, the examiners that don't need them won't have to bother with slow asynchronous puts, and the examiners that do need references can take them themselves. Reported-by: Alexander Aring Signed-off-by: Andreas Gruenbacher --- fs/gfs2/glock.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) (limited to 'fs/gfs2') diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c index 190d7eb2d0ea..6e3bd5ab9108 100644 --- a/fs/gfs2/glock.c +++ b/fs/gfs2/glock.c @@ -2101,10 +2101,10 @@ static void glock_hash_walk(glock_examiner examiner, const struct gfs2_sbd *sdp) do { rhashtable_walk_start(&iter); - while ((gl = rhashtable_walk_next(&iter)) && !IS_ERR(gl)) - if (gl->gl_name.ln_sbd == sdp && - lockref_get_not_dead(&gl->gl_lockref)) + while ((gl = rhashtable_walk_next(&iter)) && !IS_ERR(gl)) { + if (gl->gl_name.ln_sbd == sdp) examiner(gl); + } rhashtable_walk_stop(&iter); } while (cond_resched(), gl == ERR_PTR(-EAGAIN)); @@ -2146,7 +2146,6 @@ static void flush_delete_work(struct gfs2_glock *gl) &gl->gl_delete, 0); } } - gfs2_glock_queue_work(gl, 0); } void gfs2_flush_delete_work(struct gfs2_sbd *sdp) @@ -2163,10 +2162,10 @@ void gfs2_flush_delete_work(struct gfs2_sbd *sdp) static void thaw_glock(struct gfs2_glock *gl) { - if (!test_and_clear_bit(GLF_FROZEN, &gl->gl_flags)) { - gfs2_glock_put(gl); + if (!test_and_clear_bit(GLF_FROZEN, &gl->gl_flags)) + return; + if (!lockref_get_not_dead(&gl->gl_lockref)) return; - } set_bit(GLF_REPLY_PENDING, &gl->gl_flags); gfs2_glock_queue_work(gl, 0); } @@ -2182,9 +2181,12 @@ static void clear_glock(struct gfs2_glock *gl) gfs2_glock_remove_from_lru(gl); spin_lock(&gl->gl_lockref.lock); - if (gl->gl_state != LM_ST_UNLOCKED) - handle_callback(gl, LM_ST_UNLOCKED, 0, false); - __gfs2_glock_queue_work(gl, 0); + if (!__lockref_is_dead(&gl->gl_lockref)) { + gl->gl_lockref.count++; + if (gl->gl_state != LM_ST_UNLOCKED) + handle_callback(gl, LM_ST_UNLOCKED, 0, false); + __gfs2_glock_queue_work(gl, 0); + } spin_unlock(&gl->gl_lockref.lock); } -- cgit v1.2.3 From 660a6126f8c3208f6df8d552039cda078a8426d1 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Wed, 13 Oct 2021 10:02:00 -0400 Subject: gfs2: check context in gfs2_glock_put Add a might_sleep call into gfs2_glock_put which can sleep in DLM when the last reference is released. This will show problems earlier, and not only when the last reference is put. Signed-off-by: Alexander Aring Signed-off-by: Andreas Gruenbacher --- fs/gfs2/glock.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'fs/gfs2') diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c index 6e3bd5ab9108..19f38aee1b61 100644 --- a/fs/gfs2/glock.c +++ b/fs/gfs2/glock.c @@ -301,6 +301,9 @@ void gfs2_glock_queue_put(struct gfs2_glock *gl) void gfs2_glock_put(struct gfs2_glock *gl) { + /* last put could call sleepable dlm api */ + might_sleep(); + if (lockref_put_or_lock(&gl->gl_lockref)) return; -- cgit v1.2.3 From e34e6f8133b8c783a44c235b843e9046e55adbf0 Mon Sep 17 00:00:00 2001 From: Tim Gardner Date: Mon, 20 Sep 2021 13:05:18 -0600 Subject: gfs2: Fix unused value warning in do_gfs2_set_flags() Coverity complains of an unused value: CID 119623 (#1 of 1): Unused value (UNUSED_VALUE) assigned_value: Assigning value -1 to error here, but that stored value is overwritten before it can be used. 237 error = -EPERM; Fix it by removing the assignment. Signed-off-by: Tim Gardner Signed-off-by: Andreas Gruenbacher --- fs/gfs2/file.c | 1 - 1 file changed, 1 deletion(-) (limited to 'fs/gfs2') diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c index 0f54d870e402..8267ab9b57de 100644 --- a/fs/gfs2/file.c +++ b/fs/gfs2/file.c @@ -234,7 +234,6 @@ static int do_gfs2_set_flags(struct inode *inode, u32 reqflags, u32 mask) if ((new_flags ^ flags) == 0) goto out; - error = -EPERM; if (!IS_IMMUTABLE(inode)) { error = gfs2_permission(&init_user_ns, inode, MAY_WRITE); if (error) -- cgit v1.2.3