remove generic_block_fiemap

remove generic_block_fiemap

[Christoph Hellwig]

Hi all,

this series removes the get_block-based generic_block_fiemap helper
by switching the last two users to use the iomap version instead.

The ext2 version has been tested using xfstests, but the hpfs one
is only compile tested due to the lack of easy to run tests.

diffstat:
 fs/ext2/inode.c        |   15 +--
 fs/hpfs/file.c         |   51 ++++++++++++
 fs/ioctl.c             |  203 -------------------------------------------------
 include/linux/fiemap.h |    4 
 4 files changed, 58 insertions(+), 215 deletions(-)

[PATCH 1/4] ext2: make ext2_iomap_ops available unconditionally

[Christoph Hellwig]

ext2_iomap_ops will be used for the FIEMAP support going forward,
so make it available unconditionally.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/ext2/inode.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index dadb121beb22..3e9a04770f49 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -799,7 +799,6 @@ int ext2_get_block(struct inode *inode, sector_t iblock,
 
 }
 
-#ifdef CONFIG_FS_DAX
 static int ext2_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
 		unsigned flags, struct iomap *iomap, struct iomap *srcmap)
 {
@@ -852,10 +851,6 @@ const struct iomap_ops ext2_iomap_ops = {
 	.iomap_begin		= ext2_iomap_begin,
 	.iomap_end		= ext2_iomap_end,
 };
-#else
-/* Define empty ops for !CONFIG_FS_DAX case to avoid ugly ifdefs */
-const struct iomap_ops ext2_iomap_ops;
-#endif /* CONFIG_FS_DAX */
 
 int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 		u64 start, u64 len)
-- 
2.30.2

Re: [PATCH 1/4] ext2: make ext2_iomap_ops available unconditionally

[Jan Kara]

On Tue 20-07-21 15:33:38, Christoph Hellwig wrote:
> ext2_iomap_ops will be used for the FIEMAP support going forward,
> so make it available unconditionally.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext2/inode.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> index dadb121beb22..3e9a04770f49 100644
> --- a/fs/ext2/inode.c
> +++ b/fs/ext2/inode.c
> @@ -799,7 +799,6 @@ int ext2_get_block(struct inode *inode, sector_t iblock,
>  
>  }
>  
> -#ifdef CONFIG_FS_DAX
>  static int ext2_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
>  		unsigned flags, struct iomap *iomap, struct iomap *srcmap)
>  {
> @@ -852,10 +851,6 @@ const struct iomap_ops ext2_iomap_ops = {
>  	.iomap_begin		= ext2_iomap_begin,
>  	.iomap_end		= ext2_iomap_end,
>  };
> -#else
> -/* Define empty ops for !CONFIG_FS_DAX case to avoid ugly ifdefs */
> -const struct iomap_ops ext2_iomap_ops;
> -#endif /* CONFIG_FS_DAX */
>  
>  int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>  		u64 start, u64 len)
> -- 
> 2.30.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH 2/4] ext2: use iomap_fiemap to implement ->fiemap

[Christoph Hellwig]

Switch from generic_block_fiemap to use the iomap version.  The only
interesting part is that ext2_get_blocks gets confused when being
asked for overly long ranges, so copy over the limit to the inode
size from generic_block_fiemap into ext2_fiemap.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/ext2/inode.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 3e9a04770f49..04f0def0f5eb 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -855,8 +855,14 @@ const struct iomap_ops ext2_iomap_ops = {
 int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 		u64 start, u64 len)
 {
-	return generic_block_fiemap(inode, fieinfo, start, len,
-				    ext2_get_block);
+	int ret;
+
+	inode_lock(inode);
+	len = min_t(u64, len, i_size_read(inode));
+	ret = iomap_fiemap(inode, fieinfo, start, len, &ext2_iomap_ops);
+	inode_unlock(inode);
+
+	return ret;
 }
 
 static int ext2_writepage(struct page *page, struct writeback_control *wbc)
-- 
2.30.2

Re: [PATCH 2/4] ext2: use iomap_fiemap to implement ->fiemap

