gfs2.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/15] gfs2: Revise glock refcounting
@ 2024-05-29 17:03 Andreas Gruenbacher
  2024-05-29 17:03 ` [PATCH 01/15] gfs2: Remove unnecessary function prototype Andreas Gruenbacher
                   ` (14 more replies)
  0 siblings, 15 replies; 16+ messages in thread
From: Andreas Gruenbacher @ 2024-05-29 17:03 UTC (permalink / raw
  To: gfs2; +Cc: Andreas Gruenbacher

Here is a patch queue for the next (6.11) merge window.  After a number
of cleanups, the glock reference counting model is modified so that the
number of references no longer depends on whether or not the glock is
locked.  This allows to eliminate the go_demote_ok() glock operation.

Thanks,
Andreas

Andreas Gruenbacher (15):
  gfs2: Remove unnecessary function prototype
  gfs2: Remove useless return statement in run_queue
  gfs2: Rename GLF_FREEING to GLF_UNLOCKED
  gfs2: Rename GLF_REPLY_PENDING to GLF_HAVE_REPLY
  gfs2: Rename GLF_FROZEN to GLF_HAVE_FROZEN_REPLY
  gfs2: Rename handle_callback to request_demote
  gfs2: Update glocks documentation
  gfs2: Remove outdated comment in glock_work_func
  gfs2: Invert the GLF_INITIAL flag
  gfs2: gfs2_glock_get cleanup
  gfs2: Report when glocks cannot be freed for a long time
  gfs2: Switch to a per-filesystem glock workqueue
  gfs2: Revise glock reference counting model
  Revert "GFS2: Don't add all glocks to the lru"
  gfs2: Get rid of demote_ok checks

 Documentation/filesystems/gfs2-glocks.rst |  55 +++---
 fs/gfs2/glock.c                           | 215 ++++++++++------------
 fs/gfs2/glock.h                           |   1 -
 fs/gfs2/glops.c                           |  42 ++---
 fs/gfs2/incore.h                          |  11 +-
 fs/gfs2/lock_dlm.c                        |  28 ++-
 fs/gfs2/ops_fstype.c                      |  12 +-
 fs/gfs2/super.c                           |   1 -
 fs/gfs2/trace_gfs2.h                      |   6 +-
 fs/gfs2/util.c                            |   6 +-
 10 files changed, 171 insertions(+), 206 deletions(-)

-- 
2.45.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 01/15] gfs2: Remove unnecessary function prototype
  2024-05-29 17:03 [PATCH 00/15] gfs2: Revise glock refcounting Andreas Gruenbacher
