[PATCH 0/5] gfs2 non-blocking lookup

[PATCH 0/5] gfs2 non-blocking lookup

[Andreas Gruenbacher]

This patch queue implements non-blocking lookup in gfs2.

A previous attempt to implement this feature was made in December 2023
[1], but issues were found [2] and that attempt was aborted.  This new
implementation should be free of those problems.

Please review.

Thanks,
Andreas

[1] https://lore.kernel.org/gfs2/20231220202457.1581334-2-agruenba@redhat.com/
[2] https://lore.kernel.org/gfs2/20240119212056.805617-1-agruenba@redhat.com/


Andreas Gruenbacher (5):
  gfs2: No d_revalidate op for "lock_nolock" mounts
  gfs2: Get rid of had_lock in gfs2_drevalidate
  gfs2: Use unique tokens for gfs2_revalidate
  gfs2: Enable non-blocking lookup in gfs2_permission
  Revert "gfs2: Add GL_NOBLOCK flag"

 fs/gfs2/dentry.c     | 63 +++++++++++++++++++++++++++++---------------
 fs/gfs2/glock.c      | 39 +--------------------------
 fs/gfs2/glock.h      |  1 -
 fs/gfs2/glops.c      | 15 +++++++++--
 fs/gfs2/incore.h     |  1 +
 fs/gfs2/inode.c      | 20 +++++++++-----
 fs/gfs2/ops_fstype.c |  6 ++++-
 fs/gfs2/super.h      |  1 +
 8 files changed, 76 insertions(+), 70 deletions(-)

-- 
2.51.0

[PATCH 1/5] gfs2: No d_revalidate op for "lock_nolock" mounts

[Andreas Gruenbacher]