[Jan Kara]

On Tue 20-07-21 15:33:39, Christoph Hellwig wrote:
> Switch from generic_block_fiemap to use the iomap version.  The only
> interesting part is that ext2_get_blocks gets confused when being
> asked for overly long ranges, so copy over the limit to the inode
> size from generic_block_fiemap into ext2_fiemap.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good. I'm a bit wondering about the problem with long ranges... I
guess these are larger than s_maxbytes, aren't they? Anyway the solution
you've picked makes sense to me. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext2/inode.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> index 3e9a04770f49..04f0def0f5eb 100644
> --- a/fs/ext2/inode.c
> +++ b/fs/ext2/inode.c
> @@ -855,8 +855,14 @@ const struct iomap_ops ext2_iomap_ops = {
>  int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>  		u64 start, u64 len)
>  {
> -	return generic_block_fiemap(inode, fieinfo, start, len,
> -				    ext2_get_block);
> +	int ret;
> +
> +	inode_lock(inode);
> +	len = min_t(u64, len, i_size_read(inode));
> +	ret = iomap_fiemap(inode, fieinfo, start, len, &ext2_iomap_ops);
> +	inode_unlock(inode);
> +
> +	return ret;
>  }
>  
>  static int ext2_writepage(struct page *page, struct writeback_control *wbc)
> -- 
> 2.30.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH 3/4] hpfs: use iomap_fiemap to implement ->fiemap

[Christoph Hellwig]

hpfs is the last user of generic_block_fiemap, so add a trivial
iomap_ops based on the ext2 version and switch to iomap_fiemap.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/hpfs/file.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/fs/hpfs/file.c b/fs/hpfs/file.c
index c3a49aacf20a..fb37f57130aa 100644
--- a/fs/hpfs/file.c
+++ b/fs/hpfs/file.c
@@ -9,6 +9,7 @@
 
 #include "hpfs_fn.h"
 #include <linux/mpage.h>
+#include <linux/iomap.h>
 #include <linux/fiemap.h>
 
 #define BLOCKS(size) (((size) + 511) >> 9)
@@ -116,6 +117,47 @@ static int hpfs_get_block(struct inode *inode, sector_t iblock, struct buffer_he
 	return r;
 }
 
+static int hpfs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
+		unsigned flags, struct iomap *iomap, struct iomap *srcmap)
+{
+	struct super_block *sb = inode->i_sb;
+	unsigned int blkbits = inode->i_blkbits;
+	unsigned int n_secs;
+	secno s;
+
+	if (WARN_ON_ONCE(flags & (IOMAP_WRITE | IOMAP_ZERO)))
+		return -EINVAL;
+
+	iomap->bdev = inode->i_sb->s_bdev;
+	iomap->offset = offset;
+
+	hpfs_lock(sb);
+	s = hpfs_bmap(inode, offset >> blkbits, &n_secs);
+	if (s) {
+		n_secs = hpfs_search_hotfix_map_for_range(sb, s,
+				min_t(loff_t, n_secs, length));
+		if (unlikely(!n_secs)) {
+			s = hpfs_search_hotfix_map(sb, s);
+			n_secs = 1;
+		}
+		iomap->type = IOMAP_MAPPED;
+		iomap->flags = IOMAP_F_MERGED;
+		iomap->addr = (u64)s << blkbits;
+		iomap->length = (u64)n_secs << blkbits;
+	} else {
+		iomap->type = IOMAP_HOLE;
+		iomap->addr = IOMAP_NULL_ADDR;
+		iomap->length = 1 << blkbits;
+	}
+
+	hpfs_unlock(sb);
+	return 0;
+}
+
+static const struct iomap_ops hpfs_iomap_ops = {
+	.iomap_begin		= hpfs_iomap_begin,
+};
+
 static int hpfs_readpage(struct file *file, struct page *page)
 {
 	return mpage_readpage(page, hpfs_get_block);
@@ -192,7 +234,14 @@ static sector_t _hpfs_bmap(struct address_space *mapping, sector_t block)
 
 static int hpfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, u64 start, u64 len)
 {
-	return generic_block_fiemap(inode, fieinfo, start, len, hpfs_get_block);
+	int ret;
+
+	inode_lock(inode);
+	len = min_t(u64, len, i_size_read(inode));
+	ret = iomap_fiemap(inode, fieinfo, start, len, &hpfs_iomap_ops);
+	inode_unlock(inode);
+
+	return ret;
 }
 
 const struct address_space_operations hpfs_aops = {
-- 
2.30.2

[PATCH 4/4] fs: remove generic_block_fiemap

[Christoph Hellwig]

Remove the now unused generic_block_fiemap helper.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/ioctl.c             | 203 -----------------------------------------
 include/linux/fiemap.h |   4 -
 2 files changed, 207 deletions(-)

diff --git a/fs/ioctl.c b/fs/ioctl.c
index 1e2204fa9963..eea8267ae1f2 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -263,209 +263,6 @@ static long ioctl_file_clone_range(struct file *file,
 				args.src_length, args.dest_offset);
 }
 