@ 2024-05-29 17:03 ` Andreas Gruenbacher
  2024-05-29 17:03 ` [PATCH 02/15] gfs2: Remove useless return statement in run_queue Andreas Gruenbacher
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Andreas Gruenbacher @ 2024-05-29 17:03 UTC (permalink / raw
  To: gfs2; +Cc: Andreas Gruenbacher

Function __gfs2_glock_dq() gets defined before it is used, so there is
no need for a separate function declaration.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/glock.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 4ea6c8bfb4e6..873d76670238 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -61,7 +61,6 @@ struct gfs2_glock_iter {
 typedef void (*glock_examiner) (struct gfs2_glock * gl);
 
 static void do_xmote(struct gfs2_glock *gl, struct gfs2_holder *gh, unsigned int target);
-static void __gfs2_glock_dq(struct gfs2_holder *gh);
 static void handle_callback(struct gfs2_glock *gl, unsigned int state,
 			    unsigned long delay, bool remote);
 
-- 
2.45.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 02/15] gfs2: Remove useless return statement in run_queue
  2024-05-29 17:03 [PATCH 00/15] gfs2: Revise glock refcounting Andreas Gruenbacher
  2024-05-29 17:03 ` [PATCH 01/15] gfs2: Remove unnecessary function prototype Andreas Gruenbacher
@ 2024-05-29 17:03 ` Andreas Gruenbacher
  2024-05-29 17:03 ` [PATCH 03/15] gfs2: Rename GLF_FREEING to GLF_UNLOCKED Andreas Gruenbacher
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Andreas Gruenbacher @ 2024-05-29 17:03 UTC (permalink / raw
  To: gfs2; +Cc: Andreas Gruenbacher

The return statement at the end of run_queue() is useless.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/glock.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 873d76670238..d1fb0b2b5649 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -909,7 +909,6 @@ __acquires(&gl->gl_lockref.lock)
 out_unlock:
 	clear_bit(GLF_LOCK, &gl->gl_flags);
 	smp_mb__after_atomic();
-	return;
 }
 
 /**
-- 
2.45.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 03/15] gfs2: Rename GLF_FREEING to GLF_UNLOCKED
  2024-05-29 17:03 [PATCH 00/15] gfs2: Revise glock refcounting Andreas Gruenbacher
  2024-05-29 17:03 ` [PATCH 01/15] gfs2: Remove unnecessary function prototype Andreas Gruenbacher
  2024-05-29 17:03 ` [PATCH 02/15] gfs2: Remove useless return statement in run_queue Andreas Gruenbacher
@ 2024-05-29 17:03 ` Andreas Gruenbacher
  2024-05-29 17:03 ` [PATCH 04/15] gfs2: Rename GLF_REPLY_PENDING to GLF_HAVE_REPLY Andreas Gruenbacher
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Andreas Gruenbacher @ 2024-05-29 17:03 UTC (permalink / raw
  To: gfs2; +Cc: Andreas Gruenbacher

Rename the GLF_FREEING flag to GLF_UNLOCKED, and the ->go_free glock
operation to ->go_unlocked.  This mechanism is used to wait for the
underlying DLM lock to be unlocked; being able to free the glock is a
consequence of the DLM lock being unlocked.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/glock.c    |  2 +-
 fs/gfs2/glops.c    | 16 ++++++++--------
 fs/gfs2/incore.h   |  4 ++--
 fs/gfs2/lock_dlm.c |  4 ++--
 fs/gfs2/util.c     |  6 +++---
 5 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index d1fb0b2b5649..959668e22c9d 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -2378,7 +2378,7 @@ static const char *gflags2str(char *buf, const struct gfs2_glock *gl)
 		*p++ = 'o';
 	if (test_bit(GLF_BLOCKING, gflags))
 		*p++ = 'b';
-	if (test_bit(GLF_FREEING, gflags))
+	if (test_bit(GLF_UNLOCKED, gflags))
 		*p++ = 'x';
 	if (test_bit(GLF_INSTANTIATE_NEEDED, gflags))
 		*p++ = 'n';
diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
index 68677fb69a73..39bb6758a2a0 100644
--- a/fs/gfs2/glops.c
+++ b/fs/gfs2/glops.c
@@ -648,21 +648,21 @@ static void iopen_go_callback(struct gfs2_glock *gl, bool remote)
 }
 
 /**
- * inode_go_free - wake up anyone waiting for dlm's unlock ast to free it
- * @gl: glock being freed
+ * inode_go_unlocked - wake up anyone waiting for dlm's unlock ast
+ * @gl: glock being unlocked
  *
  * For now, this is only used for the journal inode glock. In withdraw
- * situations, we need to wait for the glock to be freed so that we know
+ * situations, we need to wait for the glock to be unlocked so that we know
  * other nodes may proceed with recovery / journal replay.
  */
-static void inode_go_free(struct gfs2_glock *gl)
+static void inode_go_unlocked(struct gfs2_glock *gl)
 {
 	/* Note that we cannot reference gl_object because it's already set
 	 * to NULL by this point in its lifecycle. */
-	if (!test_bit(GLF_FREEING, &gl->gl_flags))
+	if (!test_bit(GLF_UNLOCKED, &gl->gl_flags))
 		return;
-	clear_bit_unlock(GLF_FREEING, &gl->gl_flags);
-	wake_up_bit(&gl->gl_flags, GLF_FREEING);
+	clear_bit_unlock(GLF_UNLOCKED, &gl->gl_flags);
+	wake_up_bit(&gl->gl_flags, GLF_UNLOCKED);
 }
 
 /**
@@ -728,7 +728,7 @@ const struct gfs2_glock_operations gfs2_inode_glops = {
 	.go_dump = inode_go_dump,
 	.go_type = LM_TYPE_INODE,
 	.go_flags = GLOF_ASPACE | GLOF_LRU | GLOF_LVB,
-	.go_free = inode_go_free,
+	.go_unlocked = inode_go_unlocked,
 };
 
 const struct gfs2_glock_operations gfs2_rgrp_glops = {
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index 60abd7050c99..aa6dbde9cd19 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -224,7 +224,7 @@ struct gfs2_glock_operations {
 	void (*go_dump)(struct seq_file *seq, const struct gfs2_glock *gl,
 			const char *fs_id_buf);
 	void (*go_callback)(struct gfs2_glock *gl, bool remote);
-	void (*go_free)(struct gfs2_glock *gl);
+	void (*go_unlocked)(struct gfs2_glock *gl);
 	const int go_subclass;
 	const int go_type;
 	const unsigned long go_flags;
@@ -329,7 +329,7 @@ enum {
 	GLF_LRU				= 13,
 	GLF_OBJECT			= 14, /* Used only for tracing */
 	GLF_BLOCKING			= 15,
-	GLF_FREEING			= 16, /* Wait for glock to be freed */
+	GLF_UNLOCKED			= 16, /* Wait for glock to be unlocked */
 	GLF_TRY_TO_EVICT		= 17, /* iopen glocks only */
 	GLF_VERIFY_EVICT		= 18, /* iopen glocks only */
 };
diff --git a/fs/gfs2/lock_dlm.c b/fs/gfs2/lock_dlm.c
index 49059274a528..cb2ad0031f9e 100644
--- a/fs/gfs2/lock_dlm.c
+++ b/fs/gfs2/lock_dlm.c
@@ -134,8 +134,8 @@ static void gdlm_ast(void *arg)
 
 	switch (gl->gl_lksb.sb_status) {
 	case -DLM_EUNLOCK: /* Unlocked, so glock can be freed */
-		if (gl->gl_ops->go_free)
-			gl->gl_ops->go_free(gl);
+		if (gl->gl_ops->go_unlocked)
+			gl->gl_ops->go_unlocked(gl);
 		gfs2_glock_free(gl);
 		return;
 	case -DLM_ECANCEL: /* Cancel while getting lock */
diff --git a/fs/gfs2/util.c b/fs/gfs2/util.c
index af4758d8d894..7e79b1a9e35f 100644
--- a/fs/gfs2/util.c
+++ b/fs/gfs2/util.c
@@ -206,9 +206,9 @@ static void signal_our_withdraw(struct gfs2_sbd *sdp)
 	 * on other nodes to be successful, otherwise we remain the owner of
 	 * the glock as far as dlm is concerned.
 	 */
-	if (i_gl->gl_ops->go_free) {
-		set_bit(GLF_FREEING, &i_gl->gl_flags);
-		wait_on_bit(&i_gl->gl_flags, GLF_FREEING, TASK_UNINTERRUPTIBLE);
+	if (i_gl->gl_ops->go_unlocked) {
+		set_bit(GLF_UNLOCKED, &i_gl->gl_flags);
+		wait_on_bit(&i_gl->gl_flags, GLF_UNLOCKED, TASK_UNINTERRUPTIBLE);
 	}
 
 	/*
-- 
2.45.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 04/15] gfs2: Rename GLF_REPLY_PENDING to GLF_HAVE_REPLY
  2024-05-29 17:03 [PATCH 00/15] gfs2: Revise glock refcounting Andreas Gruenbacher
                   ` (2 preceding siblings ...)
  2024-05-29 17:03 ` [PATCH 03/15] gfs2: Rename GLF_FREEING to GLF_UNLOCKED Andreas Gruenbacher
@ 2024-05-29 17:03 ` Andreas Gruenbacher
  2024-05-29 17:03 ` [PATCH 05/15] gfs2: Rename GLF_FROZEN to GLF_HAVE_FROZEN_REPLY Andreas Gruenbacher
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Andreas Gruenbacher @ 2024-05-29 17:03 UTC (permalink / raw
  To: gfs2; +Cc: Andreas Gruenbacher

The GLF_REPLY_PENDING flag indicates to glock_work_func() that in
response to a locking request, DLM has sent a reply that needs to be
processed.  A flag with that name could as well indicate that we are
waiting on a reply from DLM, however.  To disambiguate these two cases,
rename the flag to GLF_HAVE_REPLY.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/glock.c      | 14 +++++++-------
 fs/gfs2/incore.h     |  2 +-
 fs/gfs2/trace_gfs2.h |  2 +-
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 959668e22c9d..34af0e7db98b 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -1109,8 +1109,8 @@ static void glock_work_func(struct work_struct *work)
 	unsigned int drop_refs = 1;
 
 	spin_lock(&gl->gl_lockref.lock);
-	if (test_bit(GLF_REPLY_PENDING, &gl->gl_flags)) {
-		clear_bit(GLF_REPLY_PENDING, &gl->gl_flags);
+	if (test_bit(GLF_HAVE_REPLY, &gl->gl_flags)) {
+		clear_bit(GLF_HAVE_REPLY, &gl->gl_flags);
 		finish_xmote(gl, gl->gl_reply);
 		drop_refs++;
 	}
@@ -1642,7 +1642,7 @@ int gfs2_glock_nq(struct gfs2_holder *gh)
 	add_to_queue(gh);
 	if (unlikely((LM_FLAG_NOEXP & gh->gh_flags) &&
 		     test_and_clear_bit(GLF_FROZEN, &gl->gl_flags))) {
-		set_bit(GLF_REPLY_PENDING, &gl->gl_flags);
+		set_bit(GLF_HAVE_REPLY, &gl->gl_flags);
 		gl->gl_lockref.count++;
 		gfs2_glock_queue_work(gl, 0);
 	}
@@ -1930,7 +1930,7 @@ void gfs2_glock_cb(struct gfs2_glock *gl, unsigned int state)
 	    gl->gl_name.ln_type == LM_TYPE_INODE) {
 		if (time_before(now, holdtime))
 			delay = holdtime - now;
-		if (test_bit(GLF_REPLY_PENDING, &gl->gl_flags))
+		if (test_bit(GLF_HAVE_REPLY, &gl->gl_flags))
 			delay = gl->gl_hold_time;
 	}
 	handle_callback(gl, state, delay, true);
@@ -1993,7 +1993,7 @@ void gfs2_glock_complete(struct gfs2_glock *gl, int ret)
 	}
 
 	gl->gl_lockref.count++;
-	set_bit(GLF_REPLY_PENDING, &gl->gl_flags);
+	set_bit(GLF_HAVE_REPLY, &gl->gl_flags);
 	gfs2_glock_queue_work(gl, 0);
 	spin_unlock(&gl->gl_lockref.lock);
 }
@@ -2186,7 +2186,7 @@ static void thaw_glock(struct gfs2_glock *gl)
 		return;
 
 	spin_lock(&gl->gl_lockref.lock);
-	set_bit(GLF_REPLY_PENDING, &gl->gl_flags);
+	set_bit(GLF_HAVE_REPLY, &gl->gl_flags);
 	gfs2_glock_queue_work(gl, 0);
 	spin_unlock(&gl->gl_lockref.lock);
 }
@@ -2364,7 +2364,7 @@ static const char *gflags2str(char *buf, const struct gfs2_glock *gl)
 		*p++ = 'f';
 	if (test_bit(GLF_INVALIDATE_IN_PROGRESS, gflags))
 		*p++ = 'i';
-	if (test_bit(GLF_REPLY_PENDING, gflags))
+	if (test_bit(GLF_HAVE_REPLY, gflags))
 		*p++ = 'r';
 	if (test_bit(GLF_INITIAL, gflags))
 		*p++ = 'I';
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index aa6dbde9cd19..e4423075433b 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -322,7 +322,7 @@ enum {
 	GLF_DIRTY			= 6,
 	GLF_LFLUSH			= 7,
 	GLF_INVALIDATE_IN_PROGRESS	= 8,
-	GLF_REPLY_PENDING		= 9,
+	GLF_HAVE_REPLY			= 9,
 	GLF_INITIAL			= 10,
 	GLF_FROZEN			= 11,
 	GLF_INSTANTIATE_IN_PROG		= 12, /* instantiate happening now */
diff --git a/fs/gfs2/trace_gfs2.h b/fs/gfs2/trace_gfs2.h
index a5deb9f86831..3721f0333dd7 100644
--- a/fs/gfs2/trace_gfs2.h
+++ b/fs/gfs2/trace_gfs2.h
@@ -53,7 +53,7 @@
 	{(1UL << GLF_DIRTY),			"y" },		\
 	{(1UL << GLF_LFLUSH),			"f" },		\
 	{(1UL << GLF_INVALIDATE_IN_PROGRESS),	"i" },		\
-	{(1UL << GLF_REPLY_PENDING),		"r" },		\
+	{(1UL << GLF_HAVE_REPLY),		"r" },		\
 	{(1UL << GLF_INITIAL),			"I" },		\
 	{(1UL << GLF_FROZEN),			"F" },		\
 	{(1UL << GLF_LRU),			"L" },		\
-- 
2.45.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 05/15] gfs2: Rename GLF_FROZEN to GLF_HAVE_FROZEN_REPLY
  2024-05-29 17:03 [PATCH 00/15] gfs2: Revise glock refcounting Andreas Gruenbacher
                   ` (3 preceding siblings ...)
  2024-05-29 17:03 ` [PATCH 04/15] gfs2: Rename GLF_REPLY_PENDING to GLF_HAVE_REPLY Andreas Gruenbacher
@ 2024-05-29 17:03 ` Andreas Gruenbacher
  2024-05-29 17:03 ` [PATCH 06/15] gfs2: Rename handle_callback to request_demote Andreas Gruenbacher
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Andreas Gruenbacher @ 2024-05-29 17:03 UTC (permalink / raw
  To: gfs2; +Cc: Andreas Gruenbacher

The GLF_FROZEN flag indicates that a reply to a DLM locking request has
been received, but should not be processed at this time.  To clarify
that meaning, rename the flag to GLF_HAVE_FROZEN_REPLY.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/glock.c      | 8 ++++----
 fs/gfs2/incore.h     | 2 +-
 fs/gfs2/trace_gfs2.h | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 34af0e7db98b..bac7078e4870 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -1641,7 +1641,7 @@ int gfs2_glock_nq(struct gfs2_holder *gh)
 	spin_lock(&gl->gl_lockref.lock);
 	add_to_queue(gh);
 	if (unlikely((LM_FLAG_NOEXP & gh->gh_flags) &&
-		     test_and_clear_bit(GLF_FROZEN, &gl->gl_flags))) {
+		     test_and_clear_bit(GLF_HAVE_FROZEN_REPLY, &gl->gl_flags))) {
 		set_bit(GLF_HAVE_REPLY, &gl->gl_flags);
 		gl->gl_lockref.count++;
 		gfs2_glock_queue_work(gl, 0);
@@ -1986,7 +1986,7 @@ void gfs2_glock_complete(struct gfs2_glock *gl, int ret)
 
 	if (unlikely(test_bit(DFL_BLOCK_LOCKS, &ls->ls_recover_flags))) {
 		if (gfs2_should_freeze(gl)) {
-			set_bit(GLF_FROZEN, &gl->gl_flags);
+			set_bit(GLF_HAVE_FROZEN_REPLY, &gl->gl_flags);
 			spin_unlock(&gl->gl_lockref.lock);
 			return;
 		}
@@ -2180,7 +2180,7 @@ 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))
+	if (!test_and_clear_bit(GLF_HAVE_FROZEN_REPLY, &gl->gl_flags))
 		return;
 	if (!lockref_get_not_dead(&gl->gl_lockref))
 		return;
@@ -2368,7 +2368,7 @@ static const char *gflags2str(char *buf, const struct gfs2_glock *gl)
 		*p++ = 'r';
 	if (test_bit(GLF_INITIAL, gflags))
 		*p++ = 'I';
-	if (test_bit(GLF_FROZEN, gflags))
+	if (test_bit(GLF_HAVE_FROZEN_REPLY, gflags))
 		*p++ = 'F';
 	if (!list_empty(&gl->gl_holders))
 		*p++ = 'q';
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index e4423075433b..73f24b86338c 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -324,7 +324,7 @@ enum {
 	GLF_INVALIDATE_IN_PROGRESS	= 8,
 	GLF_HAVE_REPLY			= 9,
 	GLF_INITIAL			= 10,
-	GLF_FROZEN			= 11,
+	GLF_HAVE_FROZEN_REPLY		= 11,
 	GLF_INSTANTIATE_IN_PROG		= 12, /* instantiate happening now */
 	GLF_LRU				= 13,
 	GLF_OBJECT			= 14, /* Used only for tracing */
diff --git a/fs/gfs2/trace_gfs2.h b/fs/gfs2/trace_gfs2.h
index 3721f0333dd7..65748bfaef69 100644
--- a/fs/gfs2/trace_gfs2.h
+++ b/fs/gfs2/trace_gfs2.h
@@ -55,7 +55,7 @@
 	{(1UL << GLF_INVALIDATE_IN_PROGRESS),	"i" },		\
 	{(1UL << GLF_HAVE_REPLY),		"r" },		\
 	{(1UL << GLF_INITIAL),			"I" },		\
-	{(1UL << GLF_FROZEN),			"F" },		\
+	{(1UL << GLF_HAVE_FROZEN_REPLY),	"F" },		\
 	{(1UL << GLF_LRU),			"L" },		\
 	{(1UL << GLF_OBJECT),			"o" },		\
 	{(1UL << GLF_BLOCKING),			"b" })
-- 
2.45.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 06/15] gfs2: Rename handle_callback to request_demote
  2024-05-29 17:03 [PATCH 00/15] gfs2: Revise glock refcounting Andreas Gruenbacher
                   ` (4 preceding siblings ...)
  2024-05-29 17:03 ` [PATCH 05/15] gfs2: Rename GLF_FROZEN to GLF_HAVE_FROZEN_REPLY Andreas Gruenbacher
@ 2024-05-29 17:03 ` Andreas Gruenbacher
  2024-05-29 17:03 ` [PATCH 07/15] gfs2: Update glocks documentation Andreas Gruenbacher
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Andreas Gruenbacher @ 2024-05-29 17:03 UTC (permalink / raw
  To: gfs2; +Cc: Andreas Gruenbacher

Function handle_callback() is used to request a glock demote.  This
often happens in response to a conflicting remote locking request and
subsequent bast callback from DLM, but there are other reasons for
triggering a demote request as well, such as when trying to release a
glock in response to memory pressure.  To clarify that, rename the
function to request_demote().

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

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index bac7078e4870..c583ba05e8f5 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -61,8 +61,8 @@ struct gfs2_glock_iter {
 typedef void (*glock_examiner) (struct gfs2_glock * gl);
 
 static void do_xmote(struct gfs2_glock *gl, struct gfs2_holder *gh, unsigned int target);
-static void handle_callback(struct gfs2_glock *gl, unsigned int state,
-			    unsigned long delay, bool remote);
+static void request_demote(struct gfs2_glock *gl, unsigned int state,
+			   unsigned long delay, bool remote);
 
 static struct dentry *gfs2_root;
 static struct workqueue_struct *glock_workqueue;
@@ -811,7 +811,7 @@ __acquires(&gl->gl_lockref.lock)
 	    (target != LM_ST_UNLOCKED ||
 	     test_bit(SDF_WITHDRAW_RECOVERY, &sdp->sd_flags))) {
 		if (!is_system_glock(gl)) {
-			handle_callback(gl, LM_ST_UNLOCKED, 0, false); /* sets demote */
+			request_demote(gl, LM_ST_UNLOCKED, 0, false);
 			/*
 			 * Ordinarily, we would call dlm and its callback would call
 			 * finish_xmote, which would call state_change() to the new state.
@@ -1459,7 +1459,7 @@ int gfs2_glock_async_wait(unsigned int num_gh, struct gfs2_holder *ghs)
 }
 
 /**
- * handle_callback - process a demote request
+ * request_demote - process a demote request
  * @gl: the glock
  * @state: the state the caller wants us to change to
  * @delay: zero to demote immediately; otherwise pending demote
@@ -1469,8 +1469,8 @@ int gfs2_glock_async_wait(unsigned int num_gh, struct gfs2_holder *ghs)
  * practise: LM_ST_SHARED and LM_ST_UNLOCKED
  */
 
-static void handle_callback(struct gfs2_glock *gl, unsigned int state,
-			    unsigned long delay, bool remote)
+static void request_demote(struct gfs2_glock *gl, unsigned int state,
+			   unsigned long delay, bool remote)
 {
 	if (delay)
 		set_bit(GLF_PENDING_DEMOTE, &gl->gl_flags);
@@ -1686,7 +1686,7 @@ static void __gfs2_glock_dq(struct gfs2_holder *gh)
 	 * below.
 	 */
 	if (gh->gh_flags & GL_NOCACHE)
-		handle_callback(gl, LM_ST_UNLOCKED, 0, false);
+		request_demote(gl, LM_ST_UNLOCKED, 0, false);
 
 	list_del_init(&gh->gh_list);
 	clear_bit(HIF_HOLDER, &gh->gh_iflags);
@@ -1933,7 +1933,7 @@ void gfs2_glock_cb(struct gfs2_glock *gl, unsigned int state)
 		if (test_bit(GLF_HAVE_REPLY, &gl->gl_flags))
 			delay = gl->gl_hold_time;
 	}
-	handle_callback(gl, state, delay, true);
+	request_demote(gl, state, delay, true);
 	gfs2_glock_queue_work(gl, delay);
 	spin_unlock(&gl->gl_lockref.lock);
 }
@@ -2062,7 +2062,7 @@ __acquires(&lru_lock)
 		freed++;
 		gl->gl_lockref.count++;
 		if (demote_ok(gl))
-			handle_callback(gl, LM_ST_UNLOCKED, 0, false);
+			request_demote(gl, LM_ST_UNLOCKED, 0, false);
 		gfs2_glock_queue_work(gl, 0);
 		spin_unlock(&gl->gl_lockref.lock);
 		cond_resched_lock(&lru_lock);
@@ -2205,7 +2205,7 @@ static void clear_glock(struct gfs2_glock *gl)
 	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);
+			request_demote(gl, LM_ST_UNLOCKED, 0, false);
 		gfs2_glock_queue_work(gl, 0);
 	}
 	spin_unlock(&gl->gl_lockref.lock);
-- 
2.45.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 07/15] gfs2: Update glocks documentation
  2024-05-29 17:03 [PATCH 00/15] gfs2: Revise glock refcounting Andreas Gruenbacher
                   ` (5 preceding siblings ...)
  2024-05-29 17:03 ` [PATCH 06/15] gfs2: Rename handle_callback to request_demote Andreas Gruenbacher
@ 2024-05-29 17:03 ` Andreas Gruenbacher
  2024-05-29 17:03 ` [PATCH 08/15] gfs2: Remove outdated comment in glock_work_func Andreas Gruenbacher
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Andreas Gruenbacher @ 2024-05-29 17:03 UTC (permalink / raw
  To: gfs2; +Cc: Andreas Gruenbacher

Rearrange the table of locking modes and associated caching capability
to be in order of increasing caching capability.

Update the description of the glock operations.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 Documentation/filesystems/gfs2-glocks.rst | 52 +++++++++++------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/Documentation/filesystems/gfs2-glocks.rst b/Documentation/filesystems/gfs2-glocks.rst
index 8a5842929b60..17ce3413608a 100644
--- a/Documentation/filesystems/gfs2-glocks.rst
+++ b/Documentation/filesystems/gfs2-glocks.rst
@@ -40,14 +40,14 @@ shared lock mode, SH. In GFS2 the DF mode is used exclusively for direct I/O
 operations. The glocks are basically a lock plus some routines which deal
 with cache management. The following rules apply for the cache:
 
-==========      ==========   ==============   ==========   ==============
-Glock mode      Cache data   Cache Metadata   Dirty Data   Dirty Metadata
-==========      ==========   ==============   ==========   ==============
-    UN             No              No             No            No
-    SH             Yes             Yes            No            No
-    DF             No              Yes            No            No
-    EX             Yes             Yes            Yes           Yes
-==========      ==========   ==============   ==========   ==============
+==========      ==============   ==========   ==========   ==============
+Glock mode      Cache Metadata   Cache data   Dirty Data   Dirty Metadata
+==========      ==============   ==========   ==========   ==============
+    UN                No            No            No            No
+    DF                Yes           No            No            No
+    SH                Yes           Yes           No            No
+    EX                Yes           Yes           Yes           Yes
+==========      ==============   ==========   ==========   ==============
 
 These rules are implemented using the various glock operations which
 are defined for each type of glock. Not all types of glocks use
@@ -55,23 +55,24 @@ all the modes. Only inode glocks use the DF mode for example.
 
 Table of glock operations and per type constants:
 
-=============      =============================================================
+==============     =============================================================
 Field              Purpose
-=============      =============================================================
-go_xmote_th        Called before remote state change (e.g. to sync dirty data)
+==============     =============================================================
+go_sync            Called before remote state change (e.g. to sync dirty data)
 go_xmote_bh        Called after remote state change (e.g. to refill cache)
 go_inval           Called if remote state change requires invalidating the cache
 go_demote_ok       Returns boolean value of whether its ok to demote a glock
                    (e.g. checks timeout, and that there is no cached data)
-go_lock            Called for the first local holder of a lock
-go_unlock          Called on the final local unlock of a lock
+go_instantiate     Called when a glock has been acquired
+go_held            Called every time a glock holder is acquired
 go_dump            Called to print content of object for debugfs file, or on
                    error to dump glock to the log.
-go_type            The type of the glock, ``LM_TYPE_*``
 go_callback	   Called if the DLM sends a callback to drop this lock
+go_unlocked        Called when a glock is unlocked (dlm_unlock())
+go_type            The type of the glock, ``LM_TYPE_*``
 go_flags	   GLOF_ASPACE is set, if the glock has an address space
                    associated with it
-=============      =============================================================
+==============     =============================================================
 
 The minimum hold time for each lock is the time after a remote lock
 grant for which we ignore remote demote requests. This is in order to
@@ -82,26 +83,25 @@ to by multiple nodes. By delaying the demotion in response to a
 remote callback, that gives the userspace program time to make
 some progress before the pages are unmapped.
 
-There is a plan to try and remove the go_lock and go_unlock callbacks
-if possible, in order to try and speed up the fast path though the locking.
-Also, eventually we hope to make the glock "EX" mode locally shared
-such that any local locking will be done with the i_mutex as required
-rather than via the glock.
+Eventually, we hope to make the glock "EX" mode locally shared such that any
+local locking will be done with the i_mutex as required rather than via the
+glock.
 
 Locking rules for glock operations:
 
-=============    ======================    =============================
+==============   ======================    =============================
 Operation        GLF_LOCK bit lock held    gl_lockref.lock spinlock held
-=============    ======================    =============================
-go_xmote_th           Yes                       No
+==============   ======================    =============================
+go_sync               Yes                       No
 go_xmote_bh           Yes                       No
 go_inval              Yes                       No
 go_demote_ok          Sometimes                 Yes
-go_lock               Yes                       No
-go_unlock             Yes                       No
+go_instantiate        No                        No
+go_held               No                        No
 go_dump               Sometimes                 Yes
 go_callback           Sometimes (N/A)           Yes
-=============    ======================    =============================
+go_unlocked           Yes                       No
+==============   ======================    =============================
 
 .. Note::
 
-- 
2.45.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 08/15] gfs2: Remove outdated comment in glock_work_func
  2024-05-29 17:03 [PATCH 00/15] gfs2: Revise glock refcounting Andreas Gruenbacher
                   ` (6 preceding siblings ...)
  2024-05-29 17:03 ` [PATCH 07/15] gfs2: Update glocks documentation Andreas Gruenbacher
@ 2024-05-29 17:03 ` Andreas Gruenbacher
  2024-05-29 17:03 ` [PATCH 09/15] gfs2: Invert the GLF_INITIAL flag Andreas Gruenbacher
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Andreas Gruenbacher @ 2024-05-29 17:03 UTC (permalink / raw
  To: gfs2; +Cc: Andreas Gruenbacher

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

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index c583ba05e8f5..ef4fbb197c99 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -1137,11 +1137,7 @@ static void glock_work_func(struct work_struct *work)
 		gfs2_glock_queue_work(gl, delay);
 	}
 
-	/*
-	 * Drop the remaining glock references manually here. (Mind that
-	 * gfs2_glock_queue_work depends on the lockref spinlock begin held
-	 * here as well.)
-	 */
+	/* Drop the remaining glock references manually. */
 	gl->gl_lockref.count -= drop_refs;
 	if (!gl->gl_lockref.count) {
 		__gfs2_glock_put(gl);
-- 
2.45.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 09/15] gfs2: Invert the GLF_INITIAL flag
  2024-05-29 17:03 [PATCH 00/15] gfs2: Revise glock refcounting Andreas Gruenbacher
                   ` (7 preceding siblings ...)
  2024-05-29 17:03 ` [PATCH 08/15] gfs2: Remove outdated comment in glock_work_func Andreas Gruenbacher
@ 2024-05-29 17:03 ` Andreas Gruenbacher
  2024-05-29 17:03 ` [PATCH 10/15] gfs2: gfs2_glock_get cleanup Andreas Gruenbacher
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Andreas Gruenbacher @ 2024-05-29 17:03 UTC (permalink / raw
  To: gfs2; +Cc: Andreas Gruenbacher

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 <agruenba@redhat.com>
---
 fs/gfs2/glock.c      |  6 ++++--
 fs/gfs2/lock_dlm.c   | 24 +++++++++++++++++-------
 fs/gfs2/trace_gfs2.h |  2 +-
 3 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index ef4fbb197c99..5fed5a22a8e7 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 cb2ad0031f9e..fa5134df985f 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 65748bfaef69..8eae8d62a413 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" },		\
-- 
2.45.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 10/15] gfs2: gfs2_glock_get cleanup
  2024-05-29 17:03 [PATCH 00/15] gfs2: Revise glock refcounting Andreas Gruenbacher
                   ` (8 preceding siblings ...)
  2024-05-29 17:03 ` [PATCH 09/15] gfs2: Invert the GLF_INITIAL flag Andreas Gruenbacher
@ 2024-05-29 17:03 ` Andreas Gruenbacher
  2024-05-29 17:03 ` [PATCH 11/15] gfs2: Report when glocks cannot be freed for a long time Andreas Gruenbacher
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Andreas Gruenbacher @ 2024-05-29 17:03 UTC (permalink / raw
  To: gfs2; +Cc: Andreas Gruenbacher

Clean up the messy code in gfs2_glock_get().  No change in
functionality.

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

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 5fed5a22a8e7..2d4e927c4d2f 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -1203,13 +1203,10 @@ int gfs2_glock_get(struct gfs2_sbd *sdp, u64 number,
 				    .ln_sbd = sdp };
 	struct gfs2_glock *gl, *tmp;
 	struct address_space *mapping;
-	int ret = 0;
 
 	gl = find_insert_glock(&name, NULL);
-	if (gl) {
-		*glp = gl;
-		return 0;
-	}
+	if (gl)
+		goto found;
 	if (!create)
 		return -ENOENT;
 
@@ -1271,23 +1268,19 @@ int gfs2_glock_get(struct gfs2_sbd *sdp, u64 number,
 	}
 
 	tmp = find_insert_glock(&name, gl);
-	if (!tmp) {
-		*glp = gl;
-		goto out;
-	}
-	if (IS_ERR(tmp)) {
-		ret = PTR_ERR(tmp);
-		goto out_free;
-	}
-	*glp = tmp;
+	if (tmp) {
+		gfs2_glock_dealloc(&gl->gl_rcu);
+		if (atomic_dec_and_test(&sdp->sd_glock_disposal))
+			wake_up(&sdp->sd_kill_wait);
 
-out_free:
-	gfs2_glock_dealloc(&gl->gl_rcu);
-	if (atomic_dec_and_test(&sdp->sd_glock_disposal))
-		wake_up(&sdp->sd_kill_wait);
+		if (IS_ERR(tmp))
+			return PTR_ERR(tmp);
+		gl = tmp;
+	}
 
-out:
-	return ret;
+found:
+	*glp = gl;
+	return 0;
 }
 
 /**
-- 
2.45.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 11/15] gfs2: Report when glocks cannot be freed for a long time
  2024-05-29 17:03 [PATCH 00/15] gfs2: Revise glock refcounting Andreas Gruenbacher
                   ` (9 preceding siblings ...)
  2024-05-29 17:03 ` [PATCH 10/15] gfs2: gfs2_glock_get cleanup Andreas Gruenbacher
@ 2024-05-29 17:03 ` Andreas Gruenbacher
  2024-05-29 17:03 ` [PATCH 12/15] gfs2: Switch to a per-filesystem glock workqueue Andreas Gruenbacher
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Andreas Gruenbacher @ 2024-05-29 17:03 UTC (permalink / raw
  To: gfs2; +Cc: Andreas Gruenbacher

When glocks cannot be freed for a long time, avoid the "task blocked for
more than N seconds" messages and report how many glocks are still
outstanding, instead.

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

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 2d4e927c4d2f..b819f8b8d98b 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -2248,13 +2248,25 @@ void gfs2_gl_dq_holders(struct gfs2_sbd *sdp)
 
 void gfs2_gl_hash_clear(struct gfs2_sbd *sdp)
 {
+	unsigned long start = jiffies;
+	bool timed_out = false;
+
 	set_bit(SDF_SKIP_DLM_UNLOCK, &sdp->sd_flags);
 	flush_workqueue(glock_workqueue);
 	glock_hash_walk(clear_glock, sdp);
 	flush_workqueue(glock_workqueue);
-	wait_event_timeout(sdp->sd_kill_wait,
-			   atomic_read(&sdp->sd_glock_disposal) == 0,
-			   HZ * 600);
+	while (!timed_out) {
+		wait_event_timeout(sdp->sd_kill_wait,
+				   !atomic_read(&sdp->sd_glock_disposal),
+				   HZ * 60);
+		if (!atomic_read(&sdp->sd_glock_disposal))
+			break;
+		timed_out = time_after(jiffies, start + (HZ * 600));
+		fs_warn(sdp, "%u glocks left after %u seconds%s\n",
+			atomic_read(&sdp->sd_glock_disposal),
+			jiffies_to_msecs(jiffies - start) / 1000,
+			timed_out ? ":" : "; still waiting");
+	}
 	gfs2_lm_unmount(sdp);
 	gfs2_free_dead_glocks(sdp);
 	glock_hash_walk(dump_glock_func, sdp);
-- 
2.45.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 12/15] gfs2: Switch to a per-filesystem glock workqueue
  2024-05-29 17:03 [PATCH 00/15] gfs2: Revise glock refcounting Andreas Gruenbacher
                   ` (10 preceding siblings ...)
  2024-05-29 17:03 ` [PATCH 11/15] gfs2: Report when glocks cannot be freed for a long time Andreas Gruenbacher
@ 2024-05-29 17:03 ` Andreas Gruenbacher
  2024-05-29 17:03 ` [PATCH 13/15] gfs2: Revise glock reference counting model Andreas Gruenbacher
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Andreas Gruenbacher @ 2024-05-29 17:03 UTC (permalink / raw
  To: gfs2; +Cc: Andreas Gruenbacher

Switch to a per-filesystem glock workqueue.  Additional workqueues are
cheap nowadays, and keeping separate workqueues allows to flush the work
of each filesystem without affecting the others.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/glock.c      | 20 +++++++-------------
 fs/gfs2/incore.h     |  1 +
 fs/gfs2/ops_fstype.c | 12 ++++++++++--
 3 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index b819f8b8d98b..32991cb22023 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -65,7 +65,6 @@ static void request_demote(struct gfs2_glock *gl, unsigned int state,
 			   unsigned long delay, bool remote);
 
 static struct dentry *gfs2_root;
-static struct workqueue_struct *glock_workqueue;
 static LIST_HEAD(lru_list);
 static atomic_t lru_count = ATOMIC_INIT(0);
 static DEFINE_SPINLOCK(lru_lock);
@@ -274,7 +273,9 @@ static void gfs2_glock_remove_from_lru(struct gfs2_glock *gl)
  * work queue.
  */
 static void gfs2_glock_queue_work(struct gfs2_glock *gl, unsigned long delay) {
-	if (!queue_delayed_work(glock_workqueue, &gl->gl_work, delay)) {
+	struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
+
+	if (!queue_delayed_work(sdp->sd_glock_wq, &gl->gl_work, delay)) {
 		/*
 		 * We are holding the lockref spinlock, and the work was still
 		 * queued above.  The queued work (glock_work_func) takes that
@@ -2252,9 +2253,10 @@ void gfs2_gl_hash_clear(struct gfs2_sbd *sdp)
 	bool timed_out = false;
 
 	set_bit(SDF_SKIP_DLM_UNLOCK, &sdp->sd_flags);
-	flush_workqueue(glock_workqueue);
+	flush_workqueue(sdp->sd_glock_wq);
 	glock_hash_walk(clear_glock, sdp);
-	flush_workqueue(glock_workqueue);
+	flush_workqueue(sdp->sd_glock_wq);
+
 	while (!timed_out) {
 		wait_event_timeout(sdp->sd_kill_wait,
 				   !atomic_read(&sdp->sd_glock_disposal),
@@ -2270,6 +2272,7 @@ void gfs2_gl_hash_clear(struct gfs2_sbd *sdp)
 	gfs2_lm_unmount(sdp);
 	gfs2_free_dead_glocks(sdp);
 	glock_hash_walk(dump_glock_func, sdp);
+	destroy_workqueue(sdp->sd_glock_wq);
 }
 
 static const char *state2str(unsigned state)
@@ -2534,16 +2537,8 @@ int __init gfs2_glock_init(void)
 	if (ret < 0)
 		return ret;
 
-	glock_workqueue = alloc_workqueue("glock_workqueue", WQ_MEM_RECLAIM |
-					  WQ_HIGHPRI | WQ_FREEZABLE, 0);
-	if (!glock_workqueue) {
-		rhashtable_destroy(&gl_hash_table);
-		return -ENOMEM;
-	}
-
 	glock_shrinker = shrinker_alloc(0, "gfs2-glock");
 	if (!glock_shrinker) {
-		destroy_workqueue(glock_workqueue);
 		rhashtable_destroy(&gl_hash_table);
 		return -ENOMEM;
 	}
@@ -2563,7 +2558,6 @@ void gfs2_glock_exit(void)
 {
 	shrinker_free(glock_shrinker);
 	rhashtable_destroy(&gl_hash_table);
-	destroy_workqueue(glock_workqueue);
 }
 
 static void gfs2_glock_iter_next(struct gfs2_glock_iter *gi, loff_t n)
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index 73f24b86338c..5ee46af1f4bd 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -772,6 +772,7 @@ struct gfs2_sbd {
 
 	/* Workqueue stuff */
 
+	struct workqueue_struct *sd_glock_wq;
 	struct workqueue_struct *sd_delete_wq;
 
 	/* Daemon stuff */
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 05975ec76d35..0561edd6cc86 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -1188,11 +1188,17 @@ static int gfs2_fill_super(struct super_block *sb, struct fs_context *fc)
 
 	snprintf(sdp->sd_fsname, sizeof(sdp->sd_fsname), "%s", sdp->sd_table_name);
 
+	error = -ENOMEM;
+	sdp->sd_glock_wq = alloc_workqueue("gfs2-glock/%s",
+			WQ_MEM_RECLAIM | WQ_HIGHPRI | WQ_FREEZABLE, 0,
+			sdp->sd_fsname);
+	if (!sdp->sd_glock_wq)
+		goto fail_free;
+
 	sdp->sd_delete_wq = alloc_workqueue("gfs2-delete/%s",
 			WQ_MEM_RECLAIM | WQ_FREEZABLE, 0, sdp->sd_fsname);
-	error = -ENOMEM;
 	if (!sdp->sd_delete_wq)
-		goto fail_free;
+		goto fail_glock_wq;
 
 	error = gfs2_sys_fs_add(sdp);
 	if (error)
@@ -1301,6 +1307,8 @@ static int gfs2_fill_super(struct super_block *sb, struct fs_context *fc)
 	gfs2_sys_fs_del(sdp);
 fail_delete_wq:
 	destroy_workqueue(sdp->sd_delete_wq);
+fail_glock_wq:
+	destroy_workqueue(sdp->sd_glock_wq);
 fail_free:
 	free_sbd(sdp);
 	sb->s_fs_info = NULL;
-- 
2.45.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 13/15] gfs2: Revise glock reference counting model
  2024-05-29 17:03 [PATCH 00/15] gfs2: Revise glock refcounting Andreas Gruenbacher
                   ` (11 preceding siblings ...)
  2024-05-29 17:03 ` [PATCH 12/15] gfs2: Switch to a per-filesystem glock workqueue Andreas Gruenbacher
@ 2024-05-29 17:03 ` Andreas Gruenbacher
  2024-05-29 17:03 ` [PATCH 14/15] Revert "GFS2: Don't add all glocks to the lru" Andreas Gruenbacher
  2024-05-29 17:03 ` [PATCH 15/15] gfs2: Get rid of demote_ok checks Andreas Gruenbacher
  14 siblings, 0 replies; 16+ messages in thread
From: Andreas Gruenbacher @ 2024-05-29 17:03 UTC (permalink / raw
  To: gfs2; +Cc: Andreas Gruenbacher

In the current glock reference counting model, a bias of one is added to
a glock's refcount when it is locked (gl->gl_state != LM_ST_UNLOCKED).
A glock is removed from the lru_list when it is enqueued, and added back
when it is dequeued.  This isn't a very appropriate model because most
glocks are held for long periods of time (for example, the inode "owns"
references to its inode and iopen glocks as long as the inode is cached
even when the glock state changes to LM_ST_UNLOCKED), and they can only
be freed when they are no longer referenced, anyway.

Fix this by getting rid of the refcount bias for locked glocks.  That
way, we can use lockref_put_or_lock() to efficiently drop all but the
last glock reference, and put the glock onto the lru_list when the last
reference is dropped.  When find_insert_glock() returns a reference to a
cached glock, it removes the glock from the lru_list.

Dumping the "glocks" and "glstats" debugfs files also takes glock
references, but instead of removing the glocks from the lru_list in that
case as well, we leave them on the list.  This ensures that dumping
those files won't perturb the order of the glocks on the lru_list.

In addition, when the last reference to an *unlocked* glock is dropped,
we immediately free it; this preserves the preexisting behavior.  If it
later turns out that caching unlocked glocks is useful in some
situations, we can change the caching strategy.

It is currently unclear if a glock that has no active references can
have the GLF_LFLUSH flag set.  To make sure that such a glock won't
accidentally be evicted due to memory pressure, we add a GLF_LFLUSH
check to gfs2_dispose_glock_lru().

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/glock.c | 56 ++++++++++++++++++++++++++-----------------------
 fs/gfs2/glock.h |  1 -
 fs/gfs2/super.c |  1 -
 3 files changed, 30 insertions(+), 28 deletions(-)

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 32991cb22023..e2a72c21194a 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -237,7 +237,7 @@ static int demote_ok(const struct gfs2_glock *gl)
 }
 
 
-void gfs2_glock_add_to_lru(struct gfs2_glock *gl)
+static void gfs2_glock_add_to_lru(struct gfs2_glock *gl)
 {
 	if (!(gl->gl_ops->go_flags & GLOF_LRU))
 		return;
@@ -305,6 +305,20 @@ static void __gfs2_glock_put(struct gfs2_glock *gl)
 	sdp->sd_lockstruct.ls_ops->lm_put_lock(gl);
 }
 
+static bool __gfs2_glock_put_or_lock(struct gfs2_glock *gl)
+{
+	if (lockref_put_or_lock(&gl->gl_lockref))
+		return true;
+	GLOCK_BUG_ON(gl, gl->gl_lockref.count != 1);
+	if (gl->gl_state != LM_ST_UNLOCKED) {
+		gl->gl_lockref.count--;
+		gfs2_glock_add_to_lru(gl);
+		spin_unlock(&gl->gl_lockref.lock);
+		return true;
+	}
+	return false;
+}
+
 /**
  * gfs2_glock_put() - Decrement reference count on glock
  * @gl: The glock to put
@@ -313,7 +327,7 @@ static void __gfs2_glock_put(struct gfs2_glock *gl)
 
 void gfs2_glock_put(struct gfs2_glock *gl)
 {
-	if (lockref_put_or_lock(&gl->gl_lockref))
+	if (__gfs2_glock_put_or_lock(gl))
 		return;
 
 	__gfs2_glock_put(gl);
@@ -328,10 +342,9 @@ void gfs2_glock_put(struct gfs2_glock *gl)
  */
 void gfs2_glock_put_async(struct gfs2_glock *gl)
 {
-	if (lockref_put_or_lock(&gl->gl_lockref))
+	if (__gfs2_glock_put_or_lock(gl))
 		return;
 
-	GLOCK_BUG_ON(gl, gl->gl_lockref.count != 1);
 	gfs2_glock_queue_work(gl, 0);
 	spin_unlock(&gl->gl_lockref.lock);
 }
@@ -570,18 +583,6 @@ static inline struct gfs2_holder *find_last_waiter(const struct gfs2_glock *gl)
 
 static void state_change(struct gfs2_glock *gl, unsigned int new_state)
 {
-	int held1, held2;
-
-	held1 = (gl->gl_state != LM_ST_UNLOCKED);
-	held2 = (new_state != LM_ST_UNLOCKED);
-
-	if (held1 != held2) {
-		GLOCK_BUG_ON(gl, __lockref_is_dead(&gl->gl_lockref));
-		if (held2)
-			gl->gl_lockref.count++;
-		else
-			gl->gl_lockref.count--;
-	}
 	if (new_state != gl->gl_target)
 		/* shorten our minimum hold time */
 		gl->gl_hold_time = max(gl->gl_hold_time - GL_GLOCK_HOLD_DECR,
@@ -1139,10 +1140,14 @@ static void glock_work_func(struct work_struct *work)
 	}
 
 	/* Drop the remaining glock references manually. */
+	GLOCK_BUG_ON(gl, gl->gl_lockref.count < drop_refs);
 	gl->gl_lockref.count -= drop_refs;
 	if (!gl->gl_lockref.count) {
-		__gfs2_glock_put(gl);
-		return;
+		if (gl->gl_state == LM_ST_UNLOCKED) {
+			__gfs2_glock_put(gl);
+			return;
+		}
+		gfs2_glock_add_to_lru(gl);
 	}
 	spin_unlock(&gl->gl_lockref.lock);
 }
@@ -1178,6 +1183,8 @@ static struct gfs2_glock *find_insert_glock(struct lm_lockname *name,
 out:
 	rcu_read_unlock();
 	finish_wait(wq, &wait.wait);
+	if (gl)
+		gfs2_glock_remove_from_lru(gl);
 	return gl;
 }
 
@@ -1626,9 +1633,6 @@ int gfs2_glock_nq(struct gfs2_holder *gh)
 		return error;
 	}
 
