[Cluster-devel] [PATCH 0/4] Implement iomap for fiemap in GFS2

[Cluster-devel] [PATCH 0/4] Implement iomap for fiemap in GFS2

[Bob Peterson]

On 12 August, 2016, I submitted patches to make GFS2 use a new iomap
interface for its fiemap implementation. I received lots of good
feedback, including bugs and other suggestions. Since that time, the
iomap implementation has also gone upstream and changed.
This new set of 4 patches is my latest attempt to revive this effort.

The four patches are as follows:
1. GFS2: Remove bh_map requirements from gfs2_bmap_alloc
   Steve Whitehouse suggested we implement GFS2's blockmap function so
   that it can use the new iomap interface under the covers. In order
   to do that, I had to decouple the bh_map from function gfs2_bmap_alloc.
   This patch does that, in favor of more generic mapping details.
   It was either that, or implement a multi-block allocation path with
   iomap, which I didn't want to do with this iteration. 
2. GFS2: Make height info part of metapath
   This patch is another "decoupling" needed to make the GFS2 blockmap
   function work with iomap under the covers. In this case, it eliminates
   the height parameters from function gfs2_bmap_alloc in favor of
   simply fetching the values from the metapath.
3. GFS2: Implement iomap for block_map
   With those things decoupled, this patch now implements the new GFS2
   get_iomap function, and makes gfs2_block_map use it under the covers.
   Eventually we might be able to eliminate it in favor of the iomap
   stuff only, but only after we've sorted out multi-block allocations
   with iomap. Andreas's bugs are now hopefully fixed and his other
   suggestions implemented as well.
4. GFS2: Switch fiemap implementation to use iomap
   This patch switches GFS2's fiemap implementation to use the new iomap
   interface. It also uses a "select" in Kconfig to pull it all in, as
   Christoph had suggested.

This has been tested with large holey files, up to 1EB, and I also tested
some boundary conditions as well, such as 1PB plus and minus 1 byte, and
1PB plus and minus 4K. I also ran Chris Mason's fiemap_test against it
with those same files. That's not to say it's perfect, but it seems to
have basic functionality and excellent performance.

Feedback is encouraged.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
Bob Peterson (4):
  GFS2: Remove bh_map requirements from gfs2_bmap_alloc
  GFS2: Make height info part of metapath
  GFS2: Implement iomap for block_map
  GFS2: Switch fiemap implementation to use iomap

 fs/gfs2/Kconfig       |   1 +
 fs/gfs2/bmap.c        | 333 +++++++++++++++++++++++++++++++++++---------------
 fs/gfs2/bmap.h        |   4 +
 fs/gfs2/inode.c       |  60 +++++++--
 include/linux/iomap.h |   1 +
 5 files changed, 288 insertions(+), 111 deletions(-)

-- 
2.7.4

[Cluster-devel] [PATCH 1/4] GFS2: Remove bh_map requirements from gfs2_bmap_alloc

[Bob Peterson]

Function gfs2_bmap_alloc is used to allocate data blocks during a
block_map operation. Before this patch, a buffer_head pointer was
passed in. This patch removes that from the function, replacing it
with other variables. This is a step toward allowing the iomap
interface to operate properly in gfs2.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/bmap.c | 56 ++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 34 insertions(+), 22 deletions(-)

diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index fc5da4c..f50933a 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -426,11 +426,13 @@ enum alloc_state {
  * gfs2_bmap_alloc - Build a metadata tree of the requested height
  * @inode: The GFS2 inode
  * @lblock: The logical starting block of the extent
- * @bh_map: This is used to return the mapping details
+ * @zero_new: True if newly allocated blocks should be zeroed
  * @mp: The metapath
  * @sheight: The starting height (i.e. whats already mapped)
  * @height: The height to build to
  * @maxlen: The max number of data blocks to alloc
+ * @dblock: Pointer to the physical block allocated
+ * @dblks: Pointer to an int number of data blocks in the span
  *
  * In this routine we may have to alloc:
  *   i) Indirect blocks to grow the metadata tree height
