From: Andreas Gruenbacher Date: Wed, 3 Apr 2024 05:09:30 +0000 (+0200) Subject: gfs2: Invert the GLF_INITIAL flag X-Git-Url: https://www.infradead.org/git/?a=commitdiff_plain;h=c8758ad005c98b15cd8b7a559dc51f8ddbc56d0a;p=users%2Fjedix%2Flinux-maple.git gfs2: Invert the GLF_INITIAL flag Invert the meaning of the GLF_INITIAL flag: right now, when GLF_INITIAL is set, a DLM lock exists and we have a valid identifier for it; when GLF_INITIAL is cleared, no DLM lock exists (yet). This is confusing. In addition, it makes more sense to highlight the exceptional case (i.e., no DLM lock exists yet) in glock dumps and trace points than to highlight the common case. To avoid confusion between the "old" and the "new" meaning of the flag, use 'a' instead of 'I' to represent the flag. For improved code consistency, check if the GLF_INITIAL flag is cleared to determine whether a DLM lock exists instead of checking if the lock identifier is non-zero. Document what the flag is used for. Signed-off-by: Andreas Gruenbacher --- diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c index ef4fbb197c99a..5fed5a22a8e75 100644 --- a/fs/gfs2/glock.c +++ b/fs/gfs2/glock.c @@ -1237,7 +1237,9 @@ int gfs2_glock_get(struct gfs2_sbd *sdp, u64 number, atomic_inc(&sdp->sd_glock_disposal); gl->gl_node.next = NULL; - gl->gl_flags = glops->go_instantiate ? BIT(GLF_INSTANTIATE_NEEDED) : 0; + gl->gl_flags = BIT(GLF_INITIAL); + if (glops->go_instantiate) + gl->gl_flags |= BIT(GLF_INSTANTIATE_NEEDED); gl->gl_name = name; lockdep_set_subclass(&gl->gl_lockref.lock, glops->go_subclass); gl->gl_lockref.count = 1; @@ -2363,7 +2365,7 @@ static const char *gflags2str(char *buf, const struct gfs2_glock *gl) if (test_bit(GLF_HAVE_REPLY, gflags)) *p++ = 'r'; if (test_bit(GLF_INITIAL, gflags)) - *p++ = 'I'; + *p++ = 'a'; if (test_bit(GLF_HAVE_FROZEN_REPLY, gflags)) *p++ = 'F'; if (!list_empty(&gl->gl_holders)) diff --git a/fs/gfs2/lock_dlm.c b/fs/gfs2/lock_dlm.c index cb2ad0031f9e9..fa5134df985f7 100644 --- a/fs/gfs2/lock_dlm.c +++ b/fs/gfs2/lock_dlm.c @@ -163,11 +163,21 @@ static void gdlm_ast(void *arg) BUG(); } - set_bit(GLF_INITIAL, &gl->gl_flags); + /* + * The GLF_INITIAL flag is initially set for new glocks. Upon the + * first successful new (non-conversion) request, we clear this flag to + * indicate that a DLM lock exists and that gl->gl_lksb.sb_lkid is the + * identifier to use for identifying it. + * + * Any failed initial requests do not create a DLM lock, so we ignore + * the gl->gl_lksb.sb_lkid values that come with such requests. + */ + + clear_bit(GLF_INITIAL, &gl->gl_flags); gfs2_glock_complete(gl, ret); return; out: - if (!test_bit(GLF_INITIAL, &gl->gl_flags)) + if (test_bit(GLF_INITIAL, &gl->gl_flags)) gl->gl_lksb.sb_lkid = 0; gfs2_glock_complete(gl, ret); } @@ -239,7 +249,7 @@ static u32 make_flags(struct gfs2_glock *gl, const unsigned int gfs_flags, BUG(); } - if (gl->gl_lksb.sb_lkid != 0) { + if (!test_bit(GLF_INITIAL, &gl->gl_flags)) { lkf |= DLM_LKF_CONVERT; if (test_bit(GLF_BLOCKING, &gl->gl_flags)) lkf |= DLM_LKF_QUECVT; @@ -270,14 +280,14 @@ static int gdlm_lock(struct gfs2_glock *gl, unsigned int req_state, lkf = make_flags(gl, flags, req); gfs2_glstats_inc(gl, GFS2_LKS_DCOUNT); gfs2_sbstats_inc(gl, GFS2_LKS_DCOUNT); - if (gl->gl_lksb.sb_lkid) { - gfs2_update_request_times(gl); - } else { + if (test_bit(GLF_INITIAL, &gl->gl_flags)) { memset(strname, ' ', GDLM_STRNAME_BYTES - 1); strname[GDLM_STRNAME_BYTES - 1] = '\0'; gfs2_reverse_hex(strname + 7, gl->gl_name.ln_type); gfs2_reverse_hex(strname + 23, gl->gl_name.ln_number); gl->gl_dstamp = ktime_get_real(); + } else { + gfs2_update_request_times(gl); } /* * Submit the actual lock request. @@ -301,7 +311,7 @@ static void gdlm_put_lock(struct gfs2_glock *gl) BUG_ON(!__lockref_is_dead(&gl->gl_lockref)); - if (gl->gl_lksb.sb_lkid == 0) { + if (test_bit(GLF_INITIAL, &gl->gl_flags)) { gfs2_glock_free(gl); return; } diff --git a/fs/gfs2/trace_gfs2.h b/fs/gfs2/trace_gfs2.h index 65748bfaef694..8eae8d62a4132 100644 --- a/fs/gfs2/trace_gfs2.h +++ b/fs/gfs2/trace_gfs2.h @@ -54,7 +54,7 @@ {(1UL << GLF_LFLUSH), "f" }, \ {(1UL << GLF_INVALIDATE_IN_PROGRESS), "i" }, \ {(1UL << GLF_HAVE_REPLY), "r" }, \ - {(1UL << GLF_INITIAL), "I" }, \ + {(1UL << GLF_INITIAL), "a" }, \ {(1UL << GLF_HAVE_FROZEN_REPLY), "F" }, \ {(1UL << GLF_LRU), "L" }, \ {(1UL << GLF_OBJECT), "o" }, \