-	if (test_bit(GLF_LRU, &gl->gl_flags))
-		gfs2_glock_remove_from_lru(gl);
-
 	gh->gh_error = 0;
 	spin_lock(&gl->gl_lockref.lock);
 	add_to_queue(gh);
@@ -1693,9 +1697,6 @@ static void __gfs2_glock_dq(struct gfs2_holder *gh)
 			fast_path = 1;
 	}
 
-	if (!test_bit(GLF_LFLUSH, &gl->gl_flags) && demote_ok(gl))
-		gfs2_glock_add_to_lru(gl);
-
 	if (unlikely(!fast_path)) {
 		gl->gl_lockref.count++;
 		if (test_bit(GLF_PENDING_DEMOTE, &gl->gl_flags) &&
@@ -2008,10 +2009,12 @@ static int glock_cmp(void *priv, const struct list_head *a,
 
 static bool can_free_glock(struct gfs2_glock *gl)
 {
-	bool held = gl->gl_state != LM_ST_UNLOCKED;
+	struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
 
 	return !test_bit(GLF_LOCK, &gl->gl_flags) &&
-	       gl->gl_lockref.count == held;
+	       !gl->gl_lockref.count &&
+	       (!test_bit(GLF_LFLUSH, &gl->gl_flags) ||
+		test_bit(SDF_KILL, &sdp->sd_flags));
 }
 
 /**
@@ -2177,6 +2180,7 @@ static void thaw_glock(struct gfs2_glock *gl)
 	if (!lockref_get_not_dead(&gl->gl_lockref))
 		return;
 
+	gfs2_glock_remove_from_lru(gl);
 	spin_lock(&gl->gl_lockref.lock);
 	set_bit(GLF_HAVE_REPLY, &gl->gl_flags);
 	gfs2_glock_queue_work(gl, 0);
diff --git a/fs/gfs2/glock.h b/fs/gfs2/glock.h
index 19aef6d53267..adf0091cc98f 100644
--- a/fs/gfs2/glock.h
+++ b/fs/gfs2/glock.h
@@ -250,7 +250,6 @@ void gfs2_flush_delete_work(struct gfs2_sbd *sdp);
 void gfs2_gl_hash_clear(struct gfs2_sbd *sdp);
 void gfs2_gl_dq_holders(struct gfs2_sbd *sdp);
 void gfs2_glock_thaw(struct gfs2_sbd *sdp);
-void gfs2_glock_add_to_lru(struct gfs2_glock *gl);
 void gfs2_glock_free(struct gfs2_glock *gl);
 void gfs2_glock_free_later(struct gfs2_glock *gl);
 
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 7a5aedfcd52a..6678060ed4d2 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -1524,7 +1524,6 @@ static void gfs2_evict_inode(struct inode *inode)
 	if (ip->i_gl) {
 		glock_clear_object(ip->i_gl, ip);
 		wait_on_bit_io(&ip->i_flags, GIF_GLOP_PENDING, TASK_UNINTERRUPTIBLE);
-		gfs2_glock_add_to_lru(ip->i_gl);
 		gfs2_glock_put_eventually(ip->i_gl);
 		rcu_assign_pointer(ip->i_gl, NULL);
 	}
-- 
2.45.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 14/15] Revert "GFS2: Don't add all glocks to the lru"
  2024-05-29 17:03 [PATCH 00/15] gfs2: Revise glock refcounting Andreas Gruenbacher
                   ` (12 preceding siblings ...)
  2024-05-29 17:03 ` [PATCH 13/15] gfs2: Revise glock reference counting model Andreas Gruenbacher
@ 2024-05-29 17:03 ` Andreas Gruenbacher
  2024-05-29 17:03 ` [PATCH 15/15] gfs2: Get rid of demote_ok checks Andreas Gruenbacher
  14 siblings, 0 replies; 16+ messages in thread
From: Andreas Gruenbacher @ 2024-05-29 17:03 UTC (permalink / raw
  To: gfs2; +Cc: Andreas Gruenbacher

This reverts commit e7ccaf5fe1590667b3fa2f8df5c5ec9ba0dc5b85.

Before commit e7ccaf5fe159, every time a resource group glock was
dequeued by gfs2_glock_dq(), it was added to the glock LRU list even
though the glock was still referenced by the resource group and could
never be evicted, anyway.  Commit e7ccaf5fe159 added a GLOF_LRU hack to
avoid that overhead for resource group glocks, and that hack was since
adopted for some other types of glocks as well.

We now no longer add glocks to the glock LRU list while they are still
referenced.  This solves the underlying problem, and obsoletes the
GLOF_LRU hack.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
(cherry picked from commit 3e5257c810cba91e274d07f3db5cf013c7c830be)
---
 fs/gfs2/glock.c  | 7 -------
 fs/gfs2/glops.c  | 8 ++++----
 fs/gfs2/incore.h | 1 -
 3 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index e2a72c21194a..19f8df91b72e 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -239,11 +239,7 @@ static int demote_ok(const struct gfs2_glock *gl)
 
 static void gfs2_glock_add_to_lru(struct gfs2_glock *gl)
 {
-	if (!(gl->gl_ops->go_flags & GLOF_LRU))
-		return;
-
 	spin_lock(&lru_lock);
-
 	list_move_tail(&gl->gl_lru, &lru_list);
 
 	if (!test_bit(GLF_LRU, &gl->gl_flags)) {
@@ -256,9 +252,6 @@ static void gfs2_glock_add_to_lru(struct gfs2_glock *gl)
 
 static void gfs2_glock_remove_from_lru(struct gfs2_glock *gl)
 {
-	if (!(gl->gl_ops->go_flags & GLOF_LRU))
-		return;
-
 	spin_lock(&lru_lock);
 	if (test_bit(GLF_LRU, &gl->gl_flags)) {
 		list_del_init(&gl->gl_lru);
diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
index 39bb6758a2a0..7bc7f6785abd 100644
--- a/fs/gfs2/glops.c
+++ b/fs/gfs2/glops.c
@@ -727,7 +727,7 @@ const struct gfs2_glock_operations gfs2_inode_glops = {
 	.go_held = inode_go_held,
 	.go_dump = inode_go_dump,
 	.go_type = LM_TYPE_INODE,
-	.go_flags = GLOF_ASPACE | GLOF_LRU | GLOF_LVB,
+	.go_flags = GLOF_ASPACE | GLOF_LVB,
 	.go_unlocked = inode_go_unlocked,
 };
 
@@ -751,13 +751,13 @@ const struct gfs2_glock_operations gfs2_iopen_glops = {
 	.go_type = LM_TYPE_IOPEN,
 	.go_callback = iopen_go_callback,
 	.go_dump = inode_go_dump,
-	.go_flags = GLOF_LRU | GLOF_NONDISK,
+	.go_flags = GLOF_NONDISK,
 	.go_subclass = 1,
 };
 
 const struct gfs2_glock_operations gfs2_flock_glops = {
 	.go_type = LM_TYPE_FLOCK,
-	.go_flags = GLOF_LRU | GLOF_NONDISK,
+	.go_flags = GLOF_NONDISK,
 };
 
 const struct gfs2_glock_operations gfs2_nondisk_glops = {
@@ -768,7 +768,7 @@ const struct gfs2_glock_operations gfs2_nondisk_glops = {
 
 const struct gfs2_glock_operations gfs2_quota_glops = {
 	.go_type = LM_TYPE_QUOTA,
-	.go_flags = GLOF_LVB | GLOF_LRU | GLOF_NONDISK,
+	.go_flags = GLOF_LVB | GLOF_NONDISK,
 };
 
 const struct gfs2_glock_operations gfs2_journal_glops = {
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index 5ee46af1f4bd..f75982d45635 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -230,7 +230,6 @@ struct gfs2_glock_operations {
 	const unsigned long go_flags;
 #define GLOF_ASPACE 1 /* address space attached */
 #define GLOF_LVB    2 /* Lock Value Block attached */
-#define GLOF_LRU    4 /* LRU managed */
 #define GLOF_NONDISK   8 /* not I/O related */
 };
 
-- 
2.45.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 15/15] gfs2: Get rid of demote_ok checks
  2024-05-29 17:03 [PATCH 00/15] gfs2: Revise glock refcounting Andreas Gruenbacher
                   ` (13 preceding siblings ...)
  2024-05-29 17:03 ` [PATCH 14/15] Revert "GFS2: Don't add all glocks to the lru" Andreas Gruenbacher
@ 2024-05-29 17:03 ` Andreas Gruenbacher
  14 siblings, 0 replies; 16+ messages in thread
From: Andreas Gruenbacher @ 2024-05-29 17:03 UTC (permalink / raw
  To: gfs2; +Cc: Andreas Gruenbacher

The demote_ok glock operation is only still used to prevent the inode
glocks of the "jindex" and "rindex" directories from getting recycled
while they are still referenced by sdp->sd_jindex and sdp->sd_rindex.
However, the LRU walking code will no longer recycle glocks which are
referenced, so the demote_ok glock operation is obsolete and can be
removed.

Each of a glock's holders in the gl_holders list is holding a reference
on the glock, so when the list of holders isn't empty in demote_ok(),
the existing reference count check will already prevent the glock from
getting released.  This means that demote_ok() is obsolete as well.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 Documentation/filesystems/gfs2-glocks.rst |  3 ---
 fs/gfs2/glock.c                           | 23 +----------------------
 fs/gfs2/glops.c                           | 18 ------------------
 fs/gfs2/incore.h                          |  1 -
 4 files changed, 1 insertion(+), 44 deletions(-)

diff --git a/Documentation/filesystems/gfs2-glocks.rst b/Documentation/filesystems/gfs2-glocks.rst
index 17ce3413608a..adc0d4c4d979 100644
--- a/Documentation/filesystems/gfs2-glocks.rst
+++ b/Documentation/filesystems/gfs2-glocks.rst
@@ -61,8 +61,6 @@ Field              Purpose
 go_sync            Called before remote state change (e.g. to sync dirty data)
 go_xmote_bh        Called after remote state change (e.g. to refill cache)
 go_inval           Called if remote state change requires invalidating the cache
-go_demote_ok       Returns boolean value of whether its ok to demote a glock
-                   (e.g. checks timeout, and that there is no cached data)
 go_instantiate     Called when a glock has been acquired
 go_held            Called every time a glock holder is acquired
 go_dump            Called to print content of object for debugfs file, or on
@@ -95,7 +93,6 @@ Operation        GLF_LOCK bit lock held    gl_lockref.lock spinlock held
 go_sync               Yes                       No
 go_xmote_bh           Yes                       No
 go_inval              Yes                       No
-go_demote_ok          Sometimes                 Yes
 go_instantiate        No                        No
 go_held               No                        No
 go_dump               Sometimes                 Yes
diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 19f8df91b72e..7f68e3d217e6 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -216,27 +216,6 @@ struct gfs2_glock *gfs2_glock_hold(struct gfs2_glock *gl)
 	return gl;
 }
 
-/**
- * demote_ok - Check to see if it's ok to unlock a glock
- * @gl: the glock
- *
- * Returns: 1 if it's ok
- */
-
-static int demote_ok(const struct gfs2_glock *gl)
-{
-	const struct gfs2_glock_operations *glops = gl->gl_ops;
-
-	if (gl->gl_state == LM_ST_UNLOCKED)
-		return 0;
-	if (!list_empty(&gl->gl_holders))
-		return 0;
-	if (glops->go_demote_ok)
-		return glops->go_demote_ok(gl);
-	return 1;
-}
-
-
 static void gfs2_glock_add_to_lru(struct gfs2_glock *gl)
 {
 	spin_lock(&lru_lock);
@@ -2049,7 +2028,7 @@ __acquires(&lru_lock)
 		clear_bit(GLF_LRU, &gl->gl_flags);
 		freed++;
 		gl->gl_lockref.count++;
-		if (demote_ok(gl))
+		if (gl->gl_state != LM_ST_UNLOCKED)
 			request_demote(gl, LM_ST_UNLOCKED, 0, false);
 		gfs2_glock_queue_work(gl, 0);
 		spin_unlock(&gl->gl_lockref.lock);
diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
index 7bc7f6785abd..95d8081681dc 100644
--- a/fs/gfs2/glops.c
+++ b/fs/gfs2/glops.c
@@ -385,23 +385,6 @@ static void inode_go_inval(struct gfs2_glock *gl, int flags)
 	gfs2_clear_glop_pending(ip);
 }
 
-/**
- * inode_go_demote_ok - Check to see if it's ok to unlock an inode glock
- * @gl: the glock
- *
- * Returns: 1 if it's ok
- */
-
-static int inode_go_demote_ok(const struct gfs2_glock *gl)
-{
-	struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
-
-	if (sdp->sd_jindex == gl->gl_object || sdp->sd_rindex == gl->gl_object)
-		return 0;
-
-	return 1;
-}
-
 static int gfs2_dinode_in(struct gfs2_inode *ip, const void *buf)
 {
 	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
@@ -722,7 +705,6 @@ const struct gfs2_glock_operations gfs2_meta_glops = {
 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_instantiate = inode_go_instantiate,
 	.go_held = inode_go_held,
 	.go_dump = inode_go_dump,
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index f75982d45635..90fd2584357a 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -218,7 +218,6 @@ struct gfs2_glock_operations {
 	int (*go_sync) (struct gfs2_glock *gl);
 	int (*go_xmote_bh)(struct gfs2_glock *gl);
 	void (*go_inval) (struct gfs2_glock *gl, int flags);
-	int (*go_demote_ok) (const struct gfs2_glock *gl);
 	int (*go_instantiate) (struct gfs2_glock *gl);
 	int (*go_held)(struct gfs2_holder *gh);
 	void (*go_dump)(struct seq_file *seq, const struct gfs2_glock *gl,
-- 
2.45.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2024-05-29 17:04 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-29 17:03 [PATCH 00/15] gfs2: Revise glock refcounting Andreas Gruenbacher
2024-05-29 17:03 ` [PATCH 01/15] gfs2: Remove unnecessary function prototype Andreas Gruenbacher
2024-05-29 17:03 ` [PATCH 02/15] gfs2: Remove useless return statement in run_queue Andreas Gruenbacher
2024-05-29 17:03 ` [PATCH 03/15] gfs2: Rename GLF_FREEING to GLF_UNLOCKED Andreas Gruenbacher
2024-05-29 17:03 ` [PATCH 04/15] gfs2: Rename GLF_REPLY_PENDING to GLF_HAVE_REPLY Andreas Gruenbacher
2024-05-29 17:03 ` [PATCH 05/15] gfs2: Rename GLF_FROZEN to GLF_HAVE_FROZEN_REPLY Andreas Gruenbacher
2024-05-29 17:03 ` [PATCH 06/15] gfs2: Rename handle_callback to request_demote Andreas Gruenbacher
2024-05-29 17:03 ` [PATCH 07/15] gfs2: Update glocks documentation Andreas Gruenbacher
2024-05-29 17:03 ` [PATCH 08/15] gfs2: Remove outdated comment in glock_work_func Andreas Gruenbacher
2024-05-29 17:03 ` [PATCH 09/15] gfs2: Invert the GLF_INITIAL flag Andreas Gruenbacher
2024-05-29 17:03 ` [PATCH 10/15] gfs2: gfs2_glock_get cleanup Andreas Gruenbacher
2024-05-29 17:03 ` [PATCH 11/15] gfs2: Report when glocks cannot be freed for a long time Andreas Gruenbacher
2024-05-29 17:03 ` [PATCH 12/15] gfs2: Switch to a per-filesystem glock workqueue Andreas Gruenbacher
2024-05-29 17:03 ` [PATCH 13/15] gfs2: Revise glock reference counting model Andreas Gruenbacher
2024-05-29 17:03 ` [PATCH 14/15] Revert "GFS2: Don't add all glocks to the lru" Andreas Gruenbacher
2024-05-29 17:03 ` [PATCH 15/15] gfs2: Get rid of demote_ok checks Andreas Gruenbacher

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).