[PATCH 0/9] gfs2: Bugs in "Use GL_NOBLOCK flag for non-blocking lookups"

[PATCH 0/9] gfs2: Bugs in "Use GL_NOBLOCK flag for non-blocking lookups"

[Andreas Gruenbacher]

Hello,

Al Viro has reported issues with commit dd00aaeb3432 ("gfs2: Use
GL_NOBLOCK flag for non-blocking lookups"):

* First, parent can now be NULL and dereferencing it in
  gfs2_dir_check(d_inode(parent), &dentry->d_name, ip) isn't going to
  work;

* Second, gfs2_dir_check() can still sleep, which breaks LOOKUP_RCU
  mode.

These patches should fix both of these problems.  We'll either need
these fixes added, or commit dd00aaeb3432 reverted for now.

Thanks,
Andreas

Andreas Gruenbacher (9):
  gfs2: Fix gfs2_drevalidate NULL pointer dereference
  gfs2: Pass FGP flags to gfs2_getbuf
  gfs2: Split gfs2_meta_read_async off from gfs2_meta_read
  gfs2: Add FGP_NOWAIT support to gfs2_meta_read_async
  gfs2: Pass FGP flags to gfs2_meta_{,inode_}buffer
  gfs2: Pass FGP flags to gfs2_dirent_search
  gfs2: Pass FGP flags to gfs2_dir_check
  gfs2: Minor gfs2_drevalidate cleanup
  gfs2: Fix LOOKUP_RCU support in gfs2_drevalidate

 fs/gfs2/aops.c       |   4 +-
 fs/gfs2/bmap.c       |  21 ++++----
 fs/gfs2/dentry.c     |  32 ++++++++-----
 fs/gfs2/dir.c        |  52 +++++++++++---------
 fs/gfs2/dir.h        |   2 +-
 fs/gfs2/file.c       |   4 +-
 fs/gfs2/glops.c      |   2 +-
 fs/gfs2/incore.h     |   1 -
 fs/gfs2/inode.c      |  10 ++--
 fs/gfs2/meta_io.c    | 112 ++++++++++++++++++++++++++++---------------
 fs/gfs2/meta_io.h    |  15 +++---
 fs/gfs2/ops_fstype.c |   4 +-
 fs/gfs2/quota.c      |   2 +-
 fs/gfs2/recovery.c   |   2 +-
 fs/gfs2/rgrp.c       |   5 +-
 fs/gfs2/super.c      |   6 +--
 fs/gfs2/xattr.c      |  17 ++++---
 17 files changed, 173 insertions(+), 118 deletions(-)

-- 
2.43.0

[PATCH 1/9] gfs2: Fix gfs2_drevalidate NULL pointer dereference

[Andreas Gruenbacher]

Commit dd00aaeb3432 added an RCU-safe way of computing d_inode(parent)
to gfs2_drevalidate() to support the LOOKUP_RCU flag, but then failed to
convert one of the instances of d_inode(parent) to its RCU-safe
replacement.  This manifested as a NULL pointer dereference.  Fix that.

Reported-by: Al Viro <viro@zeniv.linux.org.uk>
Fixes: dd00aaeb3432 ("gfs2: Use GL_NOBLOCK flag for non-blocking lookups")
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/dentry.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/gfs2/dentry.c b/fs/gfs2/dentry.c
index 177f1f41f225..c6483fb98624 100644
--- a/fs/gfs2/dentry.c
+++ b/fs/gfs2/dentry.c
@@ -72,7 +72,7 @@ static int gfs2_drevalidate(struct dentry *dentry, unsigned int flags)
 			goto out;
 	}
 
-	error = gfs2_dir_check(d_inode(parent), &dentry->d_name, ip);
+	error = gfs2_dir_check(dinode, &dentry->d_name, ip);
 	valid = inode ? !error : (error == -ENOENT);
 
 	if (!had_lock)
-- 
2.43.0

[PATCH 2/9] gfs2: Pass FGP flags to gfs2_getbuf

[Andreas Gruenbacher]

Replace gfs2_getbuf()'s create argument with a fgp_flags argument.  Use
the FGP_CREAT flag instead of the CREATE flag to indicate that new
buffers should be created.

In addition, when the FGP_NOWAIT flag is set and gfs2_getbuf() would
sleep, -EAGAIN is returned instead.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/bmap.c    |  2 +-
 fs/gfs2/dir.c     |  2 +-
 fs/gfs2/meta_io.c | 33 +++++++++++++++++++--------------
 fs/gfs2/meta_io.h |  2 +-
 4 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index d9ccfd27e4f1..92945e5b7643 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -301,7 +301,7 @@ static void gfs2_metapath_ra(struct gfs2_glock *gl, __be64 *start, __be64 *end)
 		if (!*t)
 			continue;
 