-#ifdef CONFIG_BLOCK
-
-static inline sector_t logical_to_blk(struct inode *inode, loff_t offset)
-{
-	return (offset >> inode->i_blkbits);
-}
-
-static inline loff_t blk_to_logical(struct inode *inode, sector_t blk)
-{
-	return (blk << inode->i_blkbits);
-}
-
-/**
- * __generic_block_fiemap - FIEMAP for block based inodes (no locking)
- * @inode: the inode to map
- * @fieinfo: the fiemap info struct that will be passed back to userspace
- * @start: where to start mapping in the inode
- * @len: how much space to map
- * @get_block: the fs's get_block function
- *
- * This does FIEMAP for block based inodes.  Basically it will just loop
- * through get_block until we hit the number of extents we want to map, or we
- * go past the end of the file and hit a hole.
- *
- * If it is possible to have data blocks beyond a hole past @inode->i_size, then
- * please do not use this function, it will stop at the first unmapped block
- * beyond i_size.
- *
- * If you use this function directly, you need to do your own locking. Use
- * generic_block_fiemap if you want the locking done for you.
- */
-static int __generic_block_fiemap(struct inode *inode,
-			   struct fiemap_extent_info *fieinfo, loff_t start,
-			   loff_t len, get_block_t *get_block)
-{
-	struct buffer_head map_bh;
-	sector_t start_blk, last_blk;
-	loff_t isize = i_size_read(inode);
-	u64 logical = 0, phys = 0, size = 0;
-	u32 flags = FIEMAP_EXTENT_MERGED;
-	bool past_eof = false, whole_file = false;
-	int ret = 0;
-
-	ret = fiemap_prep(inode, fieinfo, start, &len, FIEMAP_FLAG_SYNC);
-	if (ret)
-		return ret;
-
-	/*
-	 * Either the i_mutex or other appropriate locking needs to be held
-	 * since we expect isize to not change at all through the duration of
-	 * this call.
-	 */
-	if (len >= isize) {
-		whole_file = true;
-		len = isize;
-	}
-
-	/*
-	 * Some filesystems can't deal with being asked to map less than
-	 * blocksize, so make sure our len is at least block length.
-	 */
-	if (logical_to_blk(inode, len) == 0)
-		len = blk_to_logical(inode, 1);
-
-	start_blk = logical_to_blk(inode, start);
-	last_blk = logical_to_blk(inode, start + len - 1);
-
-	do {
-		/*
-		 * we set b_size to the total size we want so it will map as
-		 * many contiguous blocks as possible at once
-		 */
-		memset(&map_bh, 0, sizeof(struct buffer_head));
-		map_bh.b_size = len;
-
-		ret = get_block(inode, start_blk, &map_bh, 0);
-		if (ret)
-			break;
-
-		/* HOLE */
-		if (!buffer_mapped(&map_bh)) {
-			start_blk++;
-
-			/*
-			 * We want to handle the case where there is an
-			 * allocated block at the front of the file, and then
-			 * nothing but holes up to the end of the file properly,
-			 * to make sure that extent at the front gets properly
-			 * marked with FIEMAP_EXTENT_LAST
-			 */
-			if (!past_eof &&
-			    blk_to_logical(inode, start_blk) >= isize)
-				past_eof = 1;
-
-			/*
-			 * First hole after going past the EOF, this is our
-			 * last extent
-			 */
-			if (past_eof && size) {
-				flags = FIEMAP_EXTENT_MERGED|FIEMAP_EXTENT_LAST;
-				ret = fiemap_fill_next_extent(fieinfo, logical,
-							      phys, size,
-							      flags);
-			} else if (size) {
-				ret = fiemap_fill_next_extent(fieinfo, logical,
-							      phys, size, flags);
-				size = 0;
-			}
-
-			/* if we have holes up to/past EOF then we're done */
-			if (start_blk > last_blk || past_eof || ret)
-				break;
-		} else {
-			/*
-			 * We have gone over the length of what we wanted to
-			 * map, and it wasn't the entire file, so add the extent
-			 * we got last time and exit.
-			 *
-			 * This is for the case where say we want to map all the
-			 * way up to the second to the last block in a file, but
-			 * the last block is a hole, making the second to last
-			 * block FIEMAP_EXTENT_LAST.  In this case we want to
-			 * see if there is a hole after the second to last block
-			 * so we can mark it properly.  If we found data after
-			 * we exceeded the length we were requesting, then we
-			 * are good to go, just add the extent to the fieinfo
-			 * and break
-			 */
-			if (start_blk > last_blk && !whole_file) {
-				ret = fiemap_fill_next_extent(fieinfo, logical,
-							      phys, size,
-							      flags);
-				break;
-			}
-
-			/*
-			 * if size != 0 then we know we already have an extent
-			 * to add, so add it.
-			 */
-			if (size) {
-				ret = fiemap_fill_next_extent(fieinfo, logical,
-							      phys, size,
-							      flags);
-				if (ret)
-					break;
-			}
-
-			logical = blk_to_logical(inode, start_blk);
-			phys = blk_to_logical(inode, map_bh.b_blocknr);
-			size = map_bh.b_size;
-			flags = FIEMAP_EXTENT_MERGED;
-
-			start_blk += logical_to_blk(inode, size);
-
-			/*
-			 * If we are past the EOF, then we need to make sure as
-			 * soon as we find a hole that the last extent we found
-			 * is marked with FIEMAP_EXTENT_LAST
-			 */
-			if (!past_eof && logical + size >= isize)
-				past_eof = true;
-		}
-		cond_resched();
-		if (fatal_signal_pending(current)) {
-			ret = -EINTR;
-			break;
-		}
-
-	} while (1);
-
-	/* If ret is 1 then we just hit the end of the extent array */
-	if (ret == 1)
-		ret = 0;
-
-	return ret;
-}
-
-/**
- * generic_block_fiemap - FIEMAP for block based inodes
- * @inode: The inode to map
- * @fieinfo: The mapping information
- * @start: The initial block to map
- * @len: The length of the extect to attempt to map
- * @get_block: The block mapping function for the fs
- *
- * Calls __generic_block_fiemap to map the inode, after taking
- * the inode's mutex lock.
- */
-
-int generic_block_fiemap(struct inode *inode,
-			 struct fiemap_extent_info *fieinfo, u64 start,
-			 u64 len, get_block_t *get_block)
-{
-	int ret;
-	inode_lock(inode);
-	ret = __generic_block_fiemap(inode, fieinfo, start, len, get_block);
-	inode_unlock(inode);
-	return ret;
-}
-EXPORT_SYMBOL(generic_block_fiemap);
-
-#endif  /*  CONFIG_BLOCK  */
-
 /*
  * This provides compatibility with legacy XFS pre-allocation ioctls
  * which predate the fallocate syscall.
diff --git a/include/linux/fiemap.h b/include/linux/fiemap.h
index 4e624c466583..c50882f19235 100644
--- a/include/linux/fiemap.h
+++ b/include/linux/fiemap.h
@@ -18,8 +18,4 @@ int fiemap_prep(struct inode *inode, struct fiemap_extent_info *fieinfo,
 int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
 			    u64 phys, u64 len, u32 flags);
 
-int generic_block_fiemap(struct inode *inode,
-		struct fiemap_extent_info *fieinfo, u64 start, u64 len,
-		get_block_t *get_block);
-
 #endif /* _LINUX_FIEMAP_H 1 */
