[Cluster-devel] [PATCH 0/4] gfs2: Various cleanups

[Cluster-devel] [PATCH 0/4] gfs2: Various cleanups

[Andreas Gruenbacher]

Here are a few minor clanups; please review.

Thanks,
Andreas

Andreas Gruenbacher (4):
  gfs2: Add new gfs2_iomap_get helper
  gfs2: Turn gfs2_extent_map into gfs2_{get,alloc}_extent
  gfs2: Replace gfs2_lblk_to_dblk with gfs2_get_extent
  gfs2: Turn gfs2_meta_indirect_buffer into gfs2_meta_buffer

 fs/gfs2/bmap.c     | 148 ++++++++++++++++++++++-----------------------
 fs/gfs2/bmap.h     |  13 ++--
 fs/gfs2/dir.c      |  13 ++--
 fs/gfs2/file.c     |   5 +-
 fs/gfs2/log.c      |   6 +-
 fs/gfs2/meta_io.c  |   9 ++-
 fs/gfs2/meta_io.h  |   6 +-
 fs/gfs2/quota.c    |   4 +-
 fs/gfs2/recovery.c |   4 +-
 9 files changed, 106 insertions(+), 102 deletions(-)

-- 
2.26.2

[Cluster-devel] [PATCH 1/4] gfs2: Add new gfs2_iomap_get helper

[Andreas Gruenbacher]

Rename the current gfs2_iomap_get and gfs2_iomap_alloc functions to __*.
Add a new gfs2_iomap_get helper that doesn't expose struct metapath.
Rename gfs2_iomap_get_alloc to gfs2_iomap_alloc.  Use the new helpers
where they make sense.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/bmap.c | 65 +++++++++++++++++++++++++++-----------------------
 fs/gfs2/bmap.h |  6 +++--
 fs/gfs2/file.c |  5 ++--
 3 files changed, 41 insertions(+), 35 deletions(-)

diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 7a358ae05185..cc12dc0d6955 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -632,7 +632,7 @@ enum alloc_state {
 };
 
 /**
- * gfs2_iomap_alloc - Build a metadata tree of the requested height
+ * __gfs2_iomap_alloc - Build a metadata tree of the requested height
  * @inode: The GFS2 inode
  * @iomap: The iomap structure
  * @mp: The metapath, with proper height information calculated
@@ -642,7 +642,7 @@ enum alloc_state {
  *  ii) Indirect blocks to fill in lower part of the metadata tree
  * iii) Data blocks
  *
- * This function is called after gfs2_iomap_get, which works out the
+ * This function is called after __gfs2_iomap_get, which works out the
  * total number of blocks which we need via gfs2_alloc_size.
  *
  * We then do the actual allocation asking for an extent at a time (if
@@ -660,8 +660,8 @@ enum alloc_state {
  * Returns: errno on error
  */
 