-		rabh = gfs2_getbuf(gl, be64_to_cpu(*t), CREATE);
+		rabh = gfs2_getbuf(gl, be64_to_cpu(*t), FGP_CREAT);
 		if (trylock_buffer(rabh)) {
 			if (!buffer_uptodate(rabh)) {
 				rabh->b_end_io = end_buffer_read_sync;
diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
index 560e4624c09f..518a7fb42df0 100644
--- a/fs/gfs2/dir.c
+++ b/fs/gfs2/dir.c
@@ -1500,7 +1500,7 @@ static void gfs2_dir_readahead(struct inode *inode, unsigned hsize, u32 index,
 		if (blocknr == last)
 			continue;
 
-		bh = gfs2_getbuf(gl, blocknr, 1);
+		bh = gfs2_getbuf(gl, blocknr, FGP_CREAT);
 		if (trylock_buffer(bh)) {
 			if (buffer_uptodate(bh)) {
 				unlock_buffer(bh);
diff --git a/fs/gfs2/meta_io.c b/fs/gfs2/meta_io.c
index f814054c8cd0..d0f3727f24db 100644
--- a/fs/gfs2/meta_io.c
+++ b/fs/gfs2/meta_io.c
@@ -106,12 +106,15 @@ const struct address_space_operations gfs2_rgrp_aops = {
  * gfs2_getbuf - Get a buffer with a given address space
  * @gl: the glock
  * @blkno: the block number (filesystem scope)
- * @create: 1 if the buffer should be created
+ * @fgp_flags: Flags like FGP_CREAT and FGP_NOWAIT
+ *
+ * Returns ERR_PTR(-EAGAIN) if the FGP_NOWAIT flag is set and the function
+ * would sleep.
  *
  * Returns: the buffer
  */
-
-struct buffer_head *gfs2_getbuf(struct gfs2_glock *gl, u64 blkno, int create)
+struct buffer_head *gfs2_getbuf(struct gfs2_glock *gl, u64 blkno,
+				fgf_t fgp_flags)
 {
 	struct address_space *mapping = gfs2_glock2aspace(gl);
 	struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
@@ -128,17 +131,19 @@ struct buffer_head *gfs2_getbuf(struct gfs2_glock *gl, u64 blkno, int create)
 	index = blkno >> shift;             /* convert block to page */
 	bufnum = blkno - (index << shift);  /* block buf index within page */
 
-	if (create) {
+	fgp_flags |= FGP_LOCK | FGP_ACCESSED;
+	if (fgp_flags & FGP_CREAT) {
 		folio = __filemap_get_folio(mapping, index,
-				FGP_LOCK | FGP_ACCESSED | FGP_CREAT,
+				fgp_flags,
 				mapping_gfp_mask(mapping) | __GFP_NOFAIL);
+		if (IS_ERR(folio))
+			return ERR_CAST(folio);
 		bh = folio_buffers(folio);
-		if (!bh)
+		if (!bh && !(fgp_flags & FGP_NOWAIT))
 			bh = create_empty_buffers(folio,
 				sdp->sd_sb.sb_bsize, 0);
 	} else {
-		folio = __filemap_get_folio(mapping, index,
-				FGP_LOCK | FGP_ACCESSED, 0);
+		folio = __filemap_get_folio(mapping, index, fgp_flags, 0);
 		if (IS_ERR(folio))
 			return NULL;
 		bh = folio_buffers(folio);
@@ -181,7 +186,7 @@ static void meta_prep_new(struct buffer_head *bh)
 struct buffer_head *gfs2_meta_new(struct gfs2_glock *gl, u64 blkno)
 {
 	struct buffer_head *bh;
-	bh = gfs2_getbuf(gl, blkno, CREATE);
+	bh = gfs2_getbuf(gl, blkno, FGP_CREAT);
 	meta_prep_new(bh);
 	return bh;
 }
@@ -258,7 +263,7 @@ int gfs2_meta_read(struct gfs2_glock *gl, u64 blkno, int flags,
 		return -EIO;
 	}
 
-	*bhp = bh = gfs2_getbuf(gl, blkno, CREATE);
+	*bhp = bh = gfs2_getbuf(gl, blkno, FGP_CREAT);
 
 	lock_buffer(bh);
 	if (buffer_uptodate(bh)) {
@@ -271,7 +276,7 @@ int gfs2_meta_read(struct gfs2_glock *gl, u64 blkno, int flags,
 	}
 
 	if (rahead) {
-		bh = gfs2_getbuf(gl, blkno + 1, CREATE);
+		bh = gfs2_getbuf(gl, blkno + 1, FGP_CREAT);
 
 		lock_buffer(bh);
 		if (buffer_uptodate(bh)) {
@@ -443,7 +448,7 @@ void gfs2_journal_wipe(struct gfs2_inode *ip, u64 bstart, u32 blen)
 	gfs2_ail1_wipe(sdp, bstart, blen);
 	while (blen) {
 		ty = REMOVE_META;
-		bh = gfs2_getbuf(ip->i_gl, bstart, NO_CREATE);
+		bh = gfs2_getbuf(ip->i_gl, bstart, 0);
 		if (!bh && gfs2_is_jdata(ip)) {
 			bh = gfs2_getjdatabuf(ip, bstart);
 			ty = REMOVE_JDATA;
@@ -519,7 +524,7 @@ struct buffer_head *gfs2_meta_ra(struct gfs2_glock *gl, u64 dblock, u32 extlen)
 	if (extlen > max_ra)
 		extlen = max_ra;
 
-	first_bh = gfs2_getbuf(gl, dblock, CREATE);
+	first_bh = gfs2_getbuf(gl, dblock, FGP_CREAT);
 
 	if (buffer_uptodate(first_bh))
 		goto out;
@@ -529,7 +534,7 @@ struct buffer_head *gfs2_meta_ra(struct gfs2_glock *gl, u64 dblock, u32 extlen)
 	extlen--;
 
 	while (extlen) {
-		bh = gfs2_getbuf(gl, dblock, CREATE);
+		bh = gfs2_getbuf(gl, dblock, FGP_CREAT);
 
 		bh_readahead(bh, REQ_RAHEAD | REQ_META | REQ_PRIO);
 		brelse(bh);
diff --git a/fs/gfs2/meta_io.h b/fs/gfs2/meta_io.h
index 831d988c2ceb..e239e410881c 100644
--- a/fs/gfs2/meta_io.h
+++ b/fs/gfs2/meta_io.h
@@ -55,7 +55,7 @@ int gfs2_meta_read(struct gfs2_glock *gl, u64 blkno, int flags,
 		   int rahead, struct buffer_head **bhp);
 int gfs2_meta_wait(struct gfs2_sbd *sdp, struct buffer_head *bh);
 struct buffer_head *gfs2_getbuf(struct gfs2_glock *gl, u64 blkno,
-			        int create);
+				fgf_t fgp_flags);
 enum {
 	REMOVE_JDATA = 0,
 	REMOVE_META = 1,
-- 
2.43.0

[PATCH 3/9] gfs2: Split gfs2_meta_read_async off from gfs2_meta_read

[Andreas Gruenbacher]

Split gfs2_meta_read_async() off from gfs2_meta_read() and implement
gfs2_meta_read() in terms of gfs2_meta_read_async() and
gfs2_meta_wait().

The initial check for filesystem withdrawal in gfs2_meta_wait() is
unnecessary and can be removed because gfs2_meta_wait() always follows
gfs2_meta_read_async() in the code and gfs2_meta_read_async() already
performs that check.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/dir.c     |  6 +++---
 fs/gfs2/incore.h  |  1 -
 fs/gfs2/meta_io.c | 46 ++++++++++++++++++++++------------------------
 fs/gfs2/meta_io.h |  6 ++++--
 fs/gfs2/quota.c   |  2 +-
 fs/gfs2/rgrp.c    |  3 ++-
 fs/gfs2/xattr.c   | 13 ++++++-------
 7 files changed, 38 insertions(+), 39 deletions(-)

diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
index 518a7fb42df0..446e374a8d78 100644
--- a/fs/gfs2/dir.c
+++ b/fs/gfs2/dir.c
@@ -105,7 +105,7 @@ static int gfs2_dir_get_existing_buffer(struct gfs2_inode *ip, u64 block,
 	struct buffer_head *bh;
 	int error;
 
-	error = gfs2_meta_read(ip->i_gl, block, DIO_WAIT, 0, &bh);
+	error = gfs2_meta_read(ip->i_gl, block, 0, &bh);
 	if (error)
 		return error;
 	if (gfs2_metatype_check(GFS2_SB(&ip->i_inode), bh, GFS2_METATYPE_JD)) {
@@ -300,7 +300,7 @@ static int gfs2_dir_read_data(struct gfs2_inode *ip, __be64 *buf,
 			BUG_ON(extlen < 1);
 			bh = gfs2_meta_ra(ip->i_gl, dblock, extlen);
 		} else {
-			error = gfs2_meta_read(ip->i_gl, dblock, DIO_WAIT, 0, &bh);
+			error = gfs2_meta_read(ip->i_gl, dblock, 0, &bh);
 			if (error)
 				goto fail;
 		}
@@ -757,7 +757,7 @@ static int get_leaf(struct gfs2_inode *dip, u64 leaf_no,
 {
 	int error;
 
-	error = gfs2_meta_read(dip->i_gl, leaf_no, DIO_WAIT, 0, bhp);
+	error = gfs2_meta_read(dip->i_gl, leaf_no, 0, bhp);
 	if (!error && gfs2_metatype_check(GFS2_SB(&dip->i_inode), *bhp, GFS2_METATYPE_LF)) {
 		/* pr_info("block num=%llu\n", leaf_no); */
 		error = -EIO;
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index 95a334d64da2..e1343fd0a5b1 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -22,7 +22,6 @@
 #include <linux/rhashtable.h>
 #include <linux/mutex.h>
 
-#define DIO_WAIT	0x00000010
 #define DIO_METADATA	0x00000020
 
 struct gfs2_log_operations;
diff --git a/fs/gfs2/meta_io.c b/fs/gfs2/meta_io.c
index d0f3727f24db..6492d000d366 100644
--- a/fs/gfs2/meta_io.c
+++ b/fs/gfs2/meta_io.c
@@ -240,18 +240,17 @@ static void gfs2_submit_bhs(blk_opf_t opf, struct buffer_head *bhs[], int num)
 }
 
 /**
- * gfs2_meta_read - Read a block from disk
+ * gfs2_meta_read_async - Read a block from disk
  * @gl: The glock covering the block
  * @blkno: The block number
- * @flags: flags
  * @rahead: Do read-ahead
  * @bhp: the place where the buffer is returned (NULL on failure)
  *
  * Returns: errno
  */
 
-int gfs2_meta_read(struct gfs2_glock *gl, u64 blkno, int flags,
-		   int rahead, struct buffer_head **bhp)
+int gfs2_meta_read_async(struct gfs2_glock *gl, u64 blkno,
+			 int rahead, struct buffer_head **bhp)
 {
 	struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
 	struct buffer_head *bh, *bhs[2];
@@ -268,7 +267,6 @@ int gfs2_meta_read(struct gfs2_glock *gl, u64 blkno, int flags,
 	lock_buffer(bh);
 	if (buffer_uptodate(bh)) {
 		unlock_buffer(bh);
-		flags &= ~DIO_WAIT;
 	} else {
 		bh->b_end_io = end_buffer_read_sync;
 		get_bh(bh);
@@ -289,20 +287,6 @@ int gfs2_meta_read(struct gfs2_glock *gl, u64 blkno, int flags,
 	}
 
 	gfs2_submit_bhs(REQ_OP_READ | REQ_META | REQ_PRIO, bhs, num);
-	if (!(flags & DIO_WAIT))
-		return 0;
-
-	bh = *bhp;
-	wait_on_buffer(bh);
-	if (unlikely(!buffer_uptodate(bh))) {
-		struct gfs2_trans *tr = current->journal_info;
-		if (tr && test_bit(TR_TOUCHED, &tr->tr_flags))
-			gfs2_io_error_bh_wd(sdp, bh);
-		brelse(bh);
-		*bhp = NULL;
-		return -EIO;
-	}
-
 	return 0;
 }
 
@@ -316,10 +300,6 @@ int gfs2_meta_read(struct gfs2_glock *gl, u64 blkno, int flags,
 
 int gfs2_meta_wait(struct gfs2_sbd *sdp, struct buffer_head *bh)
 {
-	if (gfs2_withdrawing_or_withdrawn(sdp) &&
-	    !gfs2_withdraw_in_prog(sdp))
-		return -EIO;
-
 	wait_on_buffer(bh);
 
 	if (!buffer_uptodate(bh)) {
@@ -335,6 +315,24 @@ int gfs2_meta_wait(struct gfs2_sbd *sdp, struct buffer_head *bh)
 	return 0;
 }
 
+int gfs2_meta_read(struct gfs2_glock *gl, u64 blkno,
+		   int rahead, struct buffer_head **bhp)
+{
+	struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
+	int ret;
+
+	ret = gfs2_meta_read_async(gl, blkno, rahead, bhp);
+	if (ret)
+		return ret;
+
+	ret = gfs2_meta_wait(sdp, *bhp);
+	if (ret) {
+		brelse(*bhp);
+		*bhp = NULL;
+	}
+	return ret;
+}
+
 void gfs2_remove_from_journal(struct buffer_head *bh, int meta)
 {
 	struct address_space *mapping = bh->b_folio->mapping;
@@ -491,7 +489,7 @@ int gfs2_meta_buffer(struct gfs2_inode *ip, u32 mtype, u64 num,
 	if (num == ip->i_no_addr)
 		rahead = ip->i_rahead;
 
-	ret = gfs2_meta_read(gl, num, DIO_WAIT, rahead, &bh);
+	ret = gfs2_meta_read(gl, num, rahead, &bh);
 	if (ret == 0 && gfs2_metatype_check(sdp, bh, mtype)) {
 		brelse(bh);
 		ret = -EIO;
diff --git a/fs/gfs2/meta_io.h b/fs/gfs2/meta_io.h
index e239e410881c..f04c91eadfb4 100644
--- a/fs/gfs2/meta_io.h
+++ b/fs/gfs2/meta_io.h
@@ -51,9 +51,11 @@ static inline struct gfs2_sbd *gfs2_mapping2sbd(struct address_space *mapping)
 }
 
 struct buffer_head *gfs2_meta_new(struct gfs2_glock *gl, u64 blkno);
-int gfs2_meta_read(struct gfs2_glock *gl, u64 blkno, int flags,
-		   int rahead, struct buffer_head **bhp);
+int gfs2_meta_read_async(struct gfs2_glock *gl, u64 blkno,
+			 int rahead, struct buffer_head **bhp);
 int gfs2_meta_wait(struct gfs2_sbd *sdp, struct buffer_head *bh);
+int gfs2_meta_read(struct gfs2_glock *gl, u64 blkno,
+		   int rahead, struct buffer_head **bhp);
 struct buffer_head *gfs2_getbuf(struct gfs2_glock *gl, u64 blkno,
 				fgf_t fgp_flags);
 enum {
diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index aa9cf0102848..a68af8bdf7de 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -424,7 +424,7 @@ static int bh_get(struct gfs2_quota_data *qd)
 		goto fail;
 
 	error = gfs2_meta_read(ip->i_gl, iomap.addr >> inode->i_blkbits,
-			       DIO_WAIT, 0, &bh);
+			       0, &bh);
 	if (error)
 		goto fail;
 	error = -EIO;
diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index 26d6c1eea559..fcef82c767e3 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -1210,7 +1210,8 @@ int gfs2_rgrp_go_instantiate(struct gfs2_glock *gl)
 
 	for (x = 0; x < length; x++) {
 		bi = rgd->rd_bits + x;
-		error = gfs2_meta_read(gl, rgd->rd_addr + x, 0, 0, &bi->bi_bh);
+		error = gfs2_meta_read_async(gl, rgd->rd_addr + x, 0,
+					     &bi->bi_bh);
 		if (error)
 			goto fail;
 	}
diff --git a/fs/gfs2/xattr.c b/fs/gfs2/xattr.c
index 8c96ba6230d1..759347ec67af 100644
--- a/fs/gfs2/xattr.c
+++ b/fs/gfs2/xattr.c
@@ -128,7 +128,7 @@ static int ea_foreach(struct gfs2_inode *ip, ea_call_t ea_call, void *data)
 	__be64 *eablk, *end;
 	int error;
 
-	error = gfs2_meta_read(ip->i_gl, ip->i_eattr, DIO_WAIT, 0, &bh);
+	error = gfs2_meta_read(ip->i_gl, ip->i_eattr, 0, &bh);
 	if (error)
 		return error;
 
@@ -152,7 +152,7 @@ static int ea_foreach(struct gfs2_inode *ip, ea_call_t ea_call, void *data)
 			break;
 		bn = be64_to_cpu(*eablk);
 
-		error = gfs2_meta_read(ip->i_gl, bn, DIO_WAIT, 0, &eabh);
+		error = gfs2_meta_read(ip->i_gl, bn, 0, &eabh);
 		if (error)
 			break;
 		error = ea_foreach_i(ip, eabh, ea_call, data);
@@ -468,8 +468,8 @@ static int gfs2_iter_unstuffed(struct gfs2_inode *ip, struct gfs2_ea_header *ea,
 		return -ENOMEM;
 
 	for (x = 0; x < nptrs; x++) {
-		error = gfs2_meta_read(ip->i_gl, be64_to_cpu(*dataptrs), 0, 0,
-				       bh + x);
+		error = gfs2_meta_read_async(ip->i_gl, be64_to_cpu(*dataptrs),
+					     0, bh + x);
 		if (error) {
 			while (x--)
 				brelse(bh[x]);
@@ -976,8 +976,7 @@ static int ea_set_block(struct gfs2_inode *ip, struct gfs2_ea_request *er,
 	if (ip->i_diskflags & GFS2_DIF_EA_INDIRECT) {
 		__be64 *end;
 
-		error = gfs2_meta_read(ip->i_gl, ip->i_eattr, DIO_WAIT, 0,
-				       &indbh);
+		error = gfs2_meta_read(ip->i_gl, ip->i_eattr, 0, &indbh);
 		if (error)
 			return error;
 
@@ -1279,7 +1278,7 @@ static int ea_dealloc_indirect(struct gfs2_inode *ip)
 
 	memset(&rlist, 0, sizeof(struct gfs2_rgrp_list));
 
-	error = gfs2_meta_read(ip->i_gl, ip->i_eattr, DIO_WAIT, 0, &indbh);
+	error = gfs2_meta_read(ip->i_gl, ip->i_eattr, 0, &indbh);
 	if (error)
 		return error;
 
-- 
2.43.0

[PATCH 4/9] gfs2: Add FGP_NOWAIT support to gfs2_meta_read_async

[Andreas Gruenbacher]

Add an fgp_flags argument to gfs2_meta_read_async() and gfs2_meta_read()
that is passed through to gfs2_getbuf().  When the FGP_NOWAIT flag is
set and gfs2_meta_read_async() would sleep, -EAGAIN is returned instead.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/dir.c     |  6 +++---
 fs/gfs2/meta_io.c | 40 ++++++++++++++++++++++++++++++++++------
 fs/gfs2/meta_io.h |  4 ++--
 fs/gfs2/quota.c   |  2 +-
 fs/gfs2/rgrp.c    |  2 +-
 fs/gfs2/xattr.c   | 10 +++++-----
 6 files changed, 46 insertions(+), 18 deletions(-)

diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
index 446e374a8d78..a1f47696a2ec 100644
--- a/fs/gfs2/dir.c
+++ b/fs/gfs2/dir.c
@@ -105,7 +105,7 @@ static int gfs2_dir_get_existing_buffer(struct gfs2_inode *ip, u64 block,
 	struct buffer_head *bh;
 	int error;
 
-	error = gfs2_meta_read(ip->i_gl, block, 0, &bh);
+	error = gfs2_meta_read(ip->i_gl, block, 0, 0, &bh);
 	if (error)
 		return error;
 	if (gfs2_metatype_check(GFS2_SB(&ip->i_inode), bh, GFS2_METATYPE_JD)) {
@@ -300,7 +300,7 @@ static int gfs2_dir_read_data(struct gfs2_inode *ip, __be64 *buf,
 			BUG_ON(extlen < 1);
 			bh = gfs2_meta_ra(ip->i_gl, dblock, extlen);
 		} else {
-			error = gfs2_meta_read(ip->i_gl, dblock, 0, &bh);
+			error = gfs2_meta_read(ip->i_gl, dblock, 0, 0, &bh);
 			if (error)
 				goto fail;
 		}
@@ -757,7 +757,7 @@ static int get_leaf(struct gfs2_inode *dip, u64 leaf_no,
 {
 	int error;
 
-	error = gfs2_meta_read(dip->i_gl, leaf_no, 0, bhp);
+	error = gfs2_meta_read(dip->i_gl, leaf_no, 0, 0, bhp);
 	if (!error && gfs2_metatype_check(GFS2_SB(&dip->i_inode), *bhp, GFS2_METATYPE_LF)) {
 		/* pr_info("block num=%llu\n", leaf_no); */
 		error = -EIO;
diff --git a/fs/gfs2/meta_io.c b/fs/gfs2/meta_io.c
index 6492d000d366..47a430206801 100644
--- a/fs/gfs2/meta_io.c
+++ b/fs/gfs2/meta_io.c
@@ -243,13 +243,16 @@ static void gfs2_submit_bhs(blk_opf_t opf, struct buffer_head *bhs[], int num)
  * gfs2_meta_read_async - Read a block from disk
  * @gl: The glock covering the block
  * @blkno: The block number
+ * @fgp_flags: FGP_NOWAIT if sleeping is prohibited
  * @rahead: Do read-ahead
  * @bhp: the place where the buffer is returned (NULL on failure)
  *
+ * Returns -EAGAIN if the FGP_NOWAIT flag is set and the function would sleep.
+ *
  * Returns: errno
  */
 
-int gfs2_meta_read_async(struct gfs2_glock *gl, u64 blkno,
+int gfs2_meta_read_async(struct gfs2_glock *gl, u64 blkno, fgf_t fgp_flags,
 			 int rahead, struct buffer_head **bhp)
 {
 	struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
@@ -262,7 +265,29 @@ int gfs2_meta_read_async(struct gfs2_glock *gl, u64 blkno,
 		return -EIO;
 	}
 
-	*bhp = bh = gfs2_getbuf(gl, blkno, FGP_CREAT);
+	fgp_flags |= FGP_CREAT;
+	*bhp = bh = gfs2_getbuf(gl, blkno, fgp_flags);
+	if (IS_ERR(bh)) {
+		*bhp = NULL;
+		return PTR_ERR(bh);
+	}
+
+	if (fgp_flags & FGP_NOWAIT) {
+		bool uptodate = false;
+
+		/* skips readahead entirely. */
+
+		if (trylock_buffer(bh)) {
+			uptodate = buffer_uptodate(bh);
+			unlock_buffer(bh);
+		}
+		if (!uptodate) {
+			brelse(bh);
+			*bhp = NULL;
+			return -EAGAIN;
+		}
+		return 0;
+	}
 
 	lock_buffer(bh);
 	if (buffer_uptodate(bh)) {
@@ -274,7 +299,9 @@ int gfs2_meta_read_async(struct gfs2_glock *gl, u64 blkno,
 	}
 
 	if (rahead) {
-		bh = gfs2_getbuf(gl, blkno + 1, FGP_CREAT);
+		bh = gfs2_getbuf(gl, blkno + 1, fgp_flags);
+		if (IS_ERR(bh))
+			goto out;
 
 		lock_buffer(bh);
 		if (buffer_uptodate(bh)) {
@@ -286,6 +313,7 @@ int gfs2_meta_read_async(struct gfs2_glock *gl, u64 blkno,
 		}
 	}
 
+out:
 	gfs2_submit_bhs(REQ_OP_READ | REQ_META | REQ_PRIO, bhs, num);
 	return 0;
 }
@@ -315,13 +343,13 @@ int gfs2_meta_wait(struct gfs2_sbd *sdp, struct buffer_head *bh)
 	return 0;
 }
 
-int gfs2_meta_read(struct gfs2_glock *gl, u64 blkno,
+int gfs2_meta_read(struct gfs2_glock *gl, u64 blkno, fgf_t fgp_flags,
 		   int rahead, struct buffer_head **bhp)
 {
 	struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
 	int ret;
 
-	ret = gfs2_meta_read_async(gl, blkno, rahead, bhp);
+	ret = gfs2_meta_read_async(gl, blkno, fgp_flags, rahead, bhp);
 	if (ret)
 		return ret;
 
@@ -489,7 +517,7 @@ int gfs2_meta_buffer(struct gfs2_inode *ip, u32 mtype, u64 num,
 	if (num == ip->i_no_addr)
 		rahead = ip->i_rahead;
 
-	ret = gfs2_meta_read(gl, num, rahead, &bh);
+	ret = gfs2_meta_read(gl, num, 0, rahead, &bh);
 	if (ret == 0 && gfs2_metatype_check(sdp, bh, mtype)) {
 		brelse(bh);
 		ret = -EIO;
diff --git a/fs/gfs2/meta_io.h b/fs/gfs2/meta_io.h
index f04c91eadfb4..6ca37e9f1c95 100644
--- a/fs/gfs2/meta_io.h
+++ b/fs/gfs2/meta_io.h
@@ -51,10 +51,10 @@ static inline struct gfs2_sbd *gfs2_mapping2sbd(struct address_space *mapping)
 }
 
 struct buffer_head *gfs2_meta_new(struct gfs2_glock *gl, u64 blkno);
-int gfs2_meta_read_async(struct gfs2_glock *gl, u64 blkno,
+int gfs2_meta_read_async(struct gfs2_glock *gl, u64 blkno, fgf_t fgp_flags,
 			 int rahead, struct buffer_head **bhp);
 int gfs2_meta_wait(struct gfs2_sbd *sdp, struct buffer_head *bh);
-int gfs2_meta_read(struct gfs2_glock *gl, u64 blkno,
+int gfs2_meta_read(struct gfs2_glock *gl, u64 blkno, fgf_t fgp_flags,
 		   int rahead, struct buffer_head **bhp);
 struct buffer_head *gfs2_getbuf(struct gfs2_glock *gl, u64 blkno,
 				fgf_t fgp_flags);
diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index a68af8bdf7de..8b47306816ab 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -424,7 +424,7 @@ static int bh_get(struct gfs2_quota_data *qd)
 		goto fail;
 
 	error = gfs2_meta_read(ip->i_gl, iomap.addr >> inode->i_blkbits,
-			       0, &bh);
+			       0, 0, &bh);
 	if (error)
 		goto fail;
 	error = -EIO;
diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index fcef82c767e3..9fa4fc7f4bcf 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -1210,7 +1210,7 @@ int gfs2_rgrp_go_instantiate(struct gfs2_glock *gl)
 
 	for (x = 0; x < length; x++) {
 		bi = rgd->rd_bits + x;
-		error = gfs2_meta_read_async(gl, rgd->rd_addr + x, 0,
+		error = gfs2_meta_read_async(gl, rgd->rd_addr + x, 0, 0,
 					     &bi->bi_bh);
 		if (error)
 			goto fail;
diff --git a/fs/gfs2/xattr.c b/fs/gfs2/xattr.c
index 759347ec67af..745c7cf78519 100644
--- a/fs/gfs2/xattr.c
+++ b/fs/gfs2/xattr.c
@@ -128,7 +128,7 @@ static int ea_foreach(struct gfs2_inode *ip, ea_call_t ea_call, void *data)
 	__be64 *eablk, *end;
 	int error;
 
-	error = gfs2_meta_read(ip->i_gl, ip->i_eattr, 0, &bh);
+	error = gfs2_meta_read(ip->i_gl, ip->i_eattr, 0, 0, &bh);
 	if (error)
 		return error;
 
@@ -152,7 +152,7 @@ static int ea_foreach(struct gfs2_inode *ip, ea_call_t ea_call, void *data)
 			break;
 		bn = be64_to_cpu(*eablk);
 
-		error = gfs2_meta_read(ip->i_gl, bn, 0, &eabh);
+		error = gfs2_meta_read(ip->i_gl, bn, 0, 0, &eabh);
 		if (error)
 			break;
 		error = ea_foreach_i(ip, eabh, ea_call, data);
@@ -469,7 +469,7 @@ static int gfs2_iter_unstuffed(struct gfs2_inode *ip, struct gfs2_ea_header *ea,
 
 	for (x = 0; x < nptrs; x++) {
 		error = gfs2_meta_read_async(ip->i_gl, be64_to_cpu(*dataptrs),
-					     0, bh + x);
+					     0, 0, bh + x);
 		if (error) {
 			while (x--)
 				brelse(bh[x]);
@@ -976,7 +976,7 @@ static int ea_set_block(struct gfs2_inode *ip, struct gfs2_ea_request *er,
 	if (ip->i_diskflags & GFS2_DIF_EA_INDIRECT) {
 		__be64 *end;
 
-		error = gfs2_meta_read(ip->i_gl, ip->i_eattr, 0, &indbh);
+		error = gfs2_meta_read(ip->i_gl, ip->i_eattr, 0, 0, &indbh);
 		if (error)
 			return error;
 
@@ -1278,7 +1278,7 @@ static int ea_dealloc_indirect(struct gfs2_inode *ip)
 
 	memset(&rlist, 0, sizeof(struct gfs2_rgrp_list));
 
-	error = gfs2_meta_read(ip->i_gl, ip->i_eattr, 0, &indbh);
+	error = gfs2_meta_read(ip->i_gl, ip->i_eattr, 0, 0, &indbh);
 	if (error)
 		return error;
 
-- 
2.43.0

[PATCH 5/9] gfs2: Pass FGP flags to gfs2_meta_{,inode_}buffer

[Andreas Gruenbacher]

Pass a fgp_flags argument to gfs2_meta_buffer() and
gfs2_meta_inode_buffer().  If the FGP_NOWAIT flag is set and one of
these functions would sleep, -EAGAIN is returned instead.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/aops.c       |  4 ++--
 fs/gfs2/bmap.c       | 19 ++++++++++---------
 fs/gfs2/dir.c        | 20 ++++++++++----------
 fs/gfs2/file.c       |  4 ++--
 fs/gfs2/glops.c      |  2 +-
 fs/gfs2/inode.c      |  4 ++--
 fs/gfs2/meta_io.c    |  7 +++++--
 fs/gfs2/meta_io.h    |  7 ++++---
 fs/gfs2/ops_fstype.c |  2 +-
 fs/gfs2/recovery.c   |  2 +-
 fs/gfs2/rgrp.c       |  2 +-
 fs/gfs2/super.c      |  6 +++---
 fs/gfs2/xattr.c      |  4 ++--
 13 files changed, 44 insertions(+), 39 deletions(-)

diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 974aca9c8ea8..31da7f6926c9 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -424,7 +424,7 @@ static int stuffed_read_folio(struct gfs2_inode *ip, struct folio *folio)
 	if (unlikely(folio->index)) {
 		dsize = 0;
 	} else {
-		error = gfs2_meta_inode_buffer(ip, &dibh);
+		error = gfs2_meta_inode_buffer(ip, 0, &dibh);
 		if (error)
 			goto out;
 		from = dibh->b_data + sizeof(struct gfs2_dinode);
@@ -548,7 +548,7 @@ void adjust_fs_space(struct inode *inode)
 
 	/* Total up the file system space, according to the latest rindex. */
 	fs_total = gfs2_ri_total(sdp);
-	if (gfs2_meta_inode_buffer(m_ip, &m_bh) != 0)
+	if (gfs2_meta_inode_buffer(m_ip, 0, &m_bh) != 0)
 		goto out;
 
 	spin_lock(&sdp->sd_statfs_spin);
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 92945e5b7643..ffe19d3012f5 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -95,7 +95,7 @@ static int __gfs2_unstuff_inode(struct gfs2_inode *ip, struct folio *folio)
 	int isdir = gfs2_is_dir(ip);
 	int error;
 
-	error = gfs2_meta_inode_buffer(ip, &dibh);
+	error = gfs2_meta_inode_buffer(ip, 0, &dibh);
 	if (error)
 		return error;
 
@@ -331,7 +331,8 @@ static int __fillup_metapath(struct gfs2_inode *ip, struct metapath *mp,
 
 		if (!dblock)
 			break;
-		ret = gfs2_meta_buffer(ip, GFS2_METATYPE_IN, dblock, &mp->mp_bh[x + 1]);
+		ret = gfs2_meta_buffer(ip, GFS2_METATYPE_IN, dblock, 0,
+				       &mp->mp_bh[x + 1]);
 		if (ret)
 			return ret;
 	}
@@ -858,7 +859,7 @@ static int __gfs2_iomap_get(struct inode *inode, loff_t pos, loff_t length,
 
 	down_read(&ip->i_rw_mutex);
 
-	ret = gfs2_meta_inode_buffer(ip, &dibh);
+	ret = gfs2_meta_inode_buffer(ip, 0, &dibh);
 	if (ret)
 		goto unlock;
 	mp->mp_bh[0] = dibh;
@@ -1377,7 +1378,7 @@ static int trunc_start(struct inode *inode, u64 newsize)
 	if (error)
 		return error;
 
-	error = gfs2_meta_inode_buffer(ip, &dibh);
+	error = gfs2_meta_inode_buffer(ip, 0, &dibh);
 	if (error)
 		goto out;
 
@@ -1580,7 +1581,7 @@ static int sweep_bh_for_rgrps(struct gfs2_inode *ip, struct gfs2_holder *rd_gh,
 		if (current->journal_info) {
 			struct buffer_head *dibh;
 
-			ret = gfs2_meta_inode_buffer(ip, &dibh);
+			ret = gfs2_meta_inode_buffer(ip, 0, &dibh);
 			if (ret)
 				goto out;
 
@@ -1785,7 +1786,7 @@ static int punch_hole(struct gfs2_inode *ip, u64 offset, u64 length)
 	}
 	start_aligned = mp_h;
 
-	ret = gfs2_meta_inode_buffer(ip, &dibh);
+	ret = gfs2_meta_inode_buffer(ip, 0, &dibh);
 	if (ret)
 		return ret;
 
@@ -1985,7 +1986,7 @@ static int trunc_end(struct gfs2_inode *ip)
 
 	down_write(&ip->i_rw_mutex);
 
-	error = gfs2_meta_inode_buffer(ip, &dibh);
+	error = gfs2_meta_inode_buffer(ip, 0, &dibh);
 	if (error)
 		goto out;
 
@@ -2091,7 +2092,7 @@ static int do_grow(struct inode *inode, u64 size)
 			goto do_end_trans;
 	}
 
-	error = gfs2_meta_inode_buffer(ip, &dibh);
+	error = gfs2_meta_inode_buffer(ip, 0, &dibh);
 	if (error)
 		goto do_end_trans;
 
@@ -2345,7 +2346,7 @@ static int stuffed_zero_range(struct inode *inode, loff_t offset, loff_t length)
 	if (offset + length > inode->i_size)
 		length = inode->i_size - offset;
 
-	error = gfs2_meta_inode_buffer(ip, &dibh);
+	error = gfs2_meta_inode_buffer(ip, 0, &dibh);
 	if (error)
 		return error;
 	gfs2_trans_add_meta(ip->i_gl, dibh);
diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
index a1f47696a2ec..8719c8316e28 100644
--- a/fs/gfs2/dir.c
+++ b/fs/gfs2/dir.c
@@ -122,7 +122,7 @@ static int gfs2_dir_write_stuffed(struct gfs2_inode *ip, const char *buf,
 	struct buffer_head *dibh;
 	int error;
 
-	error = gfs2_meta_inode_buffer(ip, &dibh);
+	error = gfs2_meta_inode_buffer(ip, 0, &dibh);
 	if (error)
 		return error;
 
@@ -221,7 +221,7 @@ static int gfs2_dir_write_data(struct gfs2_inode *ip, const char *buf,
 	}
 
 out:
-	error = gfs2_meta_inode_buffer(ip, &dibh);
+	error = gfs2_meta_inode_buffer(ip, 0, &dibh);
 	if (error)
 		return error;
 
@@ -246,7 +246,7 @@ static int gfs2_dir_read_stuffed(struct gfs2_inode *ip, __be64 *buf,
 	struct buffer_head *dibh;
 	int error;
 
-	error = gfs2_meta_inode_buffer(ip, &dibh);
+	error = gfs2_meta_inode_buffer(ip, 0, &dibh);
 	if (!error) {
 		memcpy(buf, dibh->b_data + sizeof(struct gfs2_dinode), size);
 		brelse(dibh);
@@ -844,7 +844,7 @@ static struct gfs2_dirent *gfs2_dirent_search(struct inode *inode,
 	}
 
 
-	error = gfs2_meta_inode_buffer(ip, &bh);
+	error = gfs2_meta_inode_buffer(ip, 0, &bh);
 	if (error)
 		return ERR_PTR(error);
 	dent = gfs2_dirent_scan(inode, bh->b_data, bh->b_size, scan, name, NULL);
@@ -915,7 +915,7 @@ static int dir_make_exhash(struct inode *inode)
 	u64 bn;
 	int error;
 
-	error = gfs2_meta_inode_buffer(dip, &dibh);
+	error = gfs2_meta_inode_buffer(dip, 0, &dibh);
 	if (error)
 		return error;
 
@@ -1115,7 +1115,7 @@ static int dir_split_leaf(struct inode *inode, const struct qstr *name)
 
 	oleaf->lf_depth = nleaf->lf_depth;
 
-	error = gfs2_meta_inode_buffer(dip, &dibh);
+	error = gfs2_meta_inode_buffer(dip, 0, &dibh);
 	if (!gfs2_assert_withdraw(GFS2_SB(&dip->i_inode), !error)) {
 		gfs2_trans_add_meta(dip->i_gl, dibh);
 		gfs2_add_inode_blocks(&dip->i_inode, 1);
@@ -1169,7 +1169,7 @@ static int dir_double_exhash(struct gfs2_inode *dip)
 		return -ENOMEM;
 
 	h = hc2;
-	error = gfs2_meta_inode_buffer(dip, &dibh);
+	error = gfs2_meta_inode_buffer(dip, 0, &dibh);
 	if (error)
 		goto out_kfree;
 
@@ -1586,7 +1586,7 @@ int gfs2_dir_read(struct inode *inode, struct dir_context *ctx,
 		return -EIO;
 	}
 
-	error = gfs2_meta_inode_buffer(dip, &dibh);
+	error = gfs2_meta_inode_buffer(dip, 0, &dibh);
 	if (error)
 		return error;
 
@@ -1758,7 +1758,7 @@ static int dir_new_leaf(struct inode *inode, const struct qstr *name)
 	brelse(bh);
 	brelse(obh);
 
-	error = gfs2_meta_inode_buffer(ip, &bh);
+	error = gfs2_meta_inode_buffer(ip, 0, &bh);
 	if (error)
 		return error;
 	gfs2_trans_add_meta(ip->i_gl, bh);
@@ -2063,7 +2063,7 @@ static int leaf_dealloc(struct gfs2_inode *dip, u32 index, u32 len,
 		goto out_end_trans;
 	}
 
-	error = gfs2_meta_inode_buffer(dip, &dibh);
+	error = gfs2_meta_inode_buffer(dip, 0, &dibh);
 	if (error)
 		goto out_end_trans;
 
diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index 992ca4effb50..ac4ed6abfc6c 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -257,7 +257,7 @@ static int do_gfs2_set_flags(struct inode *inode, u32 reqflags, u32 mask)
 	error = gfs2_trans_begin(sdp, RES_DINODE, 0);
 	if (error)
 		goto out;
-	error = gfs2_meta_inode_buffer(ip, &bh);
+	error = gfs2_meta_inode_buffer(ip, 0, &bh);
 	if (error)
 		goto out_trans_end;
 	inode_set_ctime_current(inode);
@@ -1179,7 +1179,7 @@ static int fallocate_chunk(struct inode *inode, loff_t offset, loff_t len,
 	struct buffer_head *dibh;
 	int error;
 
-	error = gfs2_meta_inode_buffer(ip, &dibh);
+	error = gfs2_meta_inode_buffer(ip, 0, &dibh);
 	if (unlikely(error))
 		return error;
 
diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
index 45653cbc8a87..cc38aa81d246 100644
--- a/fs/gfs2/glops.c
+++ b/fs/gfs2/glops.c
@@ -483,7 +483,7 @@ int gfs2_inode_refresh(struct gfs2_inode *ip)
 	struct buffer_head *dibh;
 	int error;
 
-	error = gfs2_meta_inode_buffer(ip, &dibh);
+	error = gfs2_meta_inode_buffer(ip, 0, &dibh);
 	if (error)
 		return error;
 
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 6bfc9383b7b8..a417650e378f 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -1027,7 +1027,7 @@ static int gfs2_link(struct dentry *old_dentry, struct inode *dir,
 			goto out_ipres;
 	}
 
-	error = gfs2_meta_inode_buffer(ip, &dibh);
+	error = gfs2_meta_inode_buffer(ip, 0, &dibh);
 	if (error)
 		goto out_end_trans;
 
@@ -1833,7 +1833,7 @@ static const char *gfs2_get_link(struct dentry *dentry,
 		goto out;
 	}
 
-	error = gfs2_meta_inode_buffer(ip, &dibh);
+	error = gfs2_meta_inode_buffer(ip, 0, &dibh);
 	if (error) {
 		buf = ERR_PTR(error);
 		goto out;
diff --git a/fs/gfs2/meta_io.c b/fs/gfs2/meta_io.c
index 47a430206801..c8750b337496 100644
--- a/fs/gfs2/meta_io.c
+++ b/fs/gfs2/meta_io.c
@@ -500,13 +500,16 @@ void gfs2_journal_wipe(struct gfs2_inode *ip, u64 bstart, u32 blen)
  * @ip: The GFS2 inode
  * @mtype: The block type (GFS2_METATYPE_*)
  * @num: The block number (device relative) of the buffer
+ * @fgp_flags: FGP_NOWAIT if sleeping is prohibited
  * @bhp: the buffer is returned here
  *
+ * Returns -EAGAIN if the FGP_NOWAIT flag is set and the function would sleep.
+ *
  * Returns: errno
  */
 
 int gfs2_meta_buffer(struct gfs2_inode *ip, u32 mtype, u64 num,
-		     struct buffer_head **bhp)
+		     fgf_t fgp_flags, struct buffer_head **bhp)
 {
 	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
 	struct gfs2_glock *gl = ip->i_gl;
@@ -517,7 +520,7 @@ int gfs2_meta_buffer(struct gfs2_inode *ip, u32 mtype, u64 num,
 	if (num == ip->i_no_addr)
 		rahead = ip->i_rahead;
 
-	ret = gfs2_meta_read(gl, num, 0, rahead, &bh);
+	ret = gfs2_meta_read(gl, num, fgp_flags, rahead, &bh);
 	if (ret == 0 && gfs2_metatype_check(sdp, bh, mtype)) {
 		brelse(bh);
 		ret = -EIO;
diff --git a/fs/gfs2/meta_io.h b/fs/gfs2/meta_io.h
index 6ca37e9f1c95..7b5122078113 100644
--- a/fs/gfs2/meta_io.h
+++ b/fs/gfs2/meta_io.h
@@ -66,12 +66,13 @@ enum {
 void gfs2_remove_from_journal(struct buffer_head *bh, int meta);
 void gfs2_journal_wipe(struct gfs2_inode *ip, u64 bstart, u32 blen);
 int gfs2_meta_buffer(struct gfs2_inode *ip, u32 mtype, u64 num,
-		     struct buffer_head **bhp);
+		     fgf_t fgp_flags, struct buffer_head **bhp);
 
-static inline int gfs2_meta_inode_buffer(struct gfs2_inode *ip,
+static inline int gfs2_meta_inode_buffer(struct gfs2_inode *ip, fgf_t fgp_flags,
 					 struct buffer_head **bhp)
 {
-	return gfs2_meta_buffer(ip, GFS2_METATYPE_DI, ip->i_no_addr, bhp);
+	return gfs2_meta_buffer(ip, GFS2_METATYPE_DI, ip->i_no_addr, fgp_flags,
+				bhp);
 }
 
 struct buffer_head *gfs2_meta_ra(struct gfs2_glock *gl, u64 dblock, u32 extlen);
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 1281e60be639..1a01adb48f5b 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -698,7 +698,7 @@ static int init_statfs(struct gfs2_sbd *sdp)
 		goto free_local;
 	}
 	/* read in the local statfs buffer - other nodes don't change it. */
-	error = gfs2_meta_inode_buffer(ip, &sdp->sd_sc_bh);
+	error = gfs2_meta_inode_buffer(ip, 0, &sdp->sd_sc_bh);
 	if (error) {
 		fs_err(sdp, "Cannot read in local statfs: %d\n", error);
 		goto unlock_sd_gh;
diff --git a/fs/gfs2/recovery.c b/fs/gfs2/recovery.c
index f4fe7039f725..4db3ca9c3b02 100644
--- a/fs/gfs2/recovery.c
+++ b/fs/gfs2/recovery.c
@@ -317,7 +317,7 @@ static int update_statfs_inode(struct gfs2_jdesc *jd,
 	BUG_ON(!inode);
 	ip = GFS2_I(inode);
 
-	error = gfs2_meta_inode_buffer(ip, &bh);
+	error = gfs2_meta_inode_buffer(ip, 0, &bh);
 	if (error)
 		goto out;
 
diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index 9fa4fc7f4bcf..c9aec3dba80c 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -2452,7 +2452,7 @@ int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *nblocks,
 	rbm.rgd->rd_last_alloc = block - rbm.rgd->rd_data0;
 	if (!dinode) {
 		ip->i_goal = block + *nblocks - 1;
-		error = gfs2_meta_inode_buffer(ip, &dibh);
+		error = gfs2_meta_inode_buffer(ip, 0, &dibh);
 		if (error == 0) {
 			struct gfs2_dinode *di =
 				(struct gfs2_dinode *)dibh->b_data;
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index e5f79466340d..7bfdd5924a54 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -192,7 +192,7 @@ int gfs2_statfs_init(struct gfs2_sbd *sdp)
 	if (error)
 		return error;
 
-	error = gfs2_meta_inode_buffer(m_ip, &m_bh);
+	error = gfs2_meta_inode_buffer(m_ip, 0, &m_bh);
 	if (error)
 		goto out;
 
@@ -282,7 +282,7 @@ int gfs2_statfs_sync(struct super_block *sb, int type)
 	if (error)
 		goto out;
 
-	error = gfs2_meta_inode_buffer(m_ip, &m_bh);
+	error = gfs2_meta_inode_buffer(m_ip, 0, &m_bh);
 	if (error)
 		goto out_unlock;
 
@@ -521,7 +521,7 @@ static void gfs2_dirty_inode(struct inode *inode, int flags)
 		need_endtrans = 1;
 	}
 
-	ret = gfs2_meta_inode_buffer(ip, &bh);
+	ret = gfs2_meta_inode_buffer(ip, 0, &bh);
 	if (ret == 0) {
 		gfs2_trans_add_meta(ip->i_gl, bh);
 		gfs2_dinode_out(ip, bh->b_data);
diff --git a/fs/gfs2/xattr.c b/fs/gfs2/xattr.c
index 745c7cf78519..e2ff0fc8e445 100644
--- a/fs/gfs2/xattr.c
+++ b/fs/gfs2/xattr.c
@@ -1360,7 +1360,7 @@ static int ea_dealloc_indirect(struct gfs2_inode *ip)
 
 	ip->i_diskflags &= ~GFS2_DIF_EA_INDIRECT;
 
-	error = gfs2_meta_inode_buffer(ip, &dibh);
+	error = gfs2_meta_inode_buffer(ip, 0, &dibh);
 	if (!error) {
 		gfs2_trans_add_meta(ip->i_gl, dibh);
 		gfs2_dinode_out(ip, dibh->b_data);
@@ -1412,7 +1412,7 @@ static int ea_dealloc_block(struct gfs2_inode *ip)
 	gfs2_add_inode_blocks(&ip->i_inode, -1);
 
 	if (likely(!test_bit(GIF_ALLOC_FAILED, &ip->i_flags))) {
-		error = gfs2_meta_inode_buffer(ip, &dibh);
+		error = gfs2_meta_inode_buffer(ip, 0, &dibh);
 		if (!error) {
 			gfs2_trans_add_meta(ip->i_gl, dibh);
 			gfs2_dinode_out(ip, dibh->b_data);
-- 
2.43.0

[PATCH 6/9] gfs2: Pass FGP flags to gfs2_dirent_search

[Andreas Gruenbacher]

Add an fgp_flags argument to gfs2_dirent_search().  When the FGP_NOWAIT
flag is set and gfs2_dirent_search() would sleep, -EAGAIN is returned
instead.

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

diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
index 8719c8316e28..81d1ea61f44f 100644
--- a/fs/gfs2/dir.c
+++ b/fs/gfs2/dir.c
@@ -805,6 +805,7 @@ static int get_first_leaf(struct gfs2_inode *dip, u32 index,
 static struct gfs2_dirent *gfs2_dirent_search(struct inode *inode,
 					      const struct qstr *name,
 					      gfs2_dscan_t scan,
+					      fgf_t fgp_flags,
 					      struct buffer_head **pbh)
 {
 	struct buffer_head *bh;
@@ -817,6 +818,11 @@ static struct gfs2_dirent *gfs2_dirent_search(struct inode *inode,
 		unsigned int hsize = BIT(ip->i_depth);
 		unsigned int index;
 		u64 ln;
+
+		/* no lockless lookup inside ExHash directories */
+		if (fgp_flags & FGP_NOWAIT)
+			return ERR_PTR(-EAGAIN);
+
 		if (hsize * sizeof(u64) != i_size_read(inode)) {
 			gfs2_consist_inode(ip);
 			return ERR_PTR(-EIO);
@@ -843,8 +849,7 @@ static struct gfs2_dirent *gfs2_dirent_search(struct inode *inode,
 		return error ? ERR_PTR(error) : NULL;
 	}
 
-
-	error = gfs2_meta_inode_buffer(ip, 0, &bh);
+	error = gfs2_meta_inode_buffer(ip, fgp_flags, &bh);
 	if (error)
 		return ERR_PTR(error);
 	dent = gfs2_dirent_scan(inode, bh->b_data, bh->b_size, scan, name, NULL);
@@ -1647,7 +1652,7 @@ struct inode *gfs2_dir_search(struct inode *dir, const struct qstr *name,
 	u64 addr, formal_ino;
 	u16 dtype;
 
-	dent = gfs2_dirent_search(dir, name, gfs2_dirent_find, &bh);
+	dent = gfs2_dirent_search(dir, name, gfs2_dirent_find, 0, &bh);
 	if (dent) {
 		struct inode *inode;
 		u16 rahead;
@@ -1677,7 +1682,7 @@ int gfs2_dir_check(struct inode *dir, const struct qstr *name,
 	struct gfs2_dirent *dent;
 	int ret = -ENOENT;
 
-	dent = gfs2_dirent_search(dir, name, gfs2_dirent_find, &bh);
+	dent = gfs2_dirent_search(dir, name, gfs2_dirent_find, 0, &bh);
 	if (dent) {
 		if (IS_ERR(dent))
 			return PTR_ERR(dent);
@@ -1805,7 +1810,8 @@ int gfs2_dir_add(struct inode *inode, const struct qstr *name,
 	while(1) {
 		if (da->bh == NULL) {
 			dent = gfs2_dirent_search(inode, name,
-						  gfs2_dirent_find_space, &bh);
+						  gfs2_dirent_find_space, 0,
+						  &bh);
 		}
 		if (dent) {
 			if (IS_ERR(dent))
@@ -1880,7 +1886,8 @@ int gfs2_dir_del(struct gfs2_inode *dip, const struct dentry *dentry)
 
 	/* Returns _either_ the entry (if its first in block) or the
 	   previous entry otherwise */
-	dent = gfs2_dirent_search(&dip->i_inode, name, gfs2_dirent_prev, &bh);
+	dent = gfs2_dirent_search(&dip->i_inode, name, gfs2_dirent_prev, 0,
+				  &bh);
 	if (!dent) {
 		gfs2_consist_inode(dip);
 		return -EIO;
@@ -1939,7 +1946,8 @@ int gfs2_dir_mvino(struct gfs2_inode *dip, const struct qstr *filename,
 	struct buffer_head *bh;
 	struct gfs2_dirent *dent;
 
-	dent = gfs2_dirent_search(&dip->i_inode, filename, gfs2_dirent_find, &bh);
+	dent = gfs2_dirent_search(&dip->i_inode, filename, gfs2_dirent_find, 0,
+				  &bh);
 	if (!dent) {
 		gfs2_consist_inode(dip);
 		return -EIO;
@@ -2166,7 +2174,7 @@ int gfs2_diradd_alloc_required(struct inode *inode, const struct qstr *name,
 	da->bh = NULL;
 	da->dent = NULL;
 
-	dent = gfs2_dirent_search(inode, name, gfs2_dirent_find_space, &bh);
+	dent = gfs2_dirent_search(inode, name, gfs2_dirent_find_space, 0, &bh);
 	if (!dent) {
 		da->nr_blocks = sdp->sd_max_dirres;
 		if (!(ip->i_diskflags & GFS2_DIF_EXHASH) &&
-- 
2.43.0

[PATCH 7/9] gfs2: Pass FGP flags to gfs2_dir_check

[Andreas Gruenbacher]

Pass a fgp_flags argument to gfs2_dir_check().  If the FGP_NOWAIT flag
is set and gfs2_dir_check() would sleep, -EAGAIN is returned instead.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/dentry.c     | 2 +-
 fs/gfs2/dir.c        | 4 ++--
 fs/gfs2/dir.h        | 2 +-
 fs/gfs2/inode.c      | 6 +++---
 fs/gfs2/ops_fstype.c | 2 +-
 5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/gfs2/dentry.c b/fs/gfs2/dentry.c
index c6483fb98624..f179a6d94cbe 100644
--- a/fs/gfs2/dentry.c
+++ b/fs/gfs2/dentry.c
@@ -72,7 +72,7 @@ static int gfs2_drevalidate(struct dentry *dentry, unsigned int flags)
 			goto out;
 	}
 
-	error = gfs2_dir_check(dinode, &dentry->d_name, ip);
+	error = gfs2_dir_check(dinode, &dentry->d_name, ip, 0);
 	valid = inode ? !error : (error == -ENOENT);
 
 	if (!had_lock)
diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
index 81d1ea61f44f..d3a9c9094db0 100644
--- a/fs/gfs2/dir.c
+++ b/fs/gfs2/dir.c
@@ -1676,13 +1676,13 @@ struct inode *gfs2_dir_search(struct inode *dir, const struct qstr *name,
 }
 
 int gfs2_dir_check(struct inode *dir, const struct qstr *name,
-		   const struct gfs2_inode *ip)
+		   const struct gfs2_inode *ip, fgf_t fgp_flags)
 {
 	struct buffer_head *bh;
 	struct gfs2_dirent *dent;
 	int ret = -ENOENT;
 
-	dent = gfs2_dirent_search(dir, name, gfs2_dirent_find, 0, &bh);
+	dent = gfs2_dirent_search(dir, name, gfs2_dirent_find, fgp_flags, &bh);
 	if (dent) {
 		if (IS_ERR(dent))
 			return PTR_ERR(dent);
diff --git a/fs/gfs2/dir.h b/fs/gfs2/dir.h
index 25a857c78b53..cfec869a11c8 100644
--- a/fs/gfs2/dir.h
+++ b/fs/gfs2/dir.h
@@ -27,7 +27,7 @@ struct inode *gfs2_dir_search(struct inode *dir,
 			      const struct qstr *filename,
 			      bool fail_on_exist);
 int gfs2_dir_check(struct inode *dir, const struct qstr *filename,
-		   const struct gfs2_inode *ip);
+		   const struct gfs2_inode *ip, fgf_t fgp_flags);
 int gfs2_dir_add(struct inode *inode, const struct qstr *filename,
 		 const struct gfs2_inode *ip, struct gfs2_diradd *da);
 static inline void gfs2_dir_no_add(struct gfs2_diradd *da)
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index a417650e378f..948f5c203a38 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -980,7 +980,7 @@ static int gfs2_link(struct dentry *old_dentry, struct inode *dir,
 	if (error)
 		goto out_gunlock;
 
-	error = gfs2_dir_check(dir, &dentry->d_name, NULL);
+	error = gfs2_dir_check(dir, &dentry->d_name, NULL, 0);
 	switch (error) {
 	case -ENOENT:
 		break;
@@ -1096,7 +1096,7 @@ static int gfs2_unlink_ok(struct gfs2_inode *dip, const struct qstr *name,
 	if (error)
 		return error;
 
-	return gfs2_dir_check(&dip->i_inode, name, ip);
+	return gfs2_dir_check(&dip->i_inode, name, ip, 0);
 }
 
 /**
@@ -1522,7 +1522,7 @@ static int gfs2_rename(struct inode *odir, struct dentry *odentry,
 		if (error)
 			goto out_gunlock;
 
-		error = gfs2_dir_check(ndir, &ndentry->d_name, NULL);
+		error = gfs2_dir_check(ndir, &ndentry->d_name, NULL, 0);
 		switch (error) {
 		case -ENOENT:
 			error = 0;
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 1a01adb48f5b..f6c80ff618cc 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -584,7 +584,7 @@ static int gfs2_jindex_hold(struct gfs2_sbd *sdp, struct gfs2_holder *ji_gh)
 		name.len = sprintf(buf, "journal%u", sdp->sd_journals);
 		name.hash = gfs2_disk_hash(name.name, name.len);
 
-		error = gfs2_dir_check(sdp->sd_jindex, &name, NULL);
+		error = gfs2_dir_check(sdp->sd_jindex, &name, NULL, 0);
 		if (error == -ENOENT) {
 			error = 0;
 			break;
-- 
2.43.0

[PATCH 8/9] gfs2: Minor gfs2_drevalidate cleanup

[Andreas Gruenbacher]

Get rid of the had_lock and valid variables in gfs2_drevalidate().

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

diff --git a/fs/gfs2/dentry.c b/fs/gfs2/dentry.c
index f179a6d94cbe..99239d80982b 100644
--- a/fs/gfs2/dentry.c
+++ b/fs/gfs2/dentry.c
@@ -38,8 +38,9 @@ static int gfs2_drevalidate(struct dentry *dentry, unsigned int flags)
 	struct inode *dinode, *inode;
 	struct gfs2_holder d_gh;
 	struct gfs2_inode *ip = NULL;
-	int error, valid = 0;
-	int had_lock = 0;
+	int error;
+
+	gfs2_holder_mark_uninitialized(&d_gh);
 
 	if (flags & LOOKUP_RCU) {
 		dinode = d_inode_rcu(READ_ONCE(dentry->d_parent));
@@ -54,18 +55,19 @@ static int gfs2_drevalidate(struct dentry *dentry, unsigned int flags)
 	inode = d_inode(dentry);
 
 	if (inode) {
-		if (is_bad_inode(inode))
+		if (is_bad_inode(inode)) {
+			error = 0;
 			goto out;
+		}
 		ip = GFS2_I(inode);
 	}
 
 	if (sdp->sd_lockstruct.ls_ops->lm_mount == NULL) {
-		valid = 1;
+		error = 1;
 		goto out;
 	}
 
-	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)) {
 		error = gfs2_glock_nq_init(dip->i_gl, LM_ST_SHARED,
 					   flags & LOOKUP_RCU ? GL_NOBLOCK : 0, &d_gh);
 		if (error)
@@ -73,13 +75,13 @@ static int gfs2_drevalidate(struct dentry *dentry, unsigned int flags)
 	}
 
 	error = gfs2_dir_check(dinode, &dentry->d_name, ip, 0);
-	valid = inode ? !error : (error == -ENOENT);
+	error = inode ? !error : (error == -ENOENT);
 
-	if (!had_lock)
+	if (gfs2_holder_initialized(&d_gh))
 		gfs2_glock_dq_uninit(&d_gh);
 out:
 	dput(parent);
-	return valid;
+	return error;
 }
 
 static int gfs2_dhash(const struct dentry *dentry, struct qstr *str)
-- 
2.43.0

[PATCH 9/9] gfs2: Fix LOOKUP_RCU support in gfs2_drevalidate

[Andreas Gruenbacher]

Fix non-sleeping lookups in gfs2_drevalidate() by passing the FGP_NOWAIT
flag down to gfs2_dir_check().  This will cause gfs2_dir_check() to
return -EAGAIN when it would otherwise sleep.

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

diff --git a/fs/gfs2/dentry.c b/fs/gfs2/dentry.c
index 99239d80982b..b14956cdab0e 100644
--- a/fs/gfs2/dentry.c
+++ b/fs/gfs2/dentry.c
@@ -74,12 +74,20 @@ static int gfs2_drevalidate(struct dentry *dentry, unsigned int flags)
 			goto out;
 	}
 
-	error = gfs2_dir_check(dinode, &dentry->d_name, ip, 0);
+	if (flags & LOOKUP_RCU) {
+		error = gfs2_dir_check(dinode, &dentry->d_name, ip, FGP_NOWAIT);
+		if (error == -EAGAIN) {
+			error = -ECHILD;
+			goto out;
+		}
+	} else {
+		error = gfs2_dir_check(dinode, &dentry->d_name, ip, 0);
+	}
 	error = inode ? !error : (error == -ENOENT);
 
+out:
 	if (gfs2_holder_initialized(&d_gh))
 		gfs2_glock_dq_uninit(&d_gh);
-out:
 	dput(parent);
 	return error;
 }
-- 
2.43.0

Re: [PATCH 0/9] gfs2: Bugs in "Use GL_NOBLOCK flag for non-blocking lookups"

[Al Viro]

On Fri, Jan 19, 2024 at 10:20:47PM +0100, Andreas Gruenbacher wrote:
> Hello,
> 
> Al Viro has reported issues with commit dd00aaeb3432 ("gfs2: Use
> GL_NOBLOCK flag for non-blocking lookups"):
> 
> * First, parent can now be NULL and dereferencing it in
>   gfs2_dir_check(d_inode(parent), &dentry->d_name, ip) isn't going to
>   work;

Nit: parent can't be NULL; what can be NULL in RCU mode is d_inode(parent) when
sampled inside ->d_revalidate().  IOW, it's not d_inode(parent) evaluation
that breaks, but passing the resulting NULL inode pointer to gfs2_dir_check().

> * Second, gfs2_dir_check() can still sleep, which breaks LOOKUP_RCU
>   mode.
> 
> These patches should fix both of these problems.  We'll either need
> these fixes added, or commit dd00aaeb3432 reverted for now.

Re: [PATCH 0/9] gfs2: Bugs in "Use GL_NOBLOCK flag for non-blocking lookups"

[Al Viro]

On Sat, Jan 20, 2024 at 01:36:39AM +0000, Al Viro wrote:
> On Fri, Jan 19, 2024 at 10:20:47PM +0100, Andreas Gruenbacher wrote:
> > Hello,
> > 
> > Al Viro has reported issues with commit dd00aaeb3432 ("gfs2: Use
> > GL_NOBLOCK flag for non-blocking lookups"):
> > 
> > * First, parent can now be NULL and dereferencing it in
> >   gfs2_dir_check(d_inode(parent), &dentry->d_name, ip) isn't going to
> >   work;
> 
> Nit: parent can't be NULL; what can be NULL in RCU mode is d_inode(parent) when
> sampled inside ->d_revalidate().  IOW, it's not d_inode(parent) evaluation
> that breaks, but passing the resulting NULL inode pointer to gfs2_dir_check().

Nevermind, the way it's written parent will be NULL in RCU case.  Sorry about
the noise.

Re: [PATCH 0/9] gfs2: Bugs in "Use GL_NOBLOCK flag for non-blocking lookups"

[Andrew Price]

On 19/01/2024 21:20, Andreas Gruenbacher wrote:
> Hello,
> 
> Al Viro has reported issues with commit dd00aaeb3432 ("gfs2: Use
> GL_NOBLOCK flag for non-blocking lookups"):
> 
> * First, parent can now be NULL and dereferencing it in
>    gfs2_dir_check(d_inode(parent), &dentry->d_name, ip) isn't going to
>    work;
> 
> * Second, gfs2_dir_check() can still sleep, which breaks LOOKUP_RCU
>    mode.
> 
> These patches should fix both of these problems.  We'll either need
> these fixes added, or commit dd00aaeb3432 reverted for now.

No issues spotted. I like the gfs2_meta_read() improvement. If it helps:

Reviewed-by: Andrew Price <anprice@redhat.com>

Andy

> Thanks,
> Andreas
> 
> Andreas Gruenbacher (9):
>    gfs2: Fix gfs2_drevalidate NULL pointer dereference
>    gfs2: Pass FGP flags to gfs2_getbuf
>    gfs2: Split gfs2_meta_read_async off from gfs2_meta_read
>    gfs2: Add FGP_NOWAIT support to gfs2_meta_read_async
>    gfs2: Pass FGP flags to gfs2_meta_{,inode_}buffer
>    gfs2: Pass FGP flags to gfs2_dirent_search
>    gfs2: Pass FGP flags to gfs2_dir_check
>    gfs2: Minor gfs2_drevalidate cleanup
>    gfs2: Fix LOOKUP_RCU support in gfs2_drevalidate
> 
>   fs/gfs2/aops.c       |   4 +-
>   fs/gfs2/bmap.c       |  21 ++++----
>   fs/gfs2/dentry.c     |  32 ++++++++-----
>   fs/gfs2/dir.c        |  52 +++++++++++---------
>   fs/gfs2/dir.h        |   2 +-
>   fs/gfs2/file.c       |   4 +-
>   fs/gfs2/glops.c      |   2 +-
>   fs/gfs2/incore.h     |   1 -
>   fs/gfs2/inode.c      |  10 ++--
>   fs/gfs2/meta_io.c    | 112 ++++++++++++++++++++++++++++---------------
>   fs/gfs2/meta_io.h    |  15 +++---
>   fs/gfs2/ops_fstype.c |   4 +-
>   fs/gfs2/quota.c      |   2 +-
>   fs/gfs2/recovery.c   |   2 +-
>   fs/gfs2/rgrp.c       |   5 +-
>   fs/gfs2/super.c      |   6 +--
>   fs/gfs2/xattr.c      |  17 ++++---
>   17 files changed, 173 insertions(+), 118 deletions(-)
> 

Re: [PATCH 0/9] gfs2: Bugs in "Use GL_NOBLOCK flag for non-blocking lookups"

[Al Viro]

On Fri, Jan 19, 2024 at 10:20:47PM +0100, Andreas Gruenbacher wrote:
> Hello,
> 
> Al Viro has reported issues with commit dd00aaeb3432 ("gfs2: Use
> GL_NOBLOCK flag for non-blocking lookups"):
> 
> * First, parent can now be NULL and dereferencing it in
>   gfs2_dir_check(d_inode(parent), &dentry->d_name, ip) isn't going to
>   work;
> 
> * Second, gfs2_dir_check() can still sleep, which breaks LOOKUP_RCU
>   mode.

Looks sane, but there's another piece of fun in gfs_permission():
        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(ip->i_gl) == NULL) {
                int noblock = may_not_block ? GL_NOBLOCK : 0;
                error = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED,
                                           LM_FLAG_ANY | noblock, &i_gh);
                if (error)
                        return error;
        }

See the problem?  In RCU mode we carefully fetch ->i_gl and verify
it's not NULL.  Then we proceed to dereference it again.

IOW, these ip->i_gl below ought to be replaced with gl, or you are
risking a compiler fetching the sucker again and getting NULL this
time around.

Re: [PATCH 0/9] gfs2: Bugs in "Use GL_NOBLOCK flag for non-blocking lookups"

[Al Viro]

On Fri, Feb 02, 2024 at 04:23:13AM +0000, Al Viro wrote:
> On Fri, Jan 19, 2024 at 10:20:47PM +0100, Andreas Gruenbacher wrote:
> > Hello,
> > 
> > Al Viro has reported issues with commit dd00aaeb3432 ("gfs2: Use
> > GL_NOBLOCK flag for non-blocking lookups"):
> > 
> > * First, parent can now be NULL and dereferencing it in
> >   gfs2_dir_check(d_inode(parent), &dentry->d_name, ip) isn't going to
> >   work;
> > 
> > * Second, gfs2_dir_check() can still sleep, which breaks LOOKUP_RCU
> >   mode.
> 
> Looks sane, but there's another piece of fun in gfs_permission():
>         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(ip->i_gl) == NULL) {
>                 int noblock = may_not_block ? GL_NOBLOCK : 0;
>                 error = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED,
>                                            LM_FLAG_ANY | noblock, &i_gh);
>                 if (error)
>                         return error;
>         }
> 
> See the problem?  In RCU mode we carefully fetch ->i_gl and verify
> it's not NULL.  Then we proceed to dereference it again.
> 
> IOW, these ip->i_gl below ought to be replaced with gl, or you are
> risking a compiler fetching the sucker again and getting NULL this
> time around.

fix breakage in gfs2_permission()

"gfs2: Use GL_NOBLOCK flag for non-blocking lookups" has
reintroduced the bug fixed in "gfs2: fix an oops in
gfs2_permission" a while ago.  We still fetch ->i_gl carefully,
and check if it's NULL, but then we proceed to fetch it again.
If it became NULL in the meanwhile, we get trouble...

Trivial to fix, fortunately.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 6bfc9383b7b8..d045562bf08f 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -1882,9 +1882,9 @@ int gfs2_permission(struct mnt_idmap *idmap, struct inode *inode,
 		WARN_ON_ONCE(!may_not_block);
 		return -ECHILD;
         }
-	if (gfs2_glock_is_locked_by_me(ip->i_gl) == NULL) {
+	if (gfs2_glock_is_locked_by_me(gl) == NULL) {
 		int noblock = may_not_block ? GL_NOBLOCK : 0;
-		error = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED,
+		error = gfs2_glock_nq_init(gl, LM_ST_SHARED,
 					   LM_FLAG_ANY | noblock, &i_gh);
 		if (error)
 			return error;

Re: [PATCH 0/9] gfs2: Bugs in "Use GL_NOBLOCK flag for non-blocking lookups"

[Al Viro]

On Fri, Feb 02, 2024 at 04:23:13AM +0000, Al Viro wrote:

>         if (gfs2_glock_is_locked_by_me(ip->i_gl) == NULL) {
>                 int noblock = may_not_block ? GL_NOBLOCK : 0;
>                 error = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED,
>                                            LM_FLAG_ANY | noblock, &i_gh);
>                 if (error)
>                         return error;
>         }
> 
> See the problem?  In RCU mode we carefully fetch ->i_gl and verify
> it's not NULL.  Then we proceed to dereference it again.
> 
> IOW, these ip->i_gl below ought to be replaced with gl, or you are
> risking a compiler fetching the sucker again and getting NULL this
> time around.

Wait, what's going on with this part?
        if (gfs2_holder_initialized(&i_gh))
		gfs2_glock_dq_uninit(&i_gh);

First of all, gfs2_glock_dq_uninit() can bloody well sleep in
gfs2_glock_dq().  What's more, it drops a reference to ->gh_gl,
i.e. our gl, in gfs2_holder_uninit().  Where is the matching
increment of gl's refcount?

Re: [PATCH 0/9] gfs2: Bugs in "Use GL_NOBLOCK flag for non-blocking lookups"

[Al Viro]

On Fri, Feb 02, 2024 at 04:59:44AM +0000, Al Viro wrote:
> On Fri, Feb 02, 2024 at 04:23:13AM +0000, Al Viro wrote:
> 
> >         if (gfs2_glock_is_locked_by_me(ip->i_gl) == NULL) {
> >                 int noblock = may_not_block ? GL_NOBLOCK : 0;
> >                 error = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED,
> >                                            LM_FLAG_ANY | noblock, &i_gh);
> >                 if (error)
> >                         return error;
> >         }
> > 
> > See the problem?  In RCU mode we carefully fetch ->i_gl and verify
> > it's not NULL.  Then we proceed to dereference it again.
> > 
> > IOW, these ip->i_gl below ought to be replaced with gl, or you are
> > risking a compiler fetching the sucker again and getting NULL this
> > time around.
> 
> Wait, what's going on with this part?
>         if (gfs2_holder_initialized(&i_gh))
> 		gfs2_glock_dq_uninit(&i_gh);
> 
> First of all, gfs2_glock_dq_uninit() can bloody well sleep in
> gfs2_glock_dq().  What's more, it drops a reference to ->gh_gl,
> i.e. our gl, in gfs2_holder_uninit().  Where is the matching
> increment of gl's refcount?

The same applies to gfs2_drevalidate(), both before and after your
fixes.  Am I missing something subtle here?

Re: [PATCH 0/9] gfs2: Bugs in "Use GL_NOBLOCK flag for non-blocking lookups"

[Andreas Gruenbacher]

Hi Al,

On Fri, Feb 2, 2024 at 5:34 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Fri, Feb 02, 2024 at 04:23:13AM +0000, Al Viro wrote:
> > On Fri, Jan 19, 2024 at 10:20:47PM +0100, Andreas Gruenbacher wrote:
> > > Hello,
> > >
> > > Al Viro has reported issues with commit dd00aaeb3432 ("gfs2: Use
> > > GL_NOBLOCK flag for non-blocking lookups"):
> > >
> > > * First, parent can now be NULL and dereferencing it in
> > >   gfs2_dir_check(d_inode(parent), &dentry->d_name, ip) isn't going to
> > >   work;
> > >
> > > * Second, gfs2_dir_check() can still sleep, which breaks LOOKUP_RCU
> > >   mode.
> >
> > Looks sane, but there's another piece of fun in gfs_permission():
> >         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(ip->i_gl) == NULL) {
> >                 int noblock = may_not_block ? GL_NOBLOCK : 0;
> >                 error = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED,
> >                                            LM_FLAG_ANY | noblock, &i_gh);
> >                 if (error)
> >                         return error;
> >         }
> >
> > See the problem?  In RCU mode we carefully fetch ->i_gl and verify
> > it's not NULL.  Then we proceed to dereference it again.
> >
> > IOW, these ip->i_gl below ought to be replaced with gl, or you are
> > risking a compiler fetching the sucker again and getting NULL this
> > time around.
>
> fix breakage in gfs2_permission()
>
> "gfs2: Use GL_NOBLOCK flag for non-blocking lookups" has
> reintroduced the bug fixed in "gfs2: fix an oops in
> gfs2_permission" a while ago.  We still fetch ->i_gl carefully,
> and check if it's NULL, but then we proceed to fetch it again.
> If it became NULL in the meanwhile, we get trouble...
>
> Trivial to fix, fortunately.

Indeed, and thanks for the review.

The other problem isn't quite as easy to address, unfortunately. I've
sent Linus a revert request.

Andreas

Re: [PATCH 0/9] gfs2: Bugs in "Use GL_NOBLOCK flag for non-blocking lookups"

[Andreas Gruenbacher]

On Fri, Feb 2, 2024 at 5:59 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Fri, Feb 02, 2024 at 04:23:13AM +0000, Al Viro wrote:
>
> >         if (gfs2_glock_is_locked_by_me(ip->i_gl) == NULL) {
> >                 int noblock = may_not_block ? GL_NOBLOCK : 0;
> >                 error = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED,
> >                                            LM_FLAG_ANY | noblock, &i_gh);
> >                 if (error)
> >                         return error;
> >         }
> >
> > See the problem?  In RCU mode we carefully fetch ->i_gl and verify
> > it's not NULL.  Then we proceed to dereference it again.
> >
> > IOW, these ip->i_gl below ought to be replaced with gl, or you are
> > risking a compiler fetching the sucker again and getting NULL this
> > time around.
>
> Wait, what's going on with this part?
>         if (gfs2_holder_initialized(&i_gh))
>                 gfs2_glock_dq_uninit(&i_gh);
>
> First of all, gfs2_glock_dq_uninit() can bloody well sleep in
> gfs2_glock_dq().

Right, we need to get rid of the sleeping there. It's a really bad
design and I'm trying to figure out how to avoid it.

The sleeping in gfs2_glock_dq_uninit() -> gfs2_holder_uninit() ->
gfs2_glock_put() can be avoided by deferring the final put to
workqueue context; that looks pretty simple to achieve in comparison.

> What's more, it drops a reference to ->gh_gl,
> i.e. our gl, in gfs2_holder_uninit().  Where is the matching
> increment of gl's refcount?

That happens in gfs2_glock_nq_init() -> __gfs2_holder_init() ->
gfs2_glock_hold().

Thanks again,
Andreas