@@ -447,18 +449,18 @@ enum alloc_state {
  */
 
 static int gfs2_bmap_alloc(struct inode *inode, const sector_t lblock,
-			   struct buffer_head *bh_map, struct metapath *mp,
+			   bool zero_new, struct metapath *mp,
 			   const unsigned int sheight,
 			   const unsigned int height,
-			   const size_t maxlen)
+			   const size_t maxlen, sector_t *dblock,
+			   unsigned int *dblks)
 {
 	struct gfs2_inode *ip = GFS2_I(inode);
 	struct gfs2_sbd *sdp = GFS2_SB(inode);
 	struct super_block *sb = sdp->sd_vfs;
 	struct buffer_head *dibh = mp->mp_bh[0];
-	u64 bn, dblock = 0;
+	u64 bn;
 	unsigned n, i, blks, alloced = 0, iblks = 0, branch_start = 0;
-	unsigned dblks = 0;
 	unsigned ptrs_per_blk;
 	const unsigned end_of_metadata = height - 1;
 	int ret;
@@ -469,6 +471,8 @@ static int gfs2_bmap_alloc(struct inode *inode, const sector_t lblock,
 
 	BUG_ON(sheight < 1);
 	BUG_ON(dibh == NULL);
+	*dblock = 0;
+	*dblks = 0;
 
 	gfs2_trans_add_meta(ip->i_gl, dibh);
 
@@ -477,15 +481,15 @@ static int gfs2_bmap_alloc(struct inode *inode, const sector_t lblock,
 		/* Bottom indirect block exists, find unalloced extent size */
 		ptr = metapointer(end_of_metadata, mp);
 		bh = mp->mp_bh[end_of_metadata];
-		dblks = gfs2_extent_length(bh->b_data, bh->b_size, ptr, maxlen,
-					   &eob);
-		BUG_ON(dblks < 1);
+		*dblks = gfs2_extent_length(bh->b_data, bh->b_size, ptr,
+					    maxlen, &eob);
+		BUG_ON(*dblks < 1);
 		state = ALLOC_DATA;
 	} else {
 		/* Need to allocate indirect blocks */
 		ptrs_per_blk = height > 1 ? sdp->sd_inptrs : sdp->sd_diptrs;
-		dblks = min(maxlen, (size_t)(ptrs_per_blk -
-					     mp->mp_list[end_of_metadata]));
+		*dblks = min(maxlen, (size_t)(ptrs_per_blk -
+					      mp->mp_list[end_of_metadata]));
 		if (height == ip->i_height) {
 			/* Writing into existing tree, extend tree down */
 			iblks = height - sheight;
@@ -501,7 +505,7 @@ static int gfs2_bmap_alloc(struct inode *inode, const sector_t lblock,
 
 	/* start of the second part of the function (state machine) */
 
-	blks = dblks + iblks;
+	blks = *dblks + iblks;
 	i = sheight;
 	do {
 		int error;
@@ -557,33 +561,30 @@ static int gfs2_bmap_alloc(struct inode *inode, const sector_t lblock,
 				break;
 		/* Tree complete, adding data blocks */
 		case ALLOC_DATA:
-			BUG_ON(n > dblks);
+			BUG_ON(n > *dblks);
 			BUG_ON(mp->mp_bh[end_of_metadata] == NULL);
 			gfs2_trans_add_meta(ip->i_gl, mp->mp_bh[end_of_metadata]);
-			dblks = n;
+			*dblks = n;
 			ptr = metapointer(end_of_metadata, mp);
-			dblock = bn;
+			*dblock = bn;
 			while (n-- > 0)
 				*ptr++ = cpu_to_be64(bn++);
-			if (buffer_zeronew(bh_map)) {
-				ret = sb_issue_zeroout(sb, dblock, dblks,
+			if (zero_new) {
+				ret = sb_issue_zeroout(sb, *dblock, *dblks,
 						       GFP_NOFS);
 				if (ret) {
 					fs_err(sdp,
 					       "Failed to zero data buffers\n");
-					clear_buffer_zeronew(bh_map);
+					zero_new = false;
 				}
 			}
 			break;
 		}
-	} while ((state != ALLOC_DATA) || !dblock);
+	} while ((state != ALLOC_DATA) || !(*dblock));
 
 	ip->i_height = height;
 	gfs2_add_inode_blocks(&ip->i_inode, alloced);
 	gfs2_dinode_out(ip, mp->mp_bh[0]->b_data);
-	map_bh(bh_map, inode->i_sb, dblock);
-	bh_map->b_size = dblks << inode->i_blkbits;
-	set_buffer_new(bh_map);
 	return 0;
 }
 
@@ -617,6 +618,9 @@ int gfs2_block_map(struct inode *inode, sector_t lblock,
 	unsigned int len;
 	struct buffer_head *bh;
 	u8 height;
+	bool zero_new = false;
+	sector_t dblock;
+	unsigned int dblks = 0;
 
 	BUG_ON(maxlen == 0);
 
@@ -673,7 +677,15 @@ int gfs2_block_map(struct inode *inode, sector_t lblock,
 	}
 
 	/* At this point ret is the tree depth of already allocated blocks */
-	ret = gfs2_bmap_alloc(inode, lblock, bh_map, &mp, ret, height, maxlen);
+	if (buffer_zeronew(bh_map))
+		zero_new = true;
+	ret = gfs2_bmap_alloc(inode, lblock, zero_new, &mp, ret, height,
+			      maxlen, &dblock, &dblks);
+	if (ret == 0) {
+		map_bh(bh_map, inode->i_sb, dblock);
+		bh_map->b_size = dblks << inode->i_blkbits;
+		set_buffer_new(bh_map);
+	}
 	goto out;
 }
 
-- 
2.7.4

[Cluster-devel] [PATCH 2/4] GFS2: Make height info part of metapath

[Bob Peterson]

This patch eliminates height parameters from function gfs2_bmap_alloc.
Function find_metapath determines the metapath's "find height", also
known as the desired height. Function lookup_metapath determines the
metapath's "actual height", previously known as starting height or
sheight. Function gfs2_bmap_alloc now gets both height values from
the metapath. This simplification was done as a step toward switching
the block_map functions to using iomap.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/bmap.c | 65 +++++++++++++++++++++++++++++++---------------------------
 1 file changed, 35 insertions(+), 30 deletions(-)

diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index f50933a..774bdb8 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -36,6 +36,8 @@
 struct metapath {
 	struct buffer_head *mp_bh[GFS2_MAX_META_HEIGHT];
 	__u16 mp_list[GFS2_MAX_META_HEIGHT];
+	int mp_fheight; /* find_metapath height */
+	int mp_aheight; /* actual height (lookup height) */
 };
 
 struct strip_mine {
@@ -240,9 +242,9 @@ static void find_metapath(const struct gfs2_sbd *sdp, u64 block,
 {
 	unsigned int i;
 
+	mp->mp_fheight = height;
 	for (i = height; i--;)
 		mp->mp_list[i] = do_div(block, sdp->sd_inptrs);
-
 }
 
 static inline unsigned int metapath_branch_start(const struct metapath *mp)
@@ -323,15 +325,20 @@ static int lookup_metapath(struct gfs2_inode *ip, struct metapath *mp)
 	for (x = 0; x < end_of_metadata; x++) {
 		ptr = metapointer(x, mp);
 		dblock = be64_to_cpu(*ptr);
-		if (!dblock)
-			return x + 1;
+		if (!dblock) {
+			ret = x + 1;
+			goto out;
+		}
 
 		ret = gfs2_meta_indirect_buffer(ip, x+1, dblock, &mp->mp_bh[x+1]);
 		if (ret)
-			return ret;
+			goto out;
 	}
 
-	return ip->i_height;
+	ret = ip->i_height;
+out:
+	mp->mp_aheight = ret;
+	return ret;
 }
 
 static inline void release_metapath(struct metapath *mp)
@@ -427,9 +434,7 @@ enum alloc_state {
  * @inode: The GFS2 inode
  * @lblock: The logical starting block of the extent
  * @zero_new: True if newly allocated blocks should be zeroed
- * @mp: The metapath
- * @sheight: The starting height (i.e. whats already mapped)
- * @height: The height to build to
+ * @mp: The metapath, with proper height information calculated
  * @maxlen: The max number of data blocks to alloc
  * @dblock: Pointer to the physical block allocated
  * @dblks: Pointer to an int number of data blocks in the span
@@ -450,8 +455,6 @@ enum alloc_state {
 
 static int gfs2_bmap_alloc(struct inode *inode, const sector_t lblock,
 			   bool zero_new, struct metapath *mp,
-			   const unsigned int sheight,
-			   const unsigned int height,
 			   const size_t maxlen, sector_t *dblock,
 			   unsigned int *dblks)
 {
@@ -462,21 +465,21 @@ static int gfs2_bmap_alloc(struct inode *inode, const sector_t lblock,
 	u64 bn;
 	unsigned n, i, blks, alloced = 0, iblks = 0, branch_start = 0;
 	unsigned ptrs_per_blk;
-	const unsigned end_of_metadata = height - 1;
+	const unsigned end_of_metadata = mp->mp_fheight - 1;
 	int ret;
 	int eob = 0;
 	enum alloc_state state;
 	__be64 *ptr;
 	__be64 zero_bn = 0;
 
-	BUG_ON(sheight < 1);
+	BUG_ON(mp->mp_aheight < 1);
 	BUG_ON(dibh == NULL);
 	*dblock = 0;
 	*dblks = 0;
 
 	gfs2_trans_add_meta(ip->i_gl, dibh);
 
-	if (height == sheight) {
+	if (mp->mp_fheight == mp->mp_aheight) {
 		struct buffer_head *bh;
 		/* Bottom indirect block exists, find unalloced extent size */
 		ptr = metapointer(end_of_metadata, mp);
@@ -487,26 +490,27 @@ static int gfs2_bmap_alloc(struct inode *inode, const sector_t lblock,
 		state = ALLOC_DATA;
 	} else {
 		/* Need to allocate indirect blocks */
-		ptrs_per_blk = height > 1 ? sdp->sd_inptrs : sdp->sd_diptrs;
+		ptrs_per_blk = mp->mp_fheight > 1 ? sdp->sd_inptrs :
+			sdp->sd_diptrs;
 		*dblks = min(maxlen, (size_t)(ptrs_per_blk -
 					      mp->mp_list[end_of_metadata]));
-		if (height == ip->i_height) {
+		if (mp->mp_fheight == ip->i_height) {
 			/* Writing into existing tree, extend tree down */
-			iblks = height - sheight;
+			iblks = mp->mp_fheight - mp->mp_aheight;
 			state = ALLOC_GROW_DEPTH;
 		} else {
 			/* Building up tree height */
 			state = ALLOC_GROW_HEIGHT;
-			iblks = height - ip->i_height;
+			iblks = mp->mp_fheight - ip->i_height;
 			branch_start = metapath_branch_start(mp);
-			iblks += (height - branch_start);
+			iblks += (mp->mp_fheight - branch_start);
 		}
 	}
 
 	/* start of the second part of the function (state machine) */
 
 	blks = *dblks + iblks;
-	i = sheight;
+	i = mp->mp_aheight;
 	do {
 		int error;
 		n = blks - alloced;
@@ -524,9 +528,10 @@ static int gfs2_bmap_alloc(struct inode *inode, const sector_t lblock,
 						 sizeof(struct gfs2_dinode));
 				zero_bn = *ptr;
 			}
-			for (; i - 1 < height - ip->i_height && n > 0; i++, n--)
+			for (; i - 1 < mp->mp_fheight - ip->i_height && n > 0;
+			     i++, n--)
 				gfs2_indirect_init(mp, ip->i_gl, i, 0, bn++);
-			if (i - 1 == height - ip->i_height) {
+			if (i - 1 == mp->mp_fheight - ip->i_height) {
 				i--;
 				gfs2_buffer_copy_tail(mp->mp_bh[i],
 						sizeof(struct gfs2_meta_header),
@@ -538,7 +543,7 @@ static int gfs2_bmap_alloc(struct inode *inode, const sector_t lblock,
 					sizeof(struct gfs2_meta_header));
 				*ptr = zero_bn;
 				state = ALLOC_GROW_DEPTH;
-				for(i = branch_start; i < height; i++) {
+				for(i = branch_start; i < mp->mp_fheight; i++) {
 					if (mp->mp_bh[i] == NULL)
 						break;
 					brelse(mp->mp_bh[i]);
@@ -550,12 +555,12 @@ static int gfs2_bmap_alloc(struct inode *inode, const sector_t lblock,
 				break;
 		/* Branching from existing tree */
 		case ALLOC_GROW_DEPTH:
-			if (i > 1 && i < height)
+			if (i > 1 && i < mp->mp_fheight)
 				gfs2_trans_add_meta(ip->i_gl, mp->mp_bh[i-1]);
-			for (; i < height && n > 0; i++, n--)
+			for (; i < mp->mp_fheight && n > 0; i++, n--)
 				gfs2_indirect_init(mp, ip->i_gl, i,
 						   mp->mp_list[i-1], bn++);
-			if (i == height)
+			if (i == mp->mp_fheight)
 				state = ALLOC_DATA;
 			if (n == 0)
 				break;
@@ -582,7 +587,7 @@ static int gfs2_bmap_alloc(struct inode *inode, const sector_t lblock,
 		}
 	} while ((state != ALLOC_DATA) || !(*dblock));
 
-	ip->i_height = height;
+	ip->i_height = mp->mp_fheight;
 	gfs2_add_inode_blocks(&ip->i_inode, alloced);
 	gfs2_dinode_out(ip, mp->mp_bh[0]->b_data);
 	return 0;
@@ -644,13 +649,13 @@ int gfs2_block_map(struct inode *inode, sector_t lblock,
 	while (size > arr[height])
 		height++;
 	find_metapath(sdp, lblock, &mp, height);
-	ret = 1;
+	mp.mp_aheight = 1;
 	if (height > ip->i_height || gfs2_is_stuffed(ip))
 		goto do_alloc;
 	ret = lookup_metapath(ip, &mp);
 	if (ret < 0)
 		goto out;
-	if (ret != ip->i_height)
+	if (mp.mp_aheight != ip->i_height)
 		goto do_alloc;
 	ptr = metapointer(ip->i_height - 1, &mp);
 	if (*ptr == 0)
@@ -679,8 +684,8 @@ int gfs2_block_map(struct inode *inode, sector_t lblock,
 	/* At this point ret is the tree depth of already allocated blocks */
 	if (buffer_zeronew(bh_map))
 		zero_new = true;
-	ret = gfs2_bmap_alloc(inode, lblock, zero_new, &mp, ret, height,
-			      maxlen, &dblock, &dblks);
+	ret = gfs2_bmap_alloc(inode, lblock, zero_new, &mp, maxlen, &dblock,
+			      &dblks);
 	if (ret == 0) {
 		map_bh(bh_map, inode->i_sb, dblock);
 		bh_map->b_size = dblks << inode->i_blkbits;
-- 
2.7.4

[Cluster-devel] [PATCH 3/4] GFS2: Implement iomap for block_map

[Bob Peterson]

This patch implements iomap for block mapping, and switches the
block_map function to use it under the covers.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/bmap.c | 244 ++++++++++++++++++++++++++++++++++++++++++---------------
 fs/gfs2/bmap.h |   4 +
 2 files changed, 185 insertions(+), 63 deletions(-)

diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 774bdb8..b1bcdd6 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -13,6 +13,7 @@
 #include <linux/blkdev.h>
 #include <linux/gfs2_ondisk.h>
 #include <linux/crc32.h>
+#include <linux/iomap.h>
 
 #include "gfs2.h"
 #include "incore.h"
@@ -594,104 +595,221 @@ static int gfs2_bmap_alloc(struct inode *inode, const sector_t lblock,
 }
 
 /**
- * gfs2_block_map - Map a block from an inode to a disk block
- * @inode: The inode
- * @lblock: The logical block number
- * @bh_map: The bh to be mapped
- * @create: True if its ok to alloc blocks to satify the request
+ * hole_size - figure out the size of a hole
+ * @ip: The inode
+ * @lblock: The logical starting block number
+ * @mp: The metapath
  *
- * Sets buffer_mapped() if successful, sets buffer_boundary() if a
- * read of metadata will be required before the next block can be
- * mapped. Sets buffer_new() if new blocks were allocated.
+ * Returns: The hole size in bytes
  *
- * Returns: errno
  */
+static u64 hole_size(struct inode *inode, sector_t lblock, struct metapath *mp)
+{
+	struct gfs2_inode *ip = GFS2_I(inode);
+	struct gfs2_sbd *sdp = GFS2_SB(inode);
+	struct metapath mp_eof;
+	unsigned int end_of_metadata = ip->i_height - 1;
+	u64 factor = 1;
+	int hgt = end_of_metadata;
+	u64 holesz = 0, holestep;
+	const __be64 *first, *end, *ptr;
+	const struct buffer_head *bh;
+	u64 isize_blks = (i_size_read(inode) - 1) >> inode->i_blkbits;
+	int zeroptrs;
+	bool done = false;
+
+	/* Get another metapath, to the very last byte */
+	find_metapath(sdp, isize_blks, &mp_eof, ip->i_height);
+	for (hgt = end_of_metadata; hgt >= 0 && !done; hgt--) {
+		bh = mp->mp_bh[hgt];
+		if (bh) {
+			zeroptrs = 0;
+			first = metapointer(hgt, mp);
+			end = (const __be64 *)(bh->b_data + bh->b_size);
+
+			for (ptr = first; ptr < end; ptr++) {
+				if (*ptr) {
+					done = true;
+					break;
+				} else {
+					zeroptrs++;
+				}
+			}
+		} else {
+			zeroptrs = sdp->sd_inptrs;
+		}
+		holestep = min(factor * zeroptrs,
+			       isize_blks - (lblock + (zeroptrs * holesz)));
+		holesz += holestep;
+		if (lblock + holesz >= isize_blks) {
+			holesz = isize_blks - lblock;
+			break;
+		}
 
-int gfs2_block_map(struct inode *inode, sector_t lblock,
-		   struct buffer_head *bh_map, int create)
+		factor *= sdp->sd_inptrs;
+		if (hgt && (mp->mp_list[hgt - 1] < mp_eof.mp_list[hgt - 1]))
+			(mp->mp_list[hgt - 1])++;
+	}
+	return holesz << inode->i_blkbits;
+}
+
+/**
+ * gfs2_get_iomap - Map blocks from an inode to disk blocks
+ * @mapping: The address space
+ * @pos: Starting position in bytes
+ * @length: Length to map, in bytes
+ * @iomap: The iomap structure
+ * @mp: An uninitialized metapath structure
+ * @release_mpath: if true, we need to release the metapath when done
+ *
+ * Returns: errno
+ */
+static int _gfs2_get_iomap(struct inode *inode, loff_t pos, ssize_t length,
+			   struct iomap *iomap, struct metapath *mp,
+			   bool release_mpath)
 {
 	struct gfs2_inode *ip = GFS2_I(inode);
 	struct gfs2_sbd *sdp = GFS2_SB(inode);
 	unsigned int bsize = sdp->sd_sb.sb_bsize;
-	const size_t maxlen = bh_map->b_size >> inode->i_blkbits;
 	const u64 *arr = sdp->sd_heightsize;
 	__be64 *ptr;
-	u64 size;
-	struct metapath mp;
+	sector_t lblock = pos >> sdp->sd_sb.sb_bsize_shift;
 	int ret;
 	int eob;
 	unsigned int len;
 	struct buffer_head *bh;
 	u8 height;
-	bool zero_new = false;
-	sector_t dblock;
-	unsigned int dblks = 0;
-
-	BUG_ON(maxlen == 0);
-
-	memset(mp.mp_bh, 0, sizeof(mp.mp_bh));
-	bmap_lock(ip, create);
-	clear_buffer_mapped(bh_map);
-	clear_buffer_new(bh_map);
-	clear_buffer_boundary(bh_map);
-	trace_gfs2_bmap(ip, bh_map, lblock, create, 1);
+	loff_t isize = i_size_read(inode);
+
+	if (length == 0)
+		return -EINVAL;
+
+	iomap->offset = pos;
+	iomap->blkno = IOMAP_NULL_BLOCK;
+	iomap->type = IOMAP_HOLE;
+	iomap->length = length;
+	mp->mp_aheight = 1;
+	memset(&mp->mp_bh, 0, sizeof(mp->mp_bh));
+	bmap_lock(ip, 0);
 	if (gfs2_is_dir(ip)) {
 		bsize = sdp->sd_jbsize;
 		arr = sdp->sd_jheightsize;
 	}
 
-	ret = gfs2_meta_inode_buffer(ip, &mp.mp_bh[0]);
+	ret = gfs2_meta_inode_buffer(ip, &(mp->mp_bh[0]));
 	if (ret)
 		goto out;
 
 	height = ip->i_height;
-	size = (lblock + 1) * bsize;
-	while (size > arr[height])
+	while ((lblock + 1) * bsize > arr[height])
 		height++;
-	find_metapath(sdp, lblock, &mp, height);
-	mp.mp_aheight = 1;
+	find_metapath(sdp, lblock, mp, height);
 	if (height > ip->i_height || gfs2_is_stuffed(ip))
-		goto do_alloc;
-	ret = lookup_metapath(ip, &mp);
+		goto out;
+
+	ret = lookup_metapath(ip, mp);
 	if (ret < 0)
 		goto out;
-	if (mp.mp_aheight != ip->i_height)
-		goto do_alloc;
-	ptr = metapointer(ip->i_height - 1, &mp);
-	if (*ptr == 0)
-		goto do_alloc;
-	map_bh(bh_map, inode->i_sb, be64_to_cpu(*ptr));
-	bh = mp.mp_bh[ip->i_height - 1];
-	len = gfs2_extent_length(bh->b_data, bh->b_size, ptr, maxlen, &eob);
-	bh_map->b_size = (len << inode->i_blkbits);
-	if (eob)
-		set_buffer_boundary(bh_map);
+
 	ret = 0;
+	if (mp->mp_aheight != ip->i_height) {
+		iomap->length = hole_size(inode, lblock, mp);
+		goto out;
+	}
+
+	ptr = metapointer(ip->i_height - 1, mp);
+	if (*ptr) {
+		iomap->type = IOMAP_MAPPED;
+		iomap->blkno = be64_to_cpu(*ptr);
+	}
+
+	bh = mp->mp_bh[ip->i_height - 1];
+	len = gfs2_extent_length(bh->b_data, bh->b_size, ptr,
+				 length >> inode->i_blkbits, &eob);
+	iomap->length = len << sdp->sd_sb.sb_bsize_shift;
+	/* If we go past eof, round up to the nearest block */
+	if (iomap->offset + iomap->length >= isize)
+		iomap->length = round_up((isize - iomap->offset), bsize);
+
 out:
-	release_metapath(&mp);
-	trace_gfs2_bmap(ip, bh_map, lblock, create, ret);
-	bmap_unlock(ip, create);
+	if (ret || release_mpath)
+		release_metapath(mp);
+	bmap_unlock(ip, 0);
 	return ret;
+}
+
+int gfs2_get_iomap(struct inode *inode, loff_t pos, ssize_t length,
+		   struct iomap *iomap)
+{
+	struct metapath mp;
+
+	return _gfs2_get_iomap(inode, pos, length, iomap, &mp, true);
+}
+
+/**
+ * gfs2_block_map - Map a block from an inode to a disk block
+ * @inode: The inode
+ * @lblock: The logical block number
+ * @bh_map: The bh to be mapped
+ * @create: True if its ok to alloc blocks to satify the request
+ *
+ * Sets buffer_mapped() if successful, sets buffer_boundary() if a
+ * read of metadata will be required before the next block can be
+ * mapped. Sets buffer_new() if new blocks were allocated.
+ *
+ * Returns: errno
+ */
 
-do_alloc:
-	/* All allocations are done here, firstly check create flag */
-	if (!create) {
-		BUG_ON(gfs2_is_stuffed(ip));
-		ret = 0;
+int gfs2_block_map(struct inode *inode, sector_t lblock,
+		   struct buffer_head *bh_map, int create)
+{
+	struct gfs2_inode *ip = GFS2_I(inode);
+	struct gfs2_sbd *sdp = GFS2_SB(inode);
+	const size_t maxlen = bh_map->b_size >> inode->i_blkbits;
+	struct metapath mp;
+	struct iomap iomap;
+	sector_t dblock;
+	unsigned int dblks;
+	bool zero_new = false;
+	int ret;
+
+	BUG_ON(maxlen == 0);
+
+	clear_buffer_mapped(bh_map);
+	clear_buffer_new(bh_map);
+	clear_buffer_boundary(bh_map);
+	trace_gfs2_bmap(ip, bh_map, lblock, create, 1);
+
+	/* If we're asked to alloc any missing blocks, we need to preserve
+	 * the metapath buffers and release them ourselves. */
+	ret = _gfs2_get_iomap(inode, lblock << sdp->sd_sb.sb_bsize_shift,
+			      bh_map->b_size, &iomap, &mp, false);
+
+	bh_map->b_size = iomap.length;
+	if (maxlen >= iomap.length)
+		set_buffer_boundary(bh_map);
+
+	if (ret)
 		goto out;
-	}
 
-	/* At this point ret is the tree depth of already allocated blocks */
-	if (buffer_zeronew(bh_map))
-		zero_new = true;
-	ret = gfs2_bmap_alloc(inode, lblock, zero_new, &mp, maxlen, &dblock,
-			      &dblks);
-	if (ret == 0) {
-		map_bh(bh_map, inode->i_sb, dblock);
-		bh_map->b_size = dblks << inode->i_blkbits;
-		set_buffer_new(bh_map);
+	if (iomap.type == IOMAP_MAPPED) {
+		map_bh(bh_map, inode->i_sb, iomap.blkno);
+		bh_map->b_size = iomap.length;
+	} else if (create) {
+		if (buffer_zeronew(bh_map))
+			zero_new = true;
+		ret = gfs2_bmap_alloc(inode, lblock, zero_new, &mp, maxlen,
+				      &dblock, &dblks);
+		if (ret == 0) {
+			map_bh(bh_map, inode->i_sb, dblock);
+			bh_map->b_size = dblks << inode->i_blkbits;
+			set_buffer_new(bh_map);
+		}
 	}
-	goto out;
+	release_metapath(&mp);
+out:
+	trace_gfs2_bmap(ip, bh_map, lblock, create, ret);
+	return ret;
 }
 
 /*
diff --git a/fs/gfs2/bmap.h b/fs/gfs2/bmap.h
index 81ded5e..8da2429 100644
--- a/fs/gfs2/bmap.h
+++ b/fs/gfs2/bmap.h
@@ -10,6 +10,8 @@
 #ifndef __BMAP_DOT_H__
 #define __BMAP_DOT_H__
 
+#include <linux/iomap.h>
+
 #include "inode.h"
 
 struct inode;
@@ -47,6 +49,8 @@ static inline void gfs2_write_calc_reserv(const struct gfs2_inode *ip,
 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_get_iomap(struct inode *inode, loff_t pos,
+			  ssize_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);
-- 
2.7.4

[Cluster-devel] [PATCH 4/4] GFS2: Switch fiemap implementation to use iomap

[Bob Peterson]

This patch switches GFS2's implementation of fiemap from the old
block_map code to the new iomap interface.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/Kconfig       |  1 +
 fs/gfs2/inode.c       | 60 ++++++++++++++++++++++++++++++++++++++++-----------
 include/linux/iomap.h |  1 +
 3 files changed, 50 insertions(+), 12 deletions(-)

diff --git a/fs/gfs2/Kconfig b/fs/gfs2/Kconfig
index 90c6a8f..43c827a 100644
--- a/fs/gfs2/Kconfig
+++ b/fs/gfs2/Kconfig
@@ -4,6 +4,7 @@ config GFS2_FS
 	select FS_POSIX_ACL
 	select CRC32
 	select QUOTACTL
+	select FS_IOMAP
 	help
 	  A cluster filesystem.
 
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index fe3f849..2ccb767 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -17,7 +17,7 @@
 #include <linux/posix_acl.h>
 #include <linux/gfs2_ondisk.h>
 #include <linux/crc32.h>
-#include <linux/fiemap.h>
+#include <linux/iomap.h>
 #include <linux/security.h>
 #include <asm/uaccess.h>
 
@@ -1994,28 +1994,64 @@ static int gfs2_getattr(struct vfsmount *mnt, struct dentry *dentry,
 	return 0;
 }
 
+static int gfs2_iomap_fiemap_begin(struct inode *inode, loff_t offset,
+				   loff_t length, unsigned flags,
+				   struct iomap *iomap)
+{
+	struct gfs2_inode *ip = GFS2_I(inode);
+	struct gfs2_holder *gh;
+	int ret;
+
+	gh = kzalloc(sizeof(struct gfs2_holder), GFP_NOFS);
+	if (!gh)
+		return -ENOMEM;
+
+	ret = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED, 0, gh);
+	if (ret) {
+		kfree(gh);
+		return ret;
+	}
+	iomap->private = (void *)gh;
+	ret = gfs2_get_iomap(inode, offset, length, iomap);
+	if (ret)
+		gfs2_glock_dq_uninit(gh);
+	return ret;
+}
+
+static int gfs2_iomap_fiemap_end(struct inode *inode, loff_t offset,
+				 loff_t length, ssize_t written,
+				 unsigned flags, struct iomap *iomap)
+{
+	struct gfs2_holder *gh = (struct gfs2_holder *)iomap->private;
+
+	BUG_ON(gh == NULL);
+	gfs2_glock_dq_uninit(gh);
+	return 0;
+}
+
 static int gfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 		       u64 start, u64 len)
 {
 	struct gfs2_inode *ip = GFS2_I(inode);
-	struct gfs2_holder gh;
 	int ret;
 
-	ret = fiemap_check_flags(fieinfo, FIEMAP_FLAG_SYNC);
-	if (ret)
-		return ret;
+	struct iomap_ops gfs2_iomap_fiemap_ops = {
+		.iomap_begin = gfs2_iomap_fiemap_begin,
+		.iomap_end = gfs2_iomap_fiemap_end,
+	};
 
 	inode_lock(inode);
 
-	ret = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED, 0, &gh);
-	if (ret)
-		goto out;
-
 	if (gfs2_is_stuffed(ip)) {
+		struct gfs2_holder gh;
 		u64 phys = ip->i_no_addr << inode->i_blkbits;
 		u64 size = i_size_read(inode);
 		u32 flags = FIEMAP_EXTENT_LAST|FIEMAP_EXTENT_NOT_ALIGNED|
 			    FIEMAP_EXTENT_DATA_INLINE;
+		ret = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED, 0, &gh);
+		if (ret)
+			goto out;
+
 		phys += sizeof(struct gfs2_dinode);
 		phys += start;
 		if (start + len > size)
@@ -2025,12 +2061,12 @@ static int gfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 						      len, flags);
 		if (ret == 1)
 			ret = 0;
+		gfs2_glock_dq_uninit(&gh);
 	} else {
-		ret = __generic_block_fiemap(inode, fieinfo, start, len,
-					     gfs2_block_map);
+		ret = iomap_fiemap(inode, fieinfo, start, len,
+				   &gfs2_iomap_fiemap_ops);
 	}
 
-	gfs2_glock_dq_uninit(&gh);
 out:
 	inode_unlock(inode);
 	return ret;
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index e63e288..0f177d3 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -37,6 +37,7 @@ struct iomap {
 	u16			type;	/* type of mapping */
 	u16			flags;	/* flags for mapping */
 	struct block_device	*bdev;	/* block device for I/O */
+	void			*private; /* private value for fs use */
 };
 
 /*
-- 
2.7.4

[Cluster-devel] [PATCH 3/4] GFS2: Implement iomap for block_map

[Steven Whitehouse]

Hi,

On 20/10/16 17:14, Bob Peterson wrote:
> This patch implements iomap for block mapping, and switches the
> block_map function to use it under the covers.
Definitely going in the right direction now... a few comments below.

> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> ---
>   fs/gfs2/bmap.c | 244 ++++++++++++++++++++++++++++++++++++++++++---------------
>   fs/gfs2/bmap.h |   4 +
>   2 files changed, 185 insertions(+), 63 deletions(-)
>
> diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
> index 774bdb8..b1bcdd6 100644
> --- a/fs/gfs2/bmap.c
> +++ b/fs/gfs2/bmap.c
> @@ -13,6 +13,7 @@
>   #include <linux/blkdev.h>
>   #include <linux/gfs2_ondisk.h>
>   #include <linux/crc32.h>
> +#include <linux/iomap.h>
>   
>   #include "gfs2.h"
>   #include "incore.h"
> @@ -594,104 +595,221 @@ static int gfs2_bmap_alloc(struct inode *inode, const sector_t lblock,
>   }
>   
>   /**
> - * gfs2_block_map - Map a block from an inode to a disk block
> - * @inode: The inode
> - * @lblock: The logical block number
> - * @bh_map: The bh to be mapped
> - * @create: True if its ok to alloc blocks to satify the request
> + * hole_size - figure out the size of a hole
> + * @ip: The inode
> + * @lblock: The logical starting block number
> + * @mp: The metapath
>    *
> - * Sets buffer_mapped() if successful, sets buffer_boundary() if a
> - * read of metadata will be required before the next block can be
> - * mapped. Sets buffer_new() if new blocks were allocated.
> + * Returns: The hole size in bytes
>    *
> - * Returns: errno
>    */
> +static u64 hole_size(struct inode *inode, sector_t lblock, struct metapath *mp)
> +{
> +	struct gfs2_inode *ip = GFS2_I(inode);
> +	struct gfs2_sbd *sdp = GFS2_SB(inode);
> +	struct metapath mp_eof;
> +	unsigned int end_of_metadata = ip->i_height - 1;
> +	u64 factor = 1;
> +	int hgt = end_of_metadata;
> +	u64 holesz = 0, holestep;
> +	const __be64 *first, *end, *ptr;
> +	const struct buffer_head *bh;
> +	u64 isize_blks = (i_size_read(inode) - 1) >> inode->i_blkbits;
> +	int zeroptrs;
> +	bool done = false;
> +
> +	/* Get another metapath, to the very last byte */
> +	find_metapath(sdp, isize_blks, &mp_eof, ip->i_height);
> +	for (hgt = end_of_metadata; hgt >= 0 && !done; hgt--) {
> +		bh = mp->mp_bh[hgt];
> +		if (bh) {
> +			zeroptrs = 0;
> +			first = metapointer(hgt, mp);
> +			end = (const __be64 *)(bh->b_data + bh->b_size);
> +
> +			for (ptr = first; ptr < end; ptr++) {
> +				if (*ptr) {
> +					done = true;
> +					break;
> +				} else {
> +					zeroptrs++;
> +				}
> +			}
> +		} else {
> +			zeroptrs = sdp->sd_inptrs;
> +		}
> +		holestep = min(factor * zeroptrs,
> +			       isize_blks - (lblock + (zeroptrs * holesz)));
> +		holesz += holestep;
> +		if (lblock + holesz >= isize_blks) {
> +			holesz = isize_blks - lblock;
> +			break;
> +		}
>   
> -int gfs2_block_map(struct inode *inode, sector_t lblock,
> -		   struct buffer_head *bh_map, int create)
> +		factor *= sdp->sd_inptrs;
> +		if (hgt && (mp->mp_list[hgt - 1] < mp_eof.mp_list[hgt - 1]))
> +			(mp->mp_list[hgt - 1])++;
> +	}
> +	return holesz << inode->i_blkbits;
> +}
> +
> +/**
> + * gfs2_get_iomap - Map blocks from an inode to disk blocks
> + * @mapping: The address space
> + * @pos: Starting position in bytes
> + * @length: Length to map, in bytes
> + * @iomap: The iomap structure
> + * @mp: An uninitialized metapath structure
> + * @release_mpath: if true, we need to release the metapath when done
> + *
> + * Returns: errno
> + */
> +static int _gfs2_get_iomap(struct inode *inode, loff_t pos, ssize_t length,
> +			   struct iomap *iomap, struct metapath *mp,
> +			   bool release_mpath)
>   {
>   	struct gfs2_inode *ip = GFS2_I(inode);
>   	struct gfs2_sbd *sdp = GFS2_SB(inode);
>   	unsigned int bsize = sdp->sd_sb.sb_bsize;
> -	const size_t maxlen = bh_map->b_size >> inode->i_blkbits;
>   	const u64 *arr = sdp->sd_heightsize;
>   	__be64 *ptr;
> -	u64 size;
> -	struct metapath mp;
> +	sector_t lblock = pos >> sdp->sd_sb.sb_bsize_shift;
>   	int ret;
>   	int eob;
>   	unsigned int len;
>   	struct buffer_head *bh;
>   	u8 height;
> -	bool zero_new = false;
> -	sector_t dblock;
> -	unsigned int dblks = 0;
> -
> -	BUG_ON(maxlen == 0);
> -
> -	memset(mp.mp_bh, 0, sizeof(mp.mp_bh));
> -	bmap_lock(ip, create);
> -	clear_buffer_mapped(bh_map);
> -	clear_buffer_new(bh_map);
> -	clear_buffer_boundary(bh_map);
> -	trace_gfs2_bmap(ip, bh_map, lblock, create, 1);
> +	loff_t isize = i_size_read(inode);
> +
> +	if (length == 0)
> +		return -EINVAL;
> +
> +	iomap->offset = pos;
> +	iomap->blkno = IOMAP_NULL_BLOCK;
> +	iomap->type = IOMAP_HOLE;
> +	iomap->length = length;
> +	mp->mp_aheight = 1;
> +	memset(&mp->mp_bh, 0, sizeof(mp->mp_bh));
> +	bmap_lock(ip, 0);
>   	if (gfs2_is_dir(ip)) {
>   		bsize = sdp->sd_jbsize;
>   		arr = sdp->sd_jheightsize;
>   	}
>   
> -	ret = gfs2_meta_inode_buffer(ip, &mp.mp_bh[0]);
> +	ret = gfs2_meta_inode_buffer(ip, &(mp->mp_bh[0]));
>   	if (ret)
>   		goto out;
>   
>   	height = ip->i_height;
> -	size = (lblock + 1) * bsize;
> -	while (size > arr[height])
> +	while ((lblock + 1) * bsize > arr[height])
>   		height++;
> -	find_metapath(sdp, lblock, &mp, height);
> -	mp.mp_aheight = 1;
> +	find_metapath(sdp, lblock, mp, height);
>   	if (height > ip->i_height || gfs2_is_stuffed(ip))
> -		goto do_alloc;
> -	ret = lookup_metapath(ip, &mp);
> +		goto out;
> +
> +	ret = lookup_metapath(ip, mp);
>   	if (ret < 0)
>   		goto out;
> -	if (mp.mp_aheight != ip->i_height)
> -		goto do_alloc;
> -	ptr = metapointer(ip->i_height - 1, &mp);
> -	if (*ptr == 0)
> -		goto do_alloc;
> -	map_bh(bh_map, inode->i_sb, be64_to_cpu(*ptr));
> -	bh = mp.mp_bh[ip->i_height - 1];
> -	len = gfs2_extent_length(bh->b_data, bh->b_size, ptr, maxlen, &eob);
> -	bh_map->b_size = (len << inode->i_blkbits);
> -	if (eob)
> -		set_buffer_boundary(bh_map);
> +
>   	ret = 0;
> +	if (mp->mp_aheight != ip->i_height) {
> +		iomap->length = hole_size(inode, lblock, mp);
> +		goto out;
> +	}
> +
> +	ptr = metapointer(ip->i_height - 1, mp);
> +	if (*ptr) {
> +		iomap->type = IOMAP_MAPPED;
> +		iomap->blkno = be64_to_cpu(*ptr);
> +	}
> +
> +	bh = mp->mp_bh[ip->i_height - 1];
> +	len = gfs2_extent_length(bh->b_data, bh->b_size, ptr,
> +				 length >> inode->i_blkbits, &eob);
> +	iomap->length = len << sdp->sd_sb.sb_bsize_shift;
> +	/* If we go past eof, round up to the nearest block */
> +	if (iomap->offset + iomap->length >= isize)
> +		iomap->length = round_up((isize - iomap->offset), bsize);
> +
>   out:
> -	release_metapath(&mp);
> -	trace_gfs2_bmap(ip, bh_map, lblock, create, ret);
> -	bmap_unlock(ip, create);
> +	if (ret || release_mpath)
> +		release_metapath(mp);
> +	bmap_unlock(ip, 0);
>   	return ret;
> +}
> +
> +int gfs2_get_iomap(struct inode *inode, loff_t pos, ssize_t length,
> +		   struct iomap *iomap)
> +{
> +	struct metapath mp;
> +
> +	return _gfs2_get_iomap(inode, pos, length, iomap, &mp, true);
> +}
> +
> +/**
> + * gfs2_block_map - Map a block from an inode to a disk block
> + * @inode: The inode
> + * @lblock: The logical block number
> + * @bh_map: The bh to be mapped
> + * @create: True if its ok to alloc blocks to satify the request
> + *
> + * Sets buffer_mapped() if successful, sets buffer_boundary() if a
> + * read of metadata will be required before the next block can be
> + * mapped. Sets buffer_new() if new blocks were allocated.
> + *
> + * Returns: errno
> + */
>   
> -do_alloc:
> -	/* All allocations are done here, firstly check create flag */
> -	if (!create) {
> -		BUG_ON(gfs2_is_stuffed(ip));
> -		ret = 0;
> +int gfs2_block_map(struct inode *inode, sector_t lblock,
> +		   struct buffer_head *bh_map, int create)
> +{
> +	struct gfs2_inode *ip = GFS2_I(inode);
> +	struct gfs2_sbd *sdp = GFS2_SB(inode);
> +	const size_t maxlen = bh_map->b_size >> inode->i_blkbits;
> +	struct metapath mp;
> +	struct iomap iomap;
> +	sector_t dblock;
> +	unsigned int dblks;
> +	bool zero_new = false;
> +	int ret;
> +
> +	BUG_ON(maxlen == 0);
> +
> +	clear_buffer_mapped(bh_map);
> +	clear_buffer_new(bh_map);
> +	clear_buffer_boundary(bh_map);
> +	trace_gfs2_bmap(ip, bh_map, lblock, create, 1);
> +
> +	/* If we're asked to alloc any missing blocks, we need to preserve
> +	 * the metapath buffers and release them ourselves. */
> +	ret = _gfs2_get_iomap(inode, lblock << sdp->sd_sb.sb_bsize_shift,
> +			      bh_map->b_size, &iomap, &mp, false);
> +
> +	bh_map->b_size = iomap.length;
> +	if (maxlen >= iomap.length)
> +		set_buffer_boundary(bh_map);
> +
> +	if (ret)
>   		goto out;
> -	}
>   
> -	/* At this point ret is the tree depth of already allocated blocks */
> -	if (buffer_zeronew(bh_map))
> -		zero_new = true;
> -	ret = gfs2_bmap_alloc(inode, lblock, zero_new, &mp, maxlen, &dblock,
> -			      &dblks);
> -	if (ret == 0) {
> -		map_bh(bh_map, inode->i_sb, dblock);
> -		bh_map->b_size = dblks << inode->i_blkbits;
> -		set_buffer_new(bh_map);
So far, so good I think...

> +	if (iomap.type == IOMAP_MAPPED) {
> +		map_bh(bh_map, inode->i_sb, iomap.blkno);
> +		bh_map->b_size = iomap.length;
> +	} else if (create) {
> +		if (buffer_zeronew(bh_map))
> +			zero_new = true;
> +		ret = gfs2_bmap_alloc(inode, lblock, zero_new, &mp, maxlen,
> +				      &dblock, &dblks);
however gfs2_bmap_alloc() should take the struct iomap and not the 
individual parameters. Also why is this call to gfs2_bmap_alloc() done 
here rather than in gfs2_get_iomap() ? That way there is no need to pass 
the mp around. The zero_new flag should just become an iomap flag too, 
and that should clean this bit up.

I think that gfs2_block_map() should just be a wrapper for the new 
gfs2_get_iomap() function and not try to bypass it for the create case,

Steve.

> +		if (ret == 0) {
> +			map_bh(bh_map, inode->i_sb, dblock);
> +			bh_map->b_size = dblks << inode->i_blkbits;
> +			set_buffer_new(bh_map);
> +		}
>   	}
> -	goto out;
> +	release_metapath(&mp);
> +out:
> +	trace_gfs2_bmap(ip, bh_map, lblock, create, ret);
> +	return ret;
>   }
>   
>   /*
> diff --git a/fs/gfs2/bmap.h b/fs/gfs2/bmap.h
> index 81ded5e..8da2429 100644
> --- a/fs/gfs2/bmap.h
> +++ b/fs/gfs2/bmap.h
> @@ -10,6 +10,8 @@
>   #ifndef __BMAP_DOT_H__
>   #define __BMAP_DOT_H__
>   
> +#include <linux/iomap.h>
> +
>   #include "inode.h"
>   
>   struct inode;
> @@ -47,6 +49,8 @@ static inline void gfs2_write_calc_reserv(const struct gfs2_inode *ip,
>   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_get_iomap(struct inode *inode, loff_t pos,
> +			  ssize_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);

[Cluster-devel] [PATCH 4/4] GFS2: Switch fiemap implementation to use iomap

[Christoph Hellwig]

> +	iomap->private = (void *)gh;

No need to cast here.  And while we're at it:  please split the
addition of ->private into a separate patch.

> +	struct gfs2_holder *gh = (struct gfs2_holder *)iomap->private;

Same here.