Re: [vfs PATCH v3 1/4] VFS: move iomap from exportfs.h to iomap.h

[Bob Peterson <rpeterso@redhat.com>]

----- Original Message -----
> On Wed, Mar 30, 2016 at 09:20:36AM +1100, Dave Chinner wrote:
> > > For now I'm not sure the infrastructure will stay in the generic code or
> > > in XFS, so for the first round I just wanted to ask for XFS feedback.
> > 
> > QA over the past couple of days has indicatedno regressions, so I'm
> > going to seriously consider the multipage write code for the next
> > XFS merge. I'd be happy to also take fiemap code based on the same
> > iomap interfaces, especially if we can make it a generic, fully
> > functional fiemap implementation and use it in XFS, too.
> 
> Ok.  Bob, is the code going to be useful for your in gfs2 as-is?  I'm
> torn a bit between adding it to common code because it actually is
> nicely abstracted and generic, and keepin it in XFS to make future
> changes easier (getting rid of buffer_heads is the big one here)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
Hi Christoph,

Sorry it's taken me a while to get back to you on this. I've been in fire-drill
mode way too long. Yes, your patches seem compatible with mine, with one
small exception, which I've adjusted for in the patch seen below, to
replace one of my previous patches.

Also, I joined the xfs mailing list and saw you were considering pulling
some of my stuff in. I guess I don't care where it gets pulled in; I thought
Al's vfs tree made sense.

I grabbed your iomap-related patches from the xfs set and applied them to
my copy of Al's VFS tree, added my GFS2 patches, and everything compiles.
I haven't tested it yet though.

Regards,

Bob Peterson
Red Hat File Systems
---
VFS: Add new __generic_iomap_fiemap interface

This patch adds a new function __generic_iomap_fiemap similar to
__generic_block_fiemap, but it uses the new iomap interface.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>

diff --git a/fs/ioctl.c b/fs/ioctl.c
index 116a333..0d04c2f 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -15,6 +15,8 @@
 #include <linux/writeback.h>
 #include <linux/buffer_head.h>
 #include <linux/falloc.h>
+#include <linux/iomap.h>
+
 #include "internal.h"
 
 #include <asm/ioctls.h>
@@ -251,6 +253,100 @@ static inline loff_t blk_to_logical(struct inode *inode, sector_t blk)
 }
 
 /**
+ * __generic_iomap_fiemap - FIEMAP for iomap 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
+ *
+ * This does FIEMAP for iomap based inodes.  Basically it will just loop
+ * through iomap 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_iomap_fiemap if you want the locking done for you.
+ */
+
+int __generic_iomap_fiemap(struct inode *inode,
+			   struct fiemap_extent_info *fieinfo, loff_t start,
+			   loff_t len, iomap_get_t iomap)
+{
+	struct iomap iom, prev_iom;
+	loff_t isize = i_size_read(inode);
+	int ret = 0;
+
+	ret = fiemap_check_flags(fieinfo, FIEMAP_FLAG_SYNC);
+	memset(&prev_iom, 0, sizeof(prev_iom));
+	if (len >= isize)
+		len = isize;
+
+	while ((ret == 0) && (start < isize) && len) {
+		memset(&iom, 0, sizeof(iom));
+		ret = iomap(inode->i_mapping, start, len, &iom);
+		if (ret)
+			break;
+
+		if (iom.type != IOMAP_HOLE) {
+			if (prev_iom.blkno)
+				ret = fiemap_fill_next_extent(fieinfo,
+							      prev_iom.offset,
+							      blk_to_logical(inode,
+							      prev_iom.blkno),
+							      prev_iom.length,
+							      FIEMAP_EXTENT_MERGED);
+			prev_iom = iom;
+		}
+		start += iom.length;
+		if (len < iom.length)
+			break;
+		len -= iom.length;
+		cond_resched();
+	}
+
+	if (prev_iom.blkno)
+		ret = fiemap_fill_next_extent(fieinfo, prev_iom.offset,
+					      blk_to_logical(inode,
+							     prev_iom.blkno),
+					      prev_iom.length,
+					      FIEMAP_EXTENT_MERGED |
+					      FIEMAP_EXTENT_LAST);
+	/* If ret is 1 then we just hit the end of the extent array */
+	if (ret == 1)
+		ret = 0;
+
+	return ret;
+}
+EXPORT_SYMBOL(__generic_iomap_fiemap);
+
+/**
+ * generic_iomap_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_iomap_fiemap(struct inode *inode,
+			 struct fiemap_extent_info *fieinfo, u64 start,
+			 u64 len, iomap_get_t iomap)
+{
+	int ret;
+	mutex_lock(&inode->i_mutex);
+	ret = __generic_iomap_fiemap(inode, fieinfo, start, len, iomap);
+	mutex_unlock(&inode->i_mutex);
+	return ret;
+}
+EXPORT_SYMBOL(generic_iomap_fiemap);
+
+/**
  * __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
diff --git a/include/linux/fs.h b/include/linux/fs.h
index c460175..31a0f5e 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -70,8 +70,12 @@ extern int sysctl_protected_symlinks;
 extern int sysctl_protected_hardlinks;
 
 struct buffer_head;
+struct address_space;
+struct iomap;
 typedef int (get_block_t)(struct inode *inode, sector_t iblock,
 			struct buffer_head *bh_result, int create);
+typedef int (iomap_get_t)(struct address_space *mapping, loff_t pos,
+			  ssize_t length, struct iomap *iomap);
 typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset,
 			ssize_t bytes, void *private);
 typedef void (dax_iodone_t)(struct buffer_head *bh_map, int uptodate);
@@ -316,7 +320,6 @@ enum positive_aop_returns {
  * oh the beauties of C type declarations.
  */
 struct page;
-struct address_space;
 struct writeback_control;
 
 #define IOCB_EVENTFD		(1 << 0)
@@ -2848,6 +2851,12 @@ extern int vfs_lstat(const char __user *, struct kstat *);
 extern int vfs_fstat(unsigned int, struct kstat *);
 extern int vfs_fstatat(int , const char __user *, struct kstat *, int);
 
+extern int __generic_iomap_fiemap(struct inode *inode,
+				  struct fiemap_extent_info *fieinfo,
+				  loff_t start, loff_t len, iomap_get_t iomap);
+extern int generic_iomap_fiemap(struct inode *inode,
+				struct fiemap_extent_info *fieinfo, u64 start,
+				u64 len, iomap_get_t iomap);
 extern int __generic_block_fiemap(struct inode *inode,
 				  struct fiemap_extent_info *fieinfo,
 				  loff_t start, loff_t len,