-- 
2.30.2

Re: [PATCH 4/4] fs: remove generic_block_fiemap

[Jan Kara]

On Tue 20-07-21 15:33:41, Christoph Hellwig wrote:
> Remove the now unused generic_block_fiemap helper.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Nice. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ioctl.c             | 203 -----------------------------------------
>  include/linux/fiemap.h |   4 -
>  2 files changed, 207 deletions(-)
> 
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index 1e2204fa9963..eea8267ae1f2 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -263,209 +263,6 @@ static long ioctl_file_clone_range(struct file *file,
>  				args.src_length, args.dest_offset);
>  }
>  
> -#ifdef CONFIG_BLOCK
> -
> -static inline sector_t logical_to_blk(struct inode *inode, loff_t offset)
> -{
> -	return (offset >> inode->i_blkbits);
> -}
> -
> -static inline loff_t blk_to_logical(struct inode *inode, sector_t blk)
> -{
> -	return (blk << inode->i_blkbits);
> -}
> -
> -/**
> - * __generic_block_fiemap - FIEMAP for block based inodes (no locking)
> - * @inode: the inode to map
> - * @fieinfo: the fiemap info struct that will be passed back to userspace
> - * @start: where to start mapping in the inode
> - * @len: how much space to map
> - * @get_block: the fs's get_block function
> - *
> - * This does FIEMAP for block based inodes.  Basically it will just loop
> - * through get_block until we hit the number of extents we want to map, or we
> - * go past the end of the file and hit a hole.
> - *
> - * If it is possible to have data blocks beyond a hole past @inode->i_size, then
> - * please do not use this function, it will stop at the first unmapped block
> - * beyond i_size.
> - *
> - * If you use this function directly, you need to do your own locking. Use
> - * generic_block_fiemap if you want the locking done for you.
> - */
> -static int __generic_block_fiemap(struct inode *inode,
> -			   struct fiemap_extent_info *fieinfo, loff_t start,
> -			   loff_t len, get_block_t *get_block)
> -{
> -	struct buffer_head map_bh;
> -	sector_t start_blk, last_blk;
> -	loff_t isize = i_size_read(inode);
> -	u64 logical = 0, phys = 0, size = 0;
> -	u32 flags = FIEMAP_EXTENT_MERGED;
> -	bool past_eof = false, whole_file = false;
> -	int ret = 0;
> -
> -	ret = fiemap_prep(inode, fieinfo, start, &len, FIEMAP_FLAG_SYNC);
> -	if (ret)
> -		return ret;
> -
> -	/*
> -	 * Either the i_mutex or other appropriate locking needs to be held
> -	 * since we expect isize to not change at all through the duration of
> -	 * this call.
> -	 */
> -	if (len >= isize) {
> -		whole_file = true;
> -		len = isize;
> -	}
> -
> -	/*
> -	 * Some filesystems can't deal with being asked to map less than
> -	 * blocksize, so make sure our len is at least block length.
> -	 */
> -	if (logical_to_blk(inode, len) == 0)
> -		len = blk_to_logical(inode, 1);
> -
> -	start_blk = logical_to_blk(inode, start);
> -	last_blk = logical_to_blk(inode, start + len - 1);
> -
> -	do {
> -		/*
> -		 * we set b_size to the total size we want so it will map as
> -		 * many contiguous blocks as possible at once
> -		 */
> -		memset(&map_bh, 0, sizeof(struct buffer_head));
> -		map_bh.b_size = len;
> -
> -		ret = get_block(inode, start_blk, &map_bh, 0);
> -		if (ret)
> -			break;
> -
> -		/* HOLE */
> -		if (!buffer_mapped(&map_bh)) {
> -			start_blk++;
> -
> -			/*
> -			 * We want to handle the case where there is an
> -			 * allocated block at the front of the file, and then
> -			 * nothing but holes up to the end of the file properly,
> -			 * to make sure that extent at the front gets properly
> -			 * marked with FIEMAP_EXTENT_LAST
> -			 */
> -			if (!past_eof &&
> -			    blk_to_logical(inode, start_blk) >= isize)
> -				past_eof = 1;
> -
> -			/*
> -			 * First hole after going past the EOF, this is our
> -			 * last extent
> -			 */
> -			if (past_eof && size) {
> -				flags = FIEMAP_EXTENT_MERGED|FIEMAP_EXTENT_LAST;
> -				ret = fiemap_fill_next_extent(fieinfo, logical,
> -							      phys, size,
> -							      flags);
> -			} else if (size) {
> -				ret = fiemap_fill_next_extent(fieinfo, logical,
> -							      phys, size, flags);
> -				size = 0;
> -			}
> -
> -			/* if we have holes up to/past EOF then we're done */
> -			if (start_blk > last_blk || past_eof || ret)
> -				break;
> -		} else {
> -			/*
> -			 * We have gone over the length of what we wanted to
> -			 * map, and it wasn't the entire file, so add the extent
> -			 * we got last time and exit.
> -			 *
> -			 * This is for the case where say we want to map all the
> -			 * way up to the second to the last block in a file, but
> -			 * the last block is a hole, making the second to last
> -			 * block FIEMAP_EXTENT_LAST.  In this case we want to
> -			 * see if there is a hole after the second to last block
> -			 * so we can mark it properly.  If we found data after
> -			 * we exceeded the length we were requesting, then we
> -			 * are good to go, just add the extent to the fieinfo
> -			 * and break
> -			 */
> -			if (start_blk > last_blk && !whole_file) {
> -				ret = fiemap_fill_next_extent(fieinfo, logical,
> -							      phys, size,
> -							      flags);
> -				break;
> -			}
> -
> -			/*
> -			 * if size != 0 then we know we already have an extent
> -			 * to add, so add it.
> -			 */
> -			if (size) {
> -				ret = fiemap_fill_next_extent(fieinfo, logical,
> -							      phys, size,
> -							      flags);
> -				if (ret)
> -					break;
> -			}
> -
> -			logical = blk_to_logical(inode, start_blk);
> -			phys = blk_to_logical(inode, map_bh.b_blocknr);
> -			size = map_bh.b_size;
> -			flags = FIEMAP_EXTENT_MERGED;
> -
> -			start_blk += logical_to_blk(inode, size);
> -
> -			/*
> -			 * If we are past the EOF, then we need to make sure as
> -			 * soon as we find a hole that the last extent we found
> -			 * is marked with FIEMAP_EXTENT_LAST
> -			 */
> -			if (!past_eof && logical + size >= isize)
> -				past_eof = true;
> -		}
> -		cond_resched();
> -		if (fatal_signal_pending(current)) {
> -			ret = -EINTR;
> -			break;
> -		}
> -
> -	} while (1);
> -
> -	/* If ret is 1 then we just hit the end of the extent array */
> -	if (ret == 1)
> -		ret = 0;
> -
> -	return ret;
> -}
> -
> -/**
> - * generic_block_fiemap - FIEMAP for block based inodes
> - * @inode: The inode to map
> - * @fieinfo: The mapping information
> - * @start: The initial block to map
> - * @len: The length of the extect to attempt to map
> - * @get_block: The block mapping function for the fs
> - *
> - * Calls __generic_block_fiemap to map the inode, after taking
> - * the inode's mutex lock.
> - */
> -
> -int generic_block_fiemap(struct inode *inode,
> -			 struct fiemap_extent_info *fieinfo, u64 start,
> -			 u64 len, get_block_t *get_block)
> -{
> -	int ret;
> -	inode_lock(inode);
> -	ret = __generic_block_fiemap(inode, fieinfo, start, len, get_block);
> -	inode_unlock(inode);
> -	return ret;
> -}
> -EXPORT_SYMBOL(generic_block_fiemap);
> -
> -#endif  /*  CONFIG_BLOCK  */
> -
>  /*
>   * This provides compatibility with legacy XFS pre-allocation ioctls
>   * which predate the fallocate syscall.
> diff --git a/include/linux/fiemap.h b/include/linux/fiemap.h
> index 4e624c466583..c50882f19235 100644
> --- a/include/linux/fiemap.h
> +++ b/include/linux/fiemap.h
> @@ -18,8 +18,4 @@ int fiemap_prep(struct inode *inode, struct fiemap_extent_info *fieinfo,
>  int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
>  			    u64 phys, u64 len, u32 flags);
>  
> -int generic_block_fiemap(struct inode *inode,
> -		struct fiemap_extent_info *fieinfo, u64 start, u64 len,
> -		get_block_t *get_block);
> -
>  #endif /* _LINUX_FIEMAP_H 1 */
> -- 
> 2.30.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 4/4] fs: remove generic_block_fiemap

