VFS: new want_holesize and got_holesize buffer_head flags for fiemap

Hi,

This is my sixth rework of this patch. The problem with the previous version is that the underlying file system's block_map function may set the buffer_head's b_state to 0, which may be misinterpreted by fiemap as meaning it has returned the hole size in b_size. This version implements a suggestion from Steve Whitehouse: it improves on the design by (unfortunately) using two flags: a flag to tell block_map to return hole size if possible, and another flag to tell fiemap that the hole size has been returned. This way there is no possibility of a misunderstanding.

The problem:
If you do a fiemap operation on a very large sparse file, it can take an extremely long amount of time (we're talking days here) because function __generic_block_fiemap does a block-for-block search when it encounters a hole.

The solution:
Allow the underlying file system to return the hole size so that function __generic_block_fiemap can quickly skip the hole. I have a companion patch to GFS2 that takes advantage of the new flags to speed up fiemap on sparse GFS2 files. Other file systems can do the same as they see fit. For GFS2, the time it takes to skip a 1PB hole in a sparse file goes from days to milliseconds.

Patch description:
This patch changes function __generic_block_fiemap so that it sets a new buffer_want_holesize bit. The new bit signals to the underlying file system to return a hole size from its block_map function (if possible) in the event that a hole is encountered at the requested block. If the block_map function encounters a hole, and sets buffer_got_holesize, fiemap takes the returned b_size to be the size of the hole, in bytes. It then skips the hole and moves to the next block. This may be repeated several times in a row, especially for large holes, due to possible limitations of the fs-specific block_map function. This is still much faster than trying each block individually when large holes are encountered. If the block_map function does not set buffer_got_holesize, the request for holesize has been ignored, and it falls back to today's method of doing a block-by-block search for the next valid block.

Regards,

Bob Peterson
Red Hat File Systems

---
 fs/ioctl.c                  | 7 ++++++-
 include/linux/buffer_head.h | 4 ++++
 2 files changed, 10 insertions(+), 1 deletion(-)

index 8ac3fad..9c07a40 100644
@@ -291,13 +291,18 @@ int __generic_block_fiemap(struct inode *inode,
 		memset(&map_bh, 0, sizeof(struct buffer_head));
 		map_bh.b_size = len;
 
+		set_buffer_want_holesize(&map_bh);
 		ret = get_block(inode, start_blk, &map_bh, 0);
 		if (ret)
 			break;
 
 		/* HOLE */
 		if (!buffer_mapped(&map_bh)) {
-			start_blk++;
+			if (buffer_got_holesize(&map_bh))
+				start_blk += logical_to_blk(inode,
+							    map_bh.b_size);
+			else
+				start_blk++;
 
 			/*
 			 * We want to handle the case where there is an

index 324329c..fc8e47c 100644
@@ -37,6 +37,8 @@ enum bh_state_bits {
 	BH_Meta,	/* Buffer contains metadata */
 	BH_Prio,	/* Buffer should be submitted with REQ_PRIO */
 	BH_Defer_Completion, /* Defer AIO completion to workqueue */
+	BH_Want_Holesize, /* Return hole size if possible */
+	BH_Got_Holesize, /* Hole size has been returned */
 
 	BH_PrivateStart,/* not a state bit, but the first bit available
 			 * for private allocation by other entities

@@ -128,6 +130,8 @@ BUFFER_FNS(Boundary, boundary)
 BUFFER_FNS(Write_EIO, write_io_error)
 BUFFER_FNS(Unwritten, unwritten)
 BUFFER_FNS(Meta, meta)
+BUFFER_FNS(Want_Holesize, want_holesize)
+BUFFER_FNS(Got_Holesize, got_holesize)
 BUFFER_FNS(Prio, prio)
 BUFFER_FNS(Defer_Completion, defer_completion)
 

1.9.3

This looks like a big indicator that get_blocks is the wrong interface for fiemap.

Hi,

The question is then, what a better interface would look like? We could have "get_hole" as an extra operation I suppose. Not sure that I really see why thats better or worse than the extra flag at the moment? or did you have something else in mind?

Steve.

----- Original Message -----

Hi Christoph,

Yes, I thought about that.
One of my early prototypes had a separate function used by fiemap. Function __generic_block_fiemap would call get_block() which returned an indication of a hole as it does today. When it saw the hole, fiemap called a new function get_hole_size() that was passed in like get_block. The problem is: it's grossly inefficient, since the new function get_hole_size() has to redo most of the work that get_block just did (at least in the case of GFS2). (Which in the case of a 1PB sparse file is non-trivial, since it involves several levels of metadata indirection). Combining it with get_block made it much more efficient.

Making a separate get_block_map_fiemap() function just seems like an exercise in redundancy.

Regards,

Bob Peterson
Red Hat File Systems

I was thinking of replacing get_blocks entirely. We're not actually using a buffer_head in fiemap, so the interface seems somewhat awkward. If it used something like the iomap interface proposed by Dave long time ago we'd have a much saner interface that for example XFS could use as well.

----- Original Message -----

Hi Christoph. Can you send a link to the thread regarding Dave's iomap? proposal? I don't recall it offhand, so I don't know what it was or why it was never implemented. I assume you mean Dave Chinner. Maybe it's time to revisit the concept as a long-term solution.

In the meantime, do you otherwise object to this patch as a short-term solution?

Regards,

Bob Peterson
Red Hat File Systems

Old patch I had is below.

This is actually on my radar again because I want to get rid of buffer heads in XFS for various reasons and this is one the interfaces I need to make that possible....

Cheers,

Dave.

Dave Chinner
david at fromorbit.com

multipage-write: introduce iomap infrastructure

From: Dave Chinner <[email protected]>

Add infrastructure for multipage writes by defining the mapping interface
that the multipage writes will use and the main multipage write loop.

Signed-off-by: Dave Chinner <[email protected]>

index 76041b6..1196877 100644
@@ -513,6 +513,7 @@ enum positive_aop_returns {
 struct page;
 struct address_space;
 struct writeback_control;
+struct iomap;
 
 struct iov_iter {
 	const struct iovec *iov;

@@ -604,6 +605,9 @@ struct address_space_operations {
 	int (*is_partially_uptodate) (struct page *, read_descriptor_t *,
 					unsigned long);
 	int (*error_remove_page)(struct address_space *, struct page *);
+
+	int (*iomap)(struct address_space *mapping, loff_t pos,
+			ssize_t length, struct iomap *iomap, int cmd);
 };
 
 /*