-static int gfs2_iomap_alloc(struct inode *inode, struct iomap *iomap,
-			    struct metapath *mp)
+static int __gfs2_iomap_alloc(struct inode *inode, struct iomap *iomap,
+			      struct metapath *mp)
 {
 	struct gfs2_inode *ip = GFS2_I(inode);
 	struct gfs2_sbd *sdp = GFS2_SB(inode);
@@ -802,10 +802,10 @@ static u64 gfs2_alloc_size(struct inode *inode, struct metapath *mp, u64 size)
 
 	/*
 	 * For writes to stuffed files, this function is called twice via
-	 * gfs2_iomap_get, before and after unstuffing. The size we return the
+	 * __gfs2_iomap_get, before and after unstuffing. The size we return the
 	 * first time needs to be large enough to get the reservation and
 	 * allocation sizes right.  The size we return the second time must
-	 * be exact or else gfs2_iomap_alloc won't do the right thing.
+	 * be exact or else __gfs2_iomap_alloc won't do the right thing.
 	 */
 
 	if (gfs2_is_stuffed(ip) || mp->mp_fheight != mp->mp_aheight) {
@@ -829,7 +829,7 @@ static u64 gfs2_alloc_size(struct inode *inode, struct metapath *mp, u64 size)
 }
 
 /**
- * gfs2_iomap_get - Map blocks from an inode to disk blocks
+ * __gfs2_iomap_get - Map blocks from an inode to disk blocks
  * @inode: The inode
  * @pos: Starting position in bytes
  * @length: Length to map, in bytes
@@ -839,9 +839,9 @@ static u64 gfs2_alloc_size(struct inode *inode, struct metapath *mp, u64 size)
  *
  * Returns: errno
  */
-static int gfs2_iomap_get(struct inode *inode, loff_t pos, loff_t length,
-			  unsigned flags, struct iomap *iomap,
-			  struct metapath *mp)
+static int __gfs2_iomap_get(struct inode *inode, loff_t pos, loff_t length,
+			    unsigned flags, struct iomap *iomap,
+			    struct metapath *mp)
 {
 	struct gfs2_inode *ip = GFS2_I(inode);
 	struct gfs2_sbd *sdp = GFS2_SB(inode);
@@ -975,12 +975,10 @@ static int gfs2_iomap_get(struct inode *inode, loff_t pos, loff_t length,
 int gfs2_lblk_to_dblk(struct inode *inode, u32 lblock, u64 *dblock)
 {
 	struct iomap iomap = { };
-	struct metapath mp = { .mp_aheight = 1, };
 	loff_t pos = (loff_t)lblock << inode->i_blkbits;
 	int ret;
 
-	ret = gfs2_iomap_get(inode, pos, i_blocksize(inode), 0, &iomap, &mp);
-	release_metapath(&mp);
+	ret = gfs2_iomap_get(inode, pos, i_blocksize(inode), &iomap);
 	if (ret == 0)
 		*dblock = iomap.addr >> inode->i_blkbits;
 
@@ -1109,14 +1107,14 @@ static int gfs2_iomap_begin_write(struct inode *inode, loff_t pos,
 			if (ret)
 				goto out_trans_end;
 			release_metapath(mp);
-			ret = gfs2_iomap_get(inode, iomap->offset,
-					     iomap->length, flags, iomap, mp);
+			ret = __gfs2_iomap_get(inode, iomap->offset,
+					       iomap->length, flags, iomap, mp);
 			if (ret)
 				goto out_trans_end;
 		}
 
 		if (iomap->type == IOMAP_HOLE) {
-			ret = gfs2_iomap_alloc(inode, iomap, mp);
+			ret = __gfs2_iomap_alloc(inode, iomap, mp);
 			if (ret) {
 				gfs2_trans_end(sdp);
 				gfs2_inplace_release(ip);
@@ -1168,7 +1166,7 @@ static int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
 			goto out;
 	}
 
-	ret = gfs2_iomap_get(inode, pos, length, flags, iomap, &mp);
+	ret = __gfs2_iomap_get(inode, pos, length, flags, iomap, &mp);
 	if (ret)
 		goto out_unlock;
 
@@ -1290,9 +1288,7 @@ int gfs2_block_map(struct inode *inode, sector_t lblock,
 	struct gfs2_inode *ip = GFS2_I(inode);
 	loff_t pos = (loff_t)lblock << inode->i_blkbits;
 	loff_t length = bh_map->b_size;
-	struct metapath mp = { .mp_aheight = 1, };
 	struct iomap iomap = { };
-	int flags = create ? IOMAP_WRITE : 0;
 	int ret;
 
 	clear_buffer_mapped(bh_map);
@@ -1300,10 +1296,10 @@ int gfs2_block_map(struct inode *inode, sector_t lblock,
 	clear_buffer_boundary(bh_map);
 	trace_gfs2_bmap(ip, bh_map, lblock, create, 1);
 
-	ret = gfs2_iomap_get(inode, pos, length, flags, &iomap, &mp);
-	if (create && !ret && iomap.type == IOMAP_HOLE)
-		ret = gfs2_iomap_alloc(inode, &iomap, &mp);
-	release_metapath(&mp);
+	if (!create)
+		ret = gfs2_iomap_get(inode, pos, length, &iomap);
+	else
+		ret = gfs2_iomap_alloc(inode, pos, length, &iomap);
 	if (ret)
 		goto out;
 
@@ -1461,15 +1457,26 @@ static int trunc_start(struct inode *inode, u64 newsize)
 	return error;
 }
 
-int gfs2_iomap_get_alloc(struct inode *inode, loff_t pos, loff_t length,
-			 struct iomap *iomap)
+int gfs2_iomap_get(struct inode *inode, loff_t pos, loff_t length,
+		   struct iomap *iomap)
+{
+	struct metapath mp = { .mp_aheight = 1, };
+	int ret;
+
+	ret = __gfs2_iomap_get(inode, pos, length, 0, iomap, &mp);
+	release_metapath(&mp);
+	return ret;
+}
+
+int gfs2_iomap_alloc(struct inode *inode, loff_t pos, loff_t length,
+		     struct iomap *iomap)
 {
 	struct metapath mp = { .mp_aheight = 1, };
 	int ret;
 
-	ret = gfs2_iomap_get(inode, pos, length, IOMAP_WRITE, iomap, &mp);
+	ret = __gfs2_iomap_get(inode, pos, length, IOMAP_WRITE, iomap, &mp);
 	if (!ret && iomap->type == IOMAP_HOLE)
-		ret = gfs2_iomap_alloc(inode, iomap, &mp);
+		ret = __gfs2_iomap_alloc(inode, iomap, &mp);
 	release_metapath(&mp);
 	return ret;
 }
@@ -2519,7 +2526,6 @@ int __gfs2_punch_hole(struct file *file, loff_t offset, loff_t length)
 static int gfs2_map_blocks(struct iomap_writepage_ctx *wpc, struct inode *inode,
 		loff_t offset)
 {
-	struct metapath mp = { .mp_aheight = 1, };
 	int ret;
 
 	if (WARN_ON_ONCE(gfs2_is_stuffed(GFS2_I(inode))))
@@ -2530,8 +2536,7 @@ static int gfs2_map_blocks(struct iomap_writepage_ctx *wpc, struct inode *inode,
 		return 0;
 
 	memset(&wpc->iomap, 0, sizeof(wpc->iomap));
-	ret = gfs2_iomap_get(inode, offset, INT_MAX, 0, &wpc->iomap, &mp);
-	release_metapath(&mp);
+	ret = gfs2_iomap_get(inode, offset, INT_MAX, &wpc->iomap);
 	return ret;
 }
 
diff --git a/fs/gfs2/bmap.h b/fs/gfs2/bmap.h
index aed4632d47d3..c63efee8aaa4 100644
--- a/fs/gfs2/bmap.h
+++ b/fs/gfs2/bmap.h
@@ -49,8 +49,10 @@ extern const struct iomap_writeback_ops gfs2_writeback_ops;
 extern int gfs2_unstuff_dinode(struct gfs2_inode *ip, struct page *page);
 extern int gfs2_block_map(struct inode *inode, sector_t lblock,
 			  struct buffer_head *bh, int create);
-extern int gfs2_iomap_get_alloc(struct inode *inode, loff_t pos, loff_t length,
-				struct iomap *iomap);
+extern int gfs2_iomap_get(struct inode *inode, loff_t pos, loff_t length,
+			  struct iomap *iomap);
+extern int gfs2_iomap_alloc(struct inode *inode, loff_t pos, loff_t length,
+			    struct iomap *iomap);
 extern int gfs2_extent_map(struct inode *inode, u64 lblock, int *new,
 			   u64 *dblock, unsigned *extlen);
 extern int gfs2_setattr_size(struct inode *inode, u64 size);
diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index 2d500f90cdac..5d34b974f4fd 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -421,7 +421,7 @@ static int gfs2_allocate_page_backing(struct page *page, unsigned int length)
 	do {
 		struct iomap iomap = { };
 
-		if (gfs2_iomap_get_alloc(page->mapping->host, pos, length, &iomap))
+		if (gfs2_iomap_alloc(page->mapping->host, pos, length, &iomap))
 			return -EIO;
 
 		if (length < iomap.length)
@@ -991,8 +991,7 @@ static int fallocate_chunk(struct inode *inode, loff_t offset, loff_t len,
 	while (offset < end) {
 		struct iomap iomap = { };
 
-		error = gfs2_iomap_get_alloc(inode, offset, end - offset,
-					     &iomap);
+		error = gfs2_iomap_alloc(inode, offset, end - offset, &iomap);
 		if (error)
 			goto out;
 		offset = iomap.offset + iomap.length;
-- 
2.26.2

[Cluster-devel] [PATCH 1/4] gfs2: Add new gfs2_iomap_get helper

[Bob Peterson]

----- Original Message -----
> Rename the current gfs2_iomap_get and gfs2_iomap_alloc functions to __*.
> Add a new gfs2_iomap_get helper that doesn't expose struct metapath.
> Rename gfs2_iomap_get_alloc to gfs2_iomap_alloc.  Use the new helpers
> where they make sense.
> 
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: Bob Peterson <rpeterso@redhat.com>

[Cluster-devel] [PATCH 2/4] gfs2: Turn gfs2_extent_map into gfs2_{get, alloc}_extent

[Andreas Gruenbacher]

Convert gfs2_extent_map to iomap and split it into gfs2_get_extent and
gfs2_alloc_extent.  Instead of hardcoding the extent size, pass it in
via the extlen parameter.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/bmap.c     | 59 ++++++++++++++++++++++++++++++----------------
 fs/gfs2/bmap.h     |  6 +++--
 fs/gfs2/dir.c      | 13 +++++-----
 fs/gfs2/quota.c    |  4 ++--
 fs/gfs2/recovery.c |  4 ++--
 5 files changed, 53 insertions(+), 33 deletions(-)

diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index cc12dc0d6955..ac959a99ea81 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -1320,28 +1320,47 @@ int gfs2_block_map(struct inode *inode, sector_t lblock,
 	return ret;
 }
 
-/*
- * Deprecated: do not use in new code
- */
-int gfs2_extent_map(struct inode *inode, u64 lblock, int *new, u64 *dblock, unsigned *extlen)
+int gfs2_get_extent(struct inode *inode, u64 lblock, u64 *dblock,
+		    unsigned int *extlen)
 {
-	struct buffer_head bh = { .b_state = 0, .b_blocknr = 0 };
+	unsigned int blkbits = inode->i_blkbits;
+	struct iomap iomap = { };
+	unsigned int len;
 	int ret;
-	int create = *new;
-
-	BUG_ON(!extlen);
-	BUG_ON(!dblock);
-	BUG_ON(!new);
-
-	bh.b_size = BIT(inode->i_blkbits + (create ? 0 : 5));
-	ret = gfs2_block_map(inode, lblock, &bh, create);
-	*extlen = bh.b_size >> inode->i_blkbits;
-	*dblock = bh.b_blocknr;
-	if (buffer_new(&bh))
-		*new = 1;
-	else
-		*new = 0;
-	return ret;
+
+	ret = gfs2_iomap_get(inode, lblock << blkbits, *extlen << blkbits,
+			     &iomap);
+	if (ret)
+		return ret;
+	if (iomap.type != IOMAP_MAPPED)
+		return -EIO;
+	*dblock = iomap.addr >> blkbits;
+	len = iomap.length >> blkbits;
+	if (len < *extlen)
+		*extlen = len;
+	return 0;
+}
+
+int gfs2_alloc_extent(struct inode *inode, u64 lblock, u64 *dblock,
+		      unsigned int *extlen, bool *new)
+{
+	unsigned int blkbits = inode->i_blkbits;
+	struct iomap iomap = { };
+	unsigned int len;
+	int ret;
+
+	ret = gfs2_iomap_alloc(inode, lblock << blkbits, *extlen << blkbits,
+			       &iomap);
+	if (ret)
+		return ret;
+	if (iomap.type != IOMAP_MAPPED)
+		return -EIO;
+	*dblock = iomap.addr >> blkbits;
+	len = iomap.length >> blkbits;
+	if (len < *extlen)
+		*extlen = len;
+	*new = iomap.flags & IOMAP_F_NEW;
+	return 0;
 }
 
 /*
diff --git a/fs/gfs2/bmap.h b/fs/gfs2/bmap.h
index c63efee8aaa4..67ef7cf7fdac 100644
--- a/fs/gfs2/bmap.h
+++ b/fs/gfs2/bmap.h
@@ -53,8 +53,10 @@ extern int gfs2_iomap_get(struct inode *inode, loff_t pos, loff_t length,
 			  struct iomap *iomap);
 extern int gfs2_iomap_alloc(struct inode *inode, loff_t pos, loff_t length,
 			    struct iomap *iomap);
-extern int gfs2_extent_map(struct inode *inode, u64 lblock, int *new,
-			   u64 *dblock, unsigned *extlen);
+extern int gfs2_get_extent(struct inode *inode, u64 lblock, u64 *dblock,
+			   unsigned int *extlen);
+extern int gfs2_alloc_extent(struct inode *inode, u64 lblock, u64 *dblock,
+			     unsigned *extlen, bool *new);
 extern int gfs2_setattr_size(struct inode *inode, u64 size);
 extern void gfs2_trim_blocks(struct inode *inode);
 extern int gfs2_truncatei_resume(struct gfs2_inode *ip);
diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
index c0f2875c946c..4517ffb7c13d 100644
--- a/fs/gfs2/dir.c
+++ b/fs/gfs2/dir.c
@@ -159,7 +159,7 @@ static int gfs2_dir_write_data(struct gfs2_inode *ip, const char *buf,
 	unsigned int o;
 	int copied = 0;
 	int error = 0;
-	int new = 0;
+	bool new = false;
 
 	if (!size)
 		return 0;
@@ -189,9 +189,9 @@ static int gfs2_dir_write_data(struct gfs2_inode *ip, const char *buf,
 			amount = sdp->sd_sb.sb_bsize - o;
 
 		if (!extlen) {
-			new = 1;
-			error = gfs2_extent_map(&ip->i_inode, lblock, &new,
-						&dblock, &extlen);
+			extlen = 1;
+			error = gfs2_alloc_extent(&ip->i_inode, lblock, &dblock,
+						  &extlen, &new);
 			if (error)
 				goto fail;
 			error = -EIO;
@@ -286,15 +286,14 @@ static int gfs2_dir_read_data(struct gfs2_inode *ip, __be64 *buf,
 	while (copied < size) {
 		unsigned int amount;
 		struct buffer_head *bh;
-		int new;
 
 		amount = size - copied;
 		if (amount > sdp->sd_sb.sb_bsize - o)
 			amount = sdp->sd_sb.sb_bsize - o;
 
 		if (!extlen) {
-			new = 0;
-			error = gfs2_extent_map(&ip->i_inode, lblock, &new,
+			extlen = 32;
+			error = gfs2_get_extent(&ip->i_inode, lblock,
 						&dblock, &extlen);
 			if (error || !dblock)
 				goto fail;
diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index 6e173ae378c4..9b1aca7e1264 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -1375,8 +1375,8 @@ int gfs2_quota_init(struct gfs2_sbd *sdp)
 		unsigned int y;
 
 		if (!extlen) {
-			int new = 0;
-			error = gfs2_extent_map(&ip->i_inode, x, &new, &dblock, &extlen);
+			extlen = 32;
+			error = gfs2_get_extent(&ip->i_inode, x, &dblock, &extlen);
 			if (error)
 				goto fail;
 		}
diff --git a/fs/gfs2/recovery.c b/fs/gfs2/recovery.c
index 282173774005..4ab4cdbf774a 100644
--- a/fs/gfs2/recovery.c
+++ b/fs/gfs2/recovery.c
@@ -34,12 +34,12 @@ int gfs2_replay_read_block(struct gfs2_jdesc *jd, unsigned int blk,
 {
 	struct gfs2_inode *ip = GFS2_I(jd->jd_inode);
 	struct gfs2_glock *gl = ip->i_gl;
-	int new = 0;
 	u64 dblock;
 	u32 extlen;
 	int error;
 
-	error = gfs2_extent_map(&ip->i_inode, blk, &new, &dblock, &extlen);
+	extlen = 32;
+	error = gfs2_get_extent(&ip->i_inode, blk, &dblock, &extlen);
 	if (error)
 		return error;
 	if (!dblock) {
-- 
2.26.2

[Cluster-devel] [PATCH 2/4] gfs2: Turn gfs2_extent_map into gfs2_{get, alloc}_extent

[Andrew Price]

On 01/04/2021 10:18, Andreas Gruenbacher wrote:
> Convert gfs2_extent_map to iomap and split it into gfs2_get_extent and
> gfs2_alloc_extent.  Instead of hardcoding the extent size, pass it in
> via the extlen parameter.
> 
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
>   fs/gfs2/bmap.c     | 59 ++++++++++++++++++++++++++++++----------------
>   fs/gfs2/bmap.h     |  6 +++--
>   fs/gfs2/dir.c      | 13 +++++-----
>   fs/gfs2/quota.c    |  4 ++--
>   fs/gfs2/recovery.c |  4 ++--
>   5 files changed, 53 insertions(+), 33 deletions(-)
> 
> diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
> index cc12dc0d6955..ac959a99ea81 100644
> --- a/fs/gfs2/bmap.c
> +++ b/fs/gfs2/bmap.c
> @@ -1320,28 +1320,47 @@ int gfs2_block_map(struct inode *inode, sector_t lblock,
>   	return ret;
>   }
>   
> -/*
> - * Deprecated: do not use in new code
> - */
> -int gfs2_extent_map(struct inode *inode, u64 lblock, int *new, u64 *dblock, unsigned *extlen)
> +int gfs2_get_extent(struct inode *inode, u64 lblock, u64 *dblock,
> +		    unsigned int *extlen)
>   {
> -	struct buffer_head bh = { .b_state = 0, .b_blocknr = 0 };
> +	unsigned int blkbits = inode->i_blkbits;
> +	struct iomap iomap = { };
> +	unsigned int len;
>   	int ret;
> -	int create = *new;
> -
> -	BUG_ON(!extlen);
> -	BUG_ON(!dblock);
> -	BUG_ON(!new);
> -
> -	bh.b_size = BIT(inode->i_blkbits + (create ? 0 : 5));
> -	ret = gfs2_block_map(inode, lblock, &bh, create);
> -	*extlen = bh.b_size >> inode->i_blkbits;
> -	*dblock = bh.b_blocknr;
> -	if (buffer_new(&bh))
> -		*new = 1;
> -	else
> -		*new = 0;
> -	return ret;
> +
> +	ret = gfs2_iomap_get(inode, lblock << blkbits, *extlen << blkbits,
> +			     &iomap);
> +	if (ret)
> +		return ret;
> +	if (iomap.type != IOMAP_MAPPED)
> +		return -EIO;
> +	*dblock = iomap.addr >> blkbits;
> +	len = iomap.length >> blkbits;
> +	if (len < *extlen)
> +		*extlen = len;
> +	return 0;
> +}
> +
> +int gfs2_alloc_extent(struct inode *inode, u64 lblock, u64 *dblock,
> +		      unsigned int *extlen, bool *new)
> +{
> +	unsigned int blkbits = inode->i_blkbits;
> +	struct iomap iomap = { };
> +	unsigned int len;
> +	int ret;
> +
> +	ret = gfs2_iomap_alloc(inode, lblock << blkbits, *extlen << blkbits,
> +			       &iomap);
> +	if (ret)
> +		return ret;
> +	if (iomap.type != IOMAP_MAPPED)
> +		return -EIO;
> +	*dblock = iomap.addr >> blkbits;
> +	len = iomap.length >> blkbits;
> +	if (len < *extlen)
> +		*extlen = len;
> +	*new = iomap.flags & IOMAP_F_NEW;

As *new is bool, shouldn't this be !!(iomap.flags & IOMAP_F_NEW) or similar?

Otherwise, the set looks good to me.

Andy

> +	return 0;
>   }
>   
>   /*
> diff --git a/fs/gfs2/bmap.h b/fs/gfs2/bmap.h
> index c63efee8aaa4..67ef7cf7fdac 100644
> --- a/fs/gfs2/bmap.h
> +++ b/fs/gfs2/bmap.h
> @@ -53,8 +53,10 @@ extern int gfs2_iomap_get(struct inode *inode, loff_t pos, loff_t length,
>   			  struct iomap *iomap);
>   extern int gfs2_iomap_alloc(struct inode *inode, loff_t pos, loff_t length,
>   			    struct iomap *iomap);
> -extern int gfs2_extent_map(struct inode *inode, u64 lblock, int *new,
> -			   u64 *dblock, unsigned *extlen);
> +extern int gfs2_get_extent(struct inode *inode, u64 lblock, u64 *dblock,
> +			   unsigned int *extlen);
> +extern int gfs2_alloc_extent(struct inode *inode, u64 lblock, u64 *dblock,
> +			     unsigned *extlen, bool *new);
>   extern int gfs2_setattr_size(struct inode *inode, u64 size);
>   extern void gfs2_trim_blocks(struct inode *inode);
>   extern int gfs2_truncatei_resume(struct gfs2_inode *ip);
> diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
> index c0f2875c946c..4517ffb7c13d 100644
> --- a/fs/gfs2/dir.c
> +++ b/fs/gfs2/dir.c
> @@ -159,7 +159,7 @@ static int gfs2_dir_write_data(struct gfs2_inode *ip, const char *buf,
>   	unsigned int o;
>   	int copied = 0;
>   	int error = 0;
> -	int new = 0;
> +	bool new = false;
>   
>   	if (!size)
>   		return 0;
> @@ -189,9 +189,9 @@ static int gfs2_dir_write_data(struct gfs2_inode *ip, const char *buf,
>   			amount = sdp->sd_sb.sb_bsize - o;
>   
>   		if (!extlen) {
> -			new = 1;
> -			error = gfs2_extent_map(&ip->i_inode, lblock, &new,
> -						&dblock, &extlen);
> +			extlen = 1;
> +			error = gfs2_alloc_extent(&ip->i_inode, lblock, &dblock,
> +						  &extlen, &new);
>   			if (error)
>   				goto fail;
>   			error = -EIO;
> @@ -286,15 +286,14 @@ static int gfs2_dir_read_data(struct gfs2_inode *ip, __be64 *buf,
>   	while (copied < size) {
>   		unsigned int amount;
>   		struct buffer_head *bh;
> -		int new;
>   
>   		amount = size - copied;
>   		if (amount > sdp->sd_sb.sb_bsize - o)
>   			amount = sdp->sd_sb.sb_bsize - o;
>   
>   		if (!extlen) {
> -			new = 0;
> -			error = gfs2_extent_map(&ip->i_inode, lblock, &new,
> +			extlen = 32;
> +			error = gfs2_get_extent(&ip->i_inode, lblock,
>   						&dblock, &extlen);
>   			if (error || !dblock)
>   				goto fail;
> diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
> index 6e173ae378c4..9b1aca7e1264 100644
> --- a/fs/gfs2/quota.c
> +++ b/fs/gfs2/quota.c
> @@ -1375,8 +1375,8 @@ int gfs2_quota_init(struct gfs2_sbd *sdp)
>   		unsigned int y;
>   
>   		if (!extlen) {
> -			int new = 0;
> -			error = gfs2_extent_map(&ip->i_inode, x, &new, &dblock, &extlen);
> +			extlen = 32;
> +			error = gfs2_get_extent(&ip->i_inode, x, &dblock, &extlen);
>   			if (error)
>   				goto fail;
>   		}
> diff --git a/fs/gfs2/recovery.c b/fs/gfs2/recovery.c
> index 282173774005..4ab4cdbf774a 100644
> --- a/fs/gfs2/recovery.c
> +++ b/fs/gfs2/recovery.c
> @@ -34,12 +34,12 @@ int gfs2_replay_read_block(struct gfs2_jdesc *jd, unsigned int blk,
>   {
>   	struct gfs2_inode *ip = GFS2_I(jd->jd_inode);
>   	struct gfs2_glock *gl = ip->i_gl;
> -	int new = 0;
>   	u64 dblock;
>   	u32 extlen;
>   	int error;
>   
> -	error = gfs2_extent_map(&ip->i_inode, blk, &new, &dblock, &extlen);
> +	extlen = 32;
> +	error = gfs2_get_extent(&ip->i_inode, blk, &dblock, &extlen);
>   	if (error)
>   		return error;
>   	if (!dblock) {
> 

[Cluster-devel] [PATCH 2/4] gfs2: Turn gfs2_extent_map into gfs2_{get, alloc}_extent

[Andreas Gruenbacher]

On Thu, Apr 1, 2021 at 11:46 AM Andrew Price <anprice@redhat.com> wrote:
> On 01/04/2021 10:18, Andreas Gruenbacher wrote:
> > Convert gfs2_extent_map to iomap and split it into gfs2_get_extent and
> > gfs2_alloc_extent.  Instead of hardcoding the extent size, pass it in
> > via the extlen parameter.
> >
> > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> > ---
> >   fs/gfs2/bmap.c     | 59 ++++++++++++++++++++++++++++++----------------
> >   fs/gfs2/bmap.h     |  6 +++--
> >   fs/gfs2/dir.c      | 13 +++++-----
> >   fs/gfs2/quota.c    |  4 ++--
> >   fs/gfs2/recovery.c |  4 ++--
> >   5 files changed, 53 insertions(+), 33 deletions(-)
> >
> > diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
> > index cc12dc0d6955..ac959a99ea81 100644
> > --- a/fs/gfs2/bmap.c
> > +++ b/fs/gfs2/bmap.c
> > @@ -1320,28 +1320,47 @@ int gfs2_block_map(struct inode *inode, sector_t lblock,
> >       return ret;
> >   }
> >
> > -/*
> > - * Deprecated: do not use in new code
> > - */
> > -int gfs2_extent_map(struct inode *inode, u64 lblock, int *new, u64 *dblock, unsigned *extlen)
> > +int gfs2_get_extent(struct inode *inode, u64 lblock, u64 *dblock,
> > +                 unsigned int *extlen)
> >   {
> > -     struct buffer_head bh = { .b_state = 0, .b_blocknr = 0 };
> > +     unsigned int blkbits = inode->i_blkbits;
> > +     struct iomap iomap = { };
> > +     unsigned int len;
> >       int ret;
> > -     int create = *new;
> > -
> > -     BUG_ON(!extlen);
> > -     BUG_ON(!dblock);
> > -     BUG_ON(!new);
> > -
> > -     bh.b_size = BIT(inode->i_blkbits + (create ? 0 : 5));
> > -     ret = gfs2_block_map(inode, lblock, &bh, create);
> > -     *extlen = bh.b_size >> inode->i_blkbits;
> > -     *dblock = bh.b_blocknr;
> > -     if (buffer_new(&bh))
> > -             *new = 1;
> > -     else
> > -             *new = 0;
> > -     return ret;
> > +
> > +     ret = gfs2_iomap_get(inode, lblock << blkbits, *extlen << blkbits,
> > +                          &iomap);
> > +     if (ret)
> > +             return ret;
> > +     if (iomap.type != IOMAP_MAPPED)
> > +             return -EIO;
> > +     *dblock = iomap.addr >> blkbits;
> > +     len = iomap.length >> blkbits;
> > +     if (len < *extlen)
> > +             *extlen = len;
> > +     return 0;
> > +}
> > +
> > +int gfs2_alloc_extent(struct inode *inode, u64 lblock, u64 *dblock,
> > +                   unsigned int *extlen, bool *new)
> > +{
> > +     unsigned int blkbits = inode->i_blkbits;
> > +     struct iomap iomap = { };
> > +     unsigned int len;
> > +     int ret;
> > +
> > +     ret = gfs2_iomap_alloc(inode, lblock << blkbits, *extlen << blkbits,
> > +                            &iomap);
> > +     if (ret)
> > +             return ret;
> > +     if (iomap.type != IOMAP_MAPPED)
> > +             return -EIO;
> > +     *dblock = iomap.addr >> blkbits;
> > +     len = iomap.length >> blkbits;
> > +     if (len < *extlen)
> > +             *extlen = len;
> > +     *new = iomap.flags & IOMAP_F_NEW;
>
> As *new is bool, shouldn't this be !!(iomap.flags & IOMAP_F_NEW) or similar?

That's implied with type bool (but not with integer types). For example,

  (bool)(1L << (8 * sizeof(long) - 1))

is always 1.

> Otherwise, the set looks good to me.
>
> Andy
>
> > +     return 0;
> >   }
> >
> >   /*
> > diff --git a/fs/gfs2/bmap.h b/fs/gfs2/bmap.h
> > index c63efee8aaa4..67ef7cf7fdac 100644
> > --- a/fs/gfs2/bmap.h
> > +++ b/fs/gfs2/bmap.h
> > @@ -53,8 +53,10 @@ extern int gfs2_iomap_get(struct inode *inode, loff_t pos, loff_t length,
> >                         struct iomap *iomap);
> >   extern int gfs2_iomap_alloc(struct inode *inode, loff_t pos, loff_t length,
> >                           struct iomap *iomap);
> > -extern int gfs2_extent_map(struct inode *inode, u64 lblock, int *new,
> > -                        u64 *dblock, unsigned *extlen);
> > +extern int gfs2_get_extent(struct inode *inode, u64 lblock, u64 *dblock,
> > +                        unsigned int *extlen);
> > +extern int gfs2_alloc_extent(struct inode *inode, u64 lblock, u64 *dblock,
> > +                          unsigned *extlen, bool *new);
> >   extern int gfs2_setattr_size(struct inode *inode, u64 size);
> >   extern void gfs2_trim_blocks(struct inode *inode);
> >   extern int gfs2_truncatei_resume(struct gfs2_inode *ip);
> > diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
> > index c0f2875c946c..4517ffb7c13d 100644
> > --- a/fs/gfs2/dir.c
> > +++ b/fs/gfs2/dir.c
> > @@ -159,7 +159,7 @@ static int gfs2_dir_write_data(struct gfs2_inode *ip, const char *buf,
> >       unsigned int o;
> >       int copied = 0;
> >       int error = 0;
> > -     int new = 0;
> > +     bool new = false;
> >
> >       if (!size)
> >               return 0;
> > @@ -189,9 +189,9 @@ static int gfs2_dir_write_data(struct gfs2_inode *ip, const char *buf,
> >                       amount = sdp->sd_sb.sb_bsize - o;
> >
> >               if (!extlen) {
> > -                     new = 1;
> > -                     error = gfs2_extent_map(&ip->i_inode, lblock, &new,
> > -                                             &dblock, &extlen);
> > +                     extlen = 1;
> > +                     error = gfs2_alloc_extent(&ip->i_inode, lblock, &dblock,
> > +                                               &extlen, &new);
> >                       if (error)
> >                               goto fail;
> >                       error = -EIO;
> > @@ -286,15 +286,14 @@ static int gfs2_dir_read_data(struct gfs2_inode *ip, __be64 *buf,
> >       while (copied < size) {
> >               unsigned int amount;
> >               struct buffer_head *bh;
> > -             int new;
> >
> >               amount = size - copied;
> >               if (amount > sdp->sd_sb.sb_bsize - o)
> >                       amount = sdp->sd_sb.sb_bsize - o;
> >
> >               if (!extlen) {
> > -                     new = 0;
> > -                     error = gfs2_extent_map(&ip->i_inode, lblock, &new,
> > +                     extlen = 32;
> > +                     error = gfs2_get_extent(&ip->i_inode, lblock,
> >                                               &dblock, &extlen);
> >                       if (error || !dblock)
> >                               goto fail;
> > diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
> > index 6e173ae378c4..9b1aca7e1264 100644
> > --- a/fs/gfs2/quota.c
> > +++ b/fs/gfs2/quota.c
> > @@ -1375,8 +1375,8 @@ int gfs2_quota_init(struct gfs2_sbd *sdp)
> >               unsigned int y;
> >
> >               if (!extlen) {
> > -                     int new = 0;
> > -                     error = gfs2_extent_map(&ip->i_inode, x, &new, &dblock, &extlen);
> > +                     extlen = 32;
> > +                     error = gfs2_get_extent(&ip->i_inode, x, &dblock, &extlen);
> >                       if (error)
> >                               goto fail;
> >               }
> > diff --git a/fs/gfs2/recovery.c b/fs/gfs2/recovery.c
> > index 282173774005..4ab4cdbf774a 100644
> > --- a/fs/gfs2/recovery.c
> > +++ b/fs/gfs2/recovery.c
> > @@ -34,12 +34,12 @@ int gfs2_replay_read_block(struct gfs2_jdesc *jd, unsigned int blk,
> >   {
> >       struct gfs2_inode *ip = GFS2_I(jd->jd_inode);
> >       struct gfs2_glock *gl = ip->i_gl;
> > -     int new = 0;
> >       u64 dblock;
> >       u32 extlen;
> >       int error;
> >
> > -     error = gfs2_extent_map(&ip->i_inode, blk, &new, &dblock, &extlen);
> > +     extlen = 32;
> > +     error = gfs2_get_extent(&ip->i_inode, blk, &dblock, &extlen);
> >       if (error)
> >               return error;
> >       if (!dblock) {
> >
>

Thanks,
Andreas

[Cluster-devel] [PATCH 3/4] gfs2: Replace gfs2_lblk_to_dblk with gfs2_get_extent

[Andreas Gruenbacher]

We don't need two very similar functions for mapping logical blocks to physical
blocks.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/bmap.c | 24 ------------------------
 fs/gfs2/bmap.h |  1 -
 fs/gfs2/log.c  |  6 +++++-
 3 files changed, 5 insertions(+), 26 deletions(-)

diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index ac959a99ea81..bad5dc641bbd 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -961,30 +961,6 @@ static int __gfs2_iomap_get(struct inode *inode, loff_t pos, loff_t length,
 	goto out;
 }
 
-/**
- * gfs2_lblk_to_dblk - convert logical block to disk block
- * @inode: the inode of the file we're mapping
- * @lblock: the block relative to the start of the file
- * @dblock: the returned dblock, if no error
- *
- * This function maps a single block from a file logical block (relative to
- * the start of the file) to a file system absolute block using iomap.
- *
- * Returns: the absolute file system block, or an error
- */
-int gfs2_lblk_to_dblk(struct inode *inode, u32 lblock, u64 *dblock)
-{
-	struct iomap iomap = { };
-	loff_t pos = (loff_t)lblock << inode->i_blkbits;
-	int ret;
-
-	ret = gfs2_iomap_get(inode, pos, i_blocksize(inode), &iomap);
-	if (ret == 0)
-		*dblock = iomap.addr >> inode->i_blkbits;
-
-	return ret;
-}
-
 static int gfs2_write_lock(struct inode *inode)
 {
 	struct gfs2_inode *ip = GFS2_I(inode);
diff --git a/fs/gfs2/bmap.h b/fs/gfs2/bmap.h
index 67ef7cf7fdac..6676d863faef 100644
--- a/fs/gfs2/bmap.h
+++ b/fs/gfs2/bmap.h
@@ -66,6 +66,5 @@ extern int gfs2_write_alloc_required(struct gfs2_inode *ip, u64 offset,
 extern int gfs2_map_journal_extents(struct gfs2_sbd *sdp, struct gfs2_jdesc *jd);
 extern void gfs2_free_journal_extents(struct gfs2_jdesc *jd);
 extern int __gfs2_punch_hole(struct file *file, loff_t offset, loff_t length);
-extern int gfs2_lblk_to_dblk(struct inode *inode, u32 lblock, u64 *dblock);
 
 #endif /* __BMAP_DOT_H__ */
diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
index 6410281546f9..69ddd9518396 100644
--- a/fs/gfs2/log.c
+++ b/fs/gfs2/log.c
@@ -859,7 +859,11 @@ void gfs2_write_log_header(struct gfs2_sbd *sdp, struct gfs2_jdesc *jd,
 	if (!list_empty(&jd->extent_list))
 		dblock = gfs2_log_bmap(jd, lblock);
 	else {
-		int ret = gfs2_lblk_to_dblk(jd->jd_inode, lblock, &dblock);
+		unsigned int extlen;
+		int ret;
+
+		extlen = 1;
+		ret = gfs2_get_extent(jd->jd_inode, lblock, &dblock, &extlen);
 		if (gfs2_assert_withdraw(sdp, ret == 0))
 			return;
 	}
-- 
2.26.2

[Cluster-devel] [PATCH 4/4] gfs2: Turn gfs2_meta_indirect_buffer into gfs2_meta_buffer

[Andreas Gruenbacher]

Instead of only supporting GFS2_METATYPE_DI and GFS2_METATYPE_IN blocks,
make the block type a parameter of gfs2_meta_indirect_buffer and rename
the function to gfs2_meta_buffer.

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

diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index bad5dc641bbd..ae9ba03fbd48 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -331,7 +331,7 @@ static int __fillup_metapath(struct gfs2_inode *ip, struct metapath *mp,
 
 		if (!dblock)
 			break;
-		ret = gfs2_meta_indirect_buffer(ip, x + 1, dblock, &mp->mp_bh[x + 1]);
+		ret = gfs2_meta_buffer(ip, GFS2_METATYPE_IN, dblock, &mp->mp_bh[x + 1]);
 		if (ret)
 			return ret;
 	}
diff --git a/fs/gfs2/meta_io.c b/fs/gfs2/meta_io.c
index 2db573e31f78..4946cbce1ab6 100644
--- a/fs/gfs2/meta_io.c
+++ b/fs/gfs2/meta_io.c
@@ -462,23 +462,22 @@ void gfs2_journal_wipe(struct gfs2_inode *ip, u64 bstart, u32 blen)
 }
 
 /**
- * gfs2_meta_indirect_buffer - Get a metadata buffer
+ * gfs2_meta_buffer - Get a metadata buffer
  * @ip: The GFS2 inode
- * @height: The level of this buf in the metadata (indir addr) tree (if any)
+ * @mtype: The block type (GFS2_METATYPE_*)
  * @num: The block number (device relative) of the buffer
  * @bhp: the buffer is returned here
  *
  * Returns: errno
  */
 
-int gfs2_meta_indirect_buffer(struct gfs2_inode *ip, int height, u64 num,
-			      struct buffer_head **bhp)
+int gfs2_meta_buffer(struct gfs2_inode *ip, u32 mtype, u64 num,
+		     struct buffer_head **bhp)
 {
 	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
 	struct gfs2_glock *gl = ip->i_gl;
 	struct buffer_head *bh;
 	int ret = 0;
-	u32 mtype = height ? GFS2_METATYPE_IN : GFS2_METATYPE_DI;
 	int rahead = 0;
 
 	if (num == ip->i_no_addr)
diff --git a/fs/gfs2/meta_io.h b/fs/gfs2/meta_io.h
index 4a8c01929b79..21880d72081a 100644
--- a/fs/gfs2/meta_io.h
+++ b/fs/gfs2/meta_io.h
@@ -61,13 +61,13 @@ enum {
 
 extern void gfs2_remove_from_journal(struct buffer_head *bh, int meta);
 extern void gfs2_journal_wipe(struct gfs2_inode *ip, u64 bstart, u32 blen);
-extern int gfs2_meta_indirect_buffer(struct gfs2_inode *ip, int height, u64 num,
-				     struct buffer_head **bhp);
+extern int gfs2_meta_buffer(struct gfs2_inode *ip, u32 mtype, u64 num,
+			    struct buffer_head **bhp);
 
 static inline int gfs2_meta_inode_buffer(struct gfs2_inode *ip,
 					 struct buffer_head **bhp)
 {
-	return gfs2_meta_indirect_buffer(ip, 0, ip->i_no_addr, bhp);
+	return gfs2_meta_buffer(ip, GFS2_METATYPE_DI, ip->i_no_addr, bhp);
 }
 
 struct buffer_head *gfs2_meta_ra(struct gfs2_glock *gl, u64 dblock, u32 extlen);
-- 
2.26.2