[Christoph Hellwig]

On Mon, Jul 26, 2021 at 03:52:56PM +0200, Jan Kara wrote:
> On Tue 20-07-21 15:33:41, Christoph Hellwig wrote:
> > Remove the now unused generic_block_fiemap helper.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Nice. Feel free to add:
> 
> Reviewed-by: Jan Kara <jack@suse.cz>

Do you just want to pick the whole series up through the ext2 tree?

Re: [PATCH 4/4] fs: remove generic_block_fiemap

[Jan Kara]

On Mon 26-07-21 15:54:59, Christoph Hellwig wrote:
> On Mon, Jul 26, 2021 at 03:52:56PM +0200, Jan Kara wrote:
> > On Tue 20-07-21 15:33:41, Christoph Hellwig wrote:
> > > Remove the now unused generic_block_fiemap helper.
> > > 
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > 
> > Nice. Feel free to add:
> > 
> > Reviewed-by: Jan Kara <jack@suse.cz>
> 
> Do you just want to pick the whole series up through the ext2 tree?

Sure. I've queued it up (generic_block_fiemap removal branch, pulled in
for_next branch).

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: remove generic_block_fiemap

[Mikulas Patocka]

On Tue, 20 Jul 2021, Christoph Hellwig wrote:

> Hi all,
> 
> this series removes the get_block-based generic_block_fiemap helper
> by switching the last two users to use the iomap version instead.
> 
> The ext2 version has been tested using xfstests, but the hpfs one
> is only compile tested due to the lack of easy to run tests.