Local mounts (with the "lock_nolock" lock protocol) don't need a
d_revalidate dentry operation, so leave it undefined for them.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/dentry.c     | 9 +++++----
 fs/gfs2/ops_fstype.c | 6 +++++-
 fs/gfs2/super.h      | 1 +
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/fs/gfs2/dentry.c b/fs/gfs2/dentry.c
index 95050e719233..fc2be386d3f0 100644
--- a/fs/gfs2/dentry.c
+++ b/fs/gfs2/dentry.c
@@ -35,7 +35,6 @@
 static int gfs2_drevalidate(struct inode *dir, const struct qstr *name,
 			    struct dentry *dentry, unsigned int flags)
 {
-	struct gfs2_sbd *sdp = GFS2_SB(dir);
 	struct gfs2_inode *dip = GFS2_I(dir);
 	struct inode *inode;
 	struct gfs2_holder d_gh;
@@ -54,9 +53,6 @@ static int gfs2_drevalidate(struct inode *dir, const struct qstr *name,
 		ip = GFS2_I(inode);
 	}
 
-	if (sdp->sd_lockstruct.ls_ops->lm_mount == NULL)
-		return 1;
-
 	had_lock = (gfs2_glock_is_locked_by_me(dip->i_gl) != NULL);
 	if (!had_lock) {
 		error = gfs2_glock_nq_init(dip->i_gl, LM_ST_SHARED, 0, &d_gh);
@@ -95,6 +91,11 @@ static int gfs2_dentry_delete(const struct dentry *dentry)
 	return 0;
 }
 
+const struct dentry_operations gfs2_nolock_dops = {
+	.d_hash = gfs2_dhash,
+	.d_delete = gfs2_dentry_delete,
+};
+
 const struct dentry_operations gfs2_dops = {
 	.d_revalidate = gfs2_drevalidate,
 	.d_hash = gfs2_dhash,
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index acd9eefd4ff3..3d3033a75e3b 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -1138,7 +1138,6 @@ static int gfs2_fill_super(struct super_block *sb, struct fs_context *fc)
 	sb->s_magic = GFS2_MAGIC;
 	sb->s_op = &gfs2_super_ops;
 
-	set_default_d_op(sb, &gfs2_dops);
 	sb->s_export_op = &gfs2_export_ops;
 	sb->s_qcop = &gfs2_quotactl_ops;
 	sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP;
@@ -1207,6 +1206,11 @@ static int gfs2_fill_super(struct super_block *sb, struct fs_context *fc)
 	error = gfs2_lm_mount(sdp, silent);
 	if (error)
 		goto fail_debug;
+	if (sdp->sd_lockstruct.ls_ops->lm_mount == NULL)
+		set_default_d_op(sb, &gfs2_nolock_dops);
+	else
+		set_default_d_op(sb, &gfs2_dops);
+
 
 	INIT_WORK(&sdp->sd_withdraw_work, gfs2_withdraw_func);
 
diff --git a/fs/gfs2/super.h b/fs/gfs2/super.h
index 173f1e74c2a9..d4ce3fe1835d 100644
--- a/fs/gfs2/super.h
+++ b/fs/gfs2/super.h
@@ -57,6 +57,7 @@ extern struct file_system_type gfs2_fs_type;
 extern struct file_system_type gfs2meta_fs_type;
 extern const struct export_operations gfs2_export_ops;
 extern const struct super_operations gfs2_super_ops;
+extern const struct dentry_operations gfs2_nolock_dops;
 extern const struct dentry_operations gfs2_dops;
 
 extern const struct xattr_handler * const gfs2_xattr_handlers_max[];
-- 
2.51.0

[PATCH 2/5] gfs2: Get rid of had_lock in gfs2_drevalidate

[Andreas Gruenbacher]

Get rid of variable had_lock in gfs2_drevalidate().

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

diff --git a/fs/gfs2/dentry.c b/fs/gfs2/dentry.c
index fc2be386d3f0..ca470d4c4f9e 100644
--- a/fs/gfs2/dentry.c
+++ b/fs/gfs2/dentry.c
@@ -40,11 +40,11 @@ static int gfs2_drevalidate(struct inode *dir, const struct qstr *name,
 	struct gfs2_holder d_gh;
 	struct gfs2_inode *ip = NULL;
 	int error, valid;
-	int had_lock = 0;
 
 	if (flags & LOOKUP_RCU)
 		return -ECHILD;
 
+	gfs2_holder_mark_uninitialized(&d_gh);
 	inode = d_inode(dentry);
 
 	if (inode) {
@@ -53,8 +53,7 @@ static int gfs2_drevalidate(struct inode *dir, const struct qstr *name,
 		ip = GFS2_I(inode);
 	}
 
-	had_lock = (gfs2_glock_is_locked_by_me(dip->i_gl) != NULL);
-	if (!had_lock) {
+	if (gfs2_glock_is_locked_by_me(dip->i_gl) == NULL) {
 		error = gfs2_glock_nq_init(dip->i_gl, LM_ST_SHARED, 0, &d_gh);
 		if (error)
 			return 0;
@@ -62,9 +61,9 @@ static int gfs2_drevalidate(struct inode *dir, const struct qstr *name,
 
 	error = gfs2_dir_check(dir, name, ip);
 	valid = inode ? !error : (error == -ENOENT);
-
-	if (!had_lock)
+	if (gfs2_holder_initialized(&d_gh))
 		gfs2_glock_dq_uninit(&d_gh);
+
 	return valid;
 }
 
-- 
2.51.0

[PATCH 3/5] gfs2: Use unique tokens for gfs2_revalidate

[Andreas Gruenbacher]

GFS2 doesn't have persistent inode versions which we could use to avoid
repeated lookups in gfs2_revalidate().  We can achieve a similar effect
by introducing "fake" inode versions that only change when an inode is
invalidated, though:

 - When a directory is instantiated in inode_go_instantiate(), we set
   dir->i_version to a unique non-zero token.

 - When a dentry is instantiated by d_instantiate() or d_splice_alias(),
   we set dentry->d_time to dir->i_version.

 - In gfs2_drevalidate(), when dir->i_version == dentry->d_time, we know
   that the dentry is still up to date.  Otherwise, once we have fully
   revalidated dentry, we update dentry->d_time to match dir->i_version
   again.

 - When a directory is invalidated in inode_go_inval(), we zero out
   dir->i_version.  The directory will be assigned a new unique token
   the next time it is instantiated, so d_instantiate() will fully
   revalidate all cached dentries.

The checks in gfs2_drevalidate() needed for this mechanism can be
carried out under LOOKUP_RCU.

The "fake" inode version remains unchanged as long as its directory
remains cached, independent of any local directory changes.  All cached
dentries will remain valid during that time.  With a "real", persistent
inode version that changes whenever the directory changes, dentries
would be revalidated whenever the directory changes, but they would
remain valid across glock invalidation / re-instantiation.

Reviewed-by: Miklos Szeredi <mszeredi@redhat.com>
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/dentry.c | 49 ++++++++++++++++++++++++++++++++++--------------
 fs/gfs2/glops.c  | 15 +++++++++++++--
 fs/gfs2/incore.h |  1 +
 fs/gfs2/inode.c  |  6 ++++++
 4 files changed, 55 insertions(+), 16 deletions(-)

diff --git a/fs/gfs2/dentry.c b/fs/gfs2/dentry.c
index ca470d4c4f9e..8eac9feef22e 100644
--- a/fs/gfs2/dentry.c
+++ b/fs/gfs2/dentry.c
@@ -38,29 +38,50 @@ static int gfs2_drevalidate(struct inode *dir, const struct qstr *name,
 	struct gfs2_inode *dip = GFS2_I(dir);
 	struct inode *inode;
 	struct gfs2_holder d_gh;
-	struct gfs2_inode *ip = NULL;
+	struct gfs2_inode *ip;
 	int error, valid;
-
-	if (flags & LOOKUP_RCU)
-		return -ECHILD;
+	unsigned long ver;
 
 	gfs2_holder_mark_uninitialized(&d_gh);
-	inode = d_inode(dentry);
-
-	if (inode) {
-		if (is_bad_inode(inode))
+	if (flags & LOOKUP_RCU) {
+		inode = d_inode_rcu(dentry);
+		if (!inode)
+			return -ECHILD;
+	} else {
+		inode = d_inode(dentry);
+		if (inode && is_bad_inode(inode))
 			return 0;
-		ip = GFS2_I(inode);
-	}
 
-	if (gfs2_glock_is_locked_by_me(dip->i_gl) == NULL) {
-		error = gfs2_glock_nq_init(dip->i_gl, LM_ST_SHARED, 0, &d_gh);
-		if (error)
-			return 0;
+		if (gfs2_glock_is_locked_by_me(dip->i_gl) == NULL) {
+			error = gfs2_glock_nq_init(dip->i_gl, LM_ST_SHARED, 0,
+						   &d_gh);
+			if (error)
+				return 0;
+		}
 	}
 
+	/*
+	 * GFS2 doesn't have persistent inode versions.  Instead, when a
+	 * directory is instantiated (which implies that we are holding the
+	 * corresponding glock), we set i_version to a unique token based on
+	 * sdp->sd_unique.  Later, when the directory is invalidated, we set
+	 * i_version to 0.  The next time the directory is instantiated, a new
+	 * unique token will be assigned to i_version and all cached dentries
+	 * will be fully revalidated.
+	 */
+
+	ver = atomic64_read(&dir->i_version);
+	if (ver && READ_ONCE(dentry->d_time) == ver)
+		return 1;
+
+	if (flags & LOOKUP_RCU)
+		return -ECHILD;
+
+	ip = inode ? GFS2_I(inode) : NULL;
 	error = gfs2_dir_check(dir, name, ip);
 	valid = inode ? !error : (error == -ENOENT);
+	if (valid)
+		WRITE_ONCE(dentry->d_time, ver);
 	if (gfs2_holder_initialized(&d_gh))
 		gfs2_glock_dq_uninit(&d_gh);
 
diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
index 549b59157ed4..b1061dd885c2 100644
--- a/fs/gfs2/glops.c
+++ b/fs/gfs2/glops.c
@@ -358,6 +358,7 @@ static int inode_go_sync(struct gfs2_glock *gl)
 static void inode_go_inval(struct gfs2_glock *gl, int flags)
 {
 	struct gfs2_inode *ip = gfs2_glock2inode(gl);
+	struct inode *inode = &ip->i_inode;
 
 	gfs2_assert_withdraw(gl->gl_name.ln_sbd, !atomic_read(&gl->gl_ail_count));
 
@@ -378,8 +379,12 @@ static void inode_go_inval(struct gfs2_glock *gl, int flags)
 			       GFS2_LFC_INODE_GO_INVAL);
 		gl->gl_name.ln_sbd->sd_rindex_uptodate = 0;
 	}
-	if (ip && S_ISREG(ip->i_inode.i_mode))
-		truncate_inode_pages(ip->i_inode.i_mapping, 0);
+	if (ip) {
+		if (S_ISREG(ip->i_inode.i_mode))
+			truncate_inode_pages(ip->i_inode.i_mapping, 0);
+		else if (S_ISDIR(inode->i_mode))
+			atomic64_set(&inode->i_version, 0);
+	}
 
 	gfs2_clear_glop_pending(ip);
 }
@@ -498,6 +503,7 @@ static int gfs2_inode_refresh(struct gfs2_inode *ip)
 static int inode_go_instantiate(struct gfs2_glock *gl)
 {
 	struct gfs2_inode *ip = gl->gl_object;
+	struct inode *inode = &ip->i_inode;
 	struct gfs2_glock *io_gl;
 	int error;
 
@@ -509,6 +515,11 @@ static int inode_go_instantiate(struct gfs2_glock *gl)
 		return error;
 	io_gl = ip->i_iopen_gh.gh_gl;
 	io_gl->gl_no_formal_ino = ip->i_no_formal_ino;
+	if (S_ISDIR(inode->i_mode)) {
+		struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
+		atomic64_set(&inode->i_version,
+			     atomic64_inc_return(&sdp->sd_unique));
+	}
 	return 0;
 }
 
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index d05d8fe4e456..d3a1cfcc62ec 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -698,6 +698,7 @@ struct gfs2_sbd {
 
 	struct gfs2_args sd_args;	/* Mount arguments */
 	struct gfs2_tune sd_tune;	/* Filesystem tuning structure */
+	atomic64_t sd_unique;
 
 	/* Lock Stuff */
 
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 8a7ed80d9f2d..3d87ecd277a1 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -731,6 +731,7 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
 			goto fail_gunlock;
 		}
 		d_instantiate(dentry, inode);
+		dentry->d_time = atomic64_read(&dir->i_version);
 		error = 0;
 		if (file) {
 			if (S_ISREG(inode->i_mode))
@@ -785,6 +786,8 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
 		gfs2_set_aops(inode);
 		break;
 	case S_IFDIR:
+		atomic64_set(&inode->i_version,
+			     atomic64_inc_return(&sdp->sd_unique));
 		ip->i_diskflags |= (dip->i_diskflags & GFS2_DIF_INHERIT_JDATA);
 		ip->i_diskflags |= GFS2_DIF_JDATA;
 		ip->i_entries = 2;
@@ -878,6 +881,7 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
 
 	mark_inode_dirty(inode);
 	d_instantiate(dentry, inode);
+	dentry->d_time = atomic64_read(&dir->i_version);
 	/* After instantiate, errors should result in evict which will destroy
 	 * both inode and iopen glocks properly. */
 	if (file) {
@@ -988,6 +992,7 @@ static struct dentry *__gfs2_lookup(struct inode *dir, struct dentry *dentry,
 	}
 
 	d = d_splice_alias(inode, dentry);
+	dentry->d_time = atomic64_read(&dir->i_version);
 	if (IS_ERR(d)) {
 		gfs2_glock_dq_uninit(&gh);
 		return d;
@@ -1119,6 +1124,7 @@ static int gfs2_link(struct dentry *old_dentry, struct inode *dir,
 	inode_set_ctime_current(&ip->i_inode);
 	ihold(inode);
 	d_instantiate(dentry, inode);
+	dentry->d_time = atomic64_read(&dir->i_version);
 	mark_inode_dirty(inode);
 
 out_brelse:
-- 
2.51.0

[PATCH 4/5] gfs2: Enable non-blocking lookup in gfs2_permission

[Andreas Gruenbacher]

Under MAY_NOT_BLOCK, we consider the cached permissions "good enough" if
the glock still appears to be held.

Reviewed-by: Miklos Szeredi <mszeredi@redhat.com>
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/inode.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 3d87ecd277a1..af38676a408f 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -1955,14 +1955,14 @@ int gfs2_permission(struct mnt_idmap *idmap, struct inode *inode,
 	gfs2_holder_mark_uninitialized(&i_gh);
 	ip = GFS2_I(inode);
 	gl = rcu_dereference_check(ip->i_gl, !may_not_block);
-	if (unlikely(!gl)) {
-		/* inode is getting torn down, must be RCU mode */
-		WARN_ON_ONCE(!may_not_block);
-		return -ECHILD;
-        }
-	if (gfs2_glock_is_locked_by_me(gl) == NULL) {
-		if (may_not_block)
+	if (may_not_block) {
+		/* Is the inode getting torn down? */
+		if (unlikely(!gl))
 			return -ECHILD;
+		if (gl->gl_state == LM_ST_UNLOCKED ||
+		    test_bit(GLF_INSTANTIATE_NEEDED, &gl->gl_flags))
+			return -ECHILD;
+	} else if (gfs2_glock_is_locked_by_me(gl) == NULL) {
 		error = gfs2_glock_nq_init(gl, LM_ST_SHARED, LM_FLAG_ANY, &i_gh);
 		if (error)
 			return error;
-- 
2.51.0

[PATCH 5/5] Revert "gfs2: Add GL_NOBLOCK flag"

[Andreas Gruenbacher]

This reverts commit f9f229c1f75df2f1fe63b16615d184da4e90bb10.

It turns out that for non-blocking lookup, taking glocks without
blocking isn't actually needed, so remove that feature again.

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

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index bc6f236b899c..e6f436241637 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -501,23 +501,6 @@ static inline struct gfs2_holder *find_first_waiter(const struct gfs2_glock *gl)
 	return NULL;
 }
 
-/**
- * find_last_waiter - find the last gh that's waiting for the glock
- * @gl: the glock
- *
- * This also is a fast way of finding out if there are any waiters.
- */
-
-static inline struct gfs2_holder *find_last_waiter(const struct gfs2_glock *gl)
-{
-	struct gfs2_holder *gh;
-
-	if (list_empty(&gl->gl_holders))
-		return NULL;
-	gh = list_last_entry(&gl->gl_holders, struct gfs2_holder, gh_list);
-	return test_bit(HIF_HOLDER, &gh->gh_iflags) ? NULL : gh;
-}
-
 /**
  * state_change - record that the glock is now in a different state
  * @gl: the glock
@@ -1462,30 +1445,11 @@ int gfs2_glock_nq(struct gfs2_holder *gh)
 {
 	struct gfs2_glock *gl = gh->gh_gl;
 	struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
-	int error;
+	int error = 0;
 
 	if (gfs2_withdrawn(sdp))
 		return -EIO;
 
-	if (gh->gh_flags & GL_NOBLOCK) {
-		struct gfs2_holder *current_gh;
-
-		error = -ECHILD;
-		spin_lock(&gl->gl_lockref.lock);
-		if (find_last_waiter(gl))
-			goto unlock;
-		current_gh = find_first_holder(gl);
-		if (!may_grant(gl, current_gh, gh))
-			goto unlock;
-		set_bit(HIF_HOLDER, &gh->gh_iflags);
-		list_add_tail(&gh->gh_list, &gl->gl_holders);
-		trace_gfs2_promote(gh);
-		error = 0;
-unlock:
-		spin_unlock(&gl->gl_lockref.lock);
-		return error;
-	}
-
 	gh->gh_error = 0;
 	spin_lock(&gl->gl_lockref.lock);
 	add_to_queue(gh);
@@ -1498,7 +1462,6 @@ int gfs2_glock_nq(struct gfs2_holder *gh)
 	run_queue(gl, 1);
 	spin_unlock(&gl->gl_lockref.lock);
 
-	error = 0;
 	if (!(gh->gh_flags & GL_ASYNC))
 		error = gfs2_glock_wait(gh);
 
diff --git a/fs/gfs2/glock.h b/fs/gfs2/glock.h
index 55d5985f32a0..885a749209f8 100644
--- a/fs/gfs2/glock.h
+++ b/fs/gfs2/glock.h
@@ -88,7 +88,6 @@ enum {
 #define GL_SKIP			0x0100
 #define GL_NOPID		0x0200
 #define GL_NOCACHE		0x0400
-#define GL_NOBLOCK		0x0800
   
 /*
  * lm_async_cb return flags
-- 
2.51.0

Re: [PATCH 0/5] gfs2 non-blocking lookup

[Andreas Gruenbacher]

On Sat, Nov 1, 2025 at 9:09 AM Andreas Gruenbacher <agruenba@redhat.com> wrote:
> This patch queue implements non-blocking lookup in gfs2.
>
> A previous attempt to implement this feature was made in December 2023
> [1], but issues were found [2] and that attempt was aborted.  This new
> implementation should be free of those problems.
>
> Please review.

Note that I'm removing this patch queue from for-next for the upcoming
merge window to give them some more bake time to.

Thanks,
Andreas

> Thanks,
> Andreas
>
> [1] https://lore.kernel.org/gfs2/20231220202457.1581334-2-agruenba@redhat.com/
> [2] https://lore.kernel.org/gfs2/20240119212056.805617-1-agruenba@redhat.com/
>
>
> Andreas Gruenbacher (5):
>   gfs2: No d_revalidate op for "lock_nolock" mounts
>   gfs2: Get rid of had_lock in gfs2_drevalidate
>   gfs2: Use unique tokens for gfs2_revalidate
>   gfs2: Enable non-blocking lookup in gfs2_permission
>   Revert "gfs2: Add GL_NOBLOCK flag"
>
>  fs/gfs2/dentry.c     | 63 +++++++++++++++++++++++++++++---------------
>  fs/gfs2/glock.c      | 39 +--------------------------
>  fs/gfs2/glock.h      |  1 -
>  fs/gfs2/glops.c      | 15 +++++++++--
>  fs/gfs2/incore.h     |  1 +
>  fs/gfs2/inode.c      | 20 +++++++++-----
>  fs/gfs2/ops_fstype.c |  6 ++++-
>  fs/gfs2/super.h      |  1 +
>  8 files changed, 76 insertions(+), 70 deletions(-)
>
> --
> 2.51.0
>