Hi

You can download a test HPFS partition here:
http://artax.karlin.mff.cuni.cz/~mikulas/vyplody/hpfs/test-hpfs-partition.gz

Mikulas


> diffstat:
>  fs/ext2/inode.c        |   15 +--
>  fs/hpfs/file.c         |   51 ++++++++++++
>  fs/ioctl.c             |  203 -------------------------------------------------
>  include/linux/fiemap.h |    4 
>  4 files changed, 58 insertions(+), 215 deletions(-)
> 

Re: remove generic_block_fiemap

[Christoph Hellwig]

On Tue, Jul 20, 2021 at 07:02:10PM +0200, Mikulas Patocka wrote:
> You can download a test HPFS partition here:
> http://artax.karlin.mff.cuni.cz/~mikulas/vyplody/hpfs/test-hpfs-partition.gz

looks like xfstests doesn't generally work on hpfs, and even if I tried
i would be rather slow due to the lack of sparse files.

So I tested fiemp against a few simple copied over files and it still
works.

There are some cases where the old code reported contiguous ranges
as two extents while the new one doesn't, for rasons that are not
quite clear to me:

--- fiemap.old	2021-07-21 05:00:29.000000000 +0000
+++ fiemap.iomap	2021-07-21 04:57:20.000000000 +0000
@@ -1,3 +1,2 @@
 dmesg:
-	0: [0..66]: 133817..133883
-	1: [67..67]: 133884..133884
+	0: [0..67]: 133817..133884