Re: [PATCH][RFC] fast file mapping for loop

[Jens Axboe <jens.axboe@oracle.com>]

On Mon, Jan 14 2008, Chris Mason wrote:
> Hello everyone,
> 
> Here is a modified version of Jens' patch.  The basic idea is to push
> the mapping maintenance out of loop and down into the filesystem (ext2
> in this case).
> 
> Two new address_space operations are added, one to map
> extents and the other to provide call backs into the FS as io is
> completed.
> 
> Still TODO for this patch:
> 
> * Add exclusion between filling holes and readers.  This is partly
> implemented, when a hole is filled by the FS, the extent is flagged as
> having a hole.  The idea is to check this flag before trying to read
> the blocks and just send back all zeros.
> 
> The flag would be cleared when the blocks filling the hole have been
> written.
> 
> * Exclude page cache readers and writers
> 
> * Add a way for the FS to request a commit before telling the higher
> layers the IO is complete.  This way we can make sure metadata related
> to holes is on disk before claiming the IO is really done.  COW based
> filesystems will also needed it.
> 
> * Change loop to use fast mapping only when the new address_space op is
> provided (whoops, forgot about this until just now)
> 
> * A few other bits for COW, not really relevant until there
> is...something COW using it.

Looks pretty good. Essentially the loop side is 100% the same, it just
offloads the extent ownership to the fs (where it belongs). So I like
it. Attaching a small cleanup/fixup patch for loop, don't think it needs
further explanations.

One suggestion - free_extent_map(), I would call that put_extent_map()
instead.

diff -u b/drivers/block/loop.c b/drivers/block/loop.c
--- b/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -677,13 +677,14 @@
 	if (IS_ERR(lfe))
 		return -EIO;
 
-	while(!lfe) {
+	while (!lfe) {
 		loop_schedule_extent_mapping(lo, bio->bi_sector,
 					     bio->bi_size, 1);
 		lfe = loop_lookup_extent(lo, start, GFP_ATOMIC);
 		if (IS_ERR(lfe))
 			return -EIO;
 	}
+
 	/*
 	 * handle sparse io
 	 */
@@ -802,13 +803,13 @@
 {
 	struct bio *orig_bio = bio->bi_private;
 	struct inode *inode = bio->bi_bdev->bd_inode;
+	struct address_space *mapping = inode->i_mapping;
 	u64 start = orig_bio->bi_sector << 9;
 	u64 len = bio->bi_size;
 
-	if (inode->i_mapping->a_ops->extent_io_complete) {
-		inode->i_mapping->a_ops->extent_io_complete(inode->i_mapping,
-							    start, len);
-	}
+	if (mapping->a_ops->extent_io_complete)
+		mapping->a_ops->extent_io_complete(mapping, start, len);
+
 	bio_put(bio);
 	bio_endio(orig_bio, err);
 }
@@ -829,6 +830,7 @@
 		err = -ENOMEM;
 		goto out;
 	}
+
 	/*
 	 * change the sector so we can find the correct file offset in our
 	 * endio
@@ -847,7 +849,6 @@
 		goto out;;
 	}
 
-
 	disk_block = em->block_start;
 	extent_off = start - em->start;
 	new_bio->bi_sector = (disk_block + extent_off) >> 9;
@@ -924,11 +925,8 @@
 		spin_unlock_irq(&lo->lo_lock);
 
 		BUG_ON(!bio);
-		if (lo_act_bio(bio))
-			bio_act = 1;
-		else
-			bio_act = 0;
 
+		bio_act = lo_act_bio(bio);
 		loop_handle_bio(lo, bio);
 
 		spin_lock_irq(&lo->lo_lock);
@@ -1103,11 +1101,9 @@
 		return -EINVAL;
 
 	/*
-	 * Need a working bmap. TODO: use the same optimization that
-	 * direct-io.c does for get_block() mapping more than one block
-	 * at the time.
+	 * Need a working extent_map
 	 */
-	if (inode->i_mapping->a_ops->bmap == NULL)
+	if (inode->i_mapping->a_ops->map_extent == NULL)
 		return -EINVAL;
 	/*
 	 * invalidate all page cache belonging to this file, it could become
diff -u b/include/linux/loop.h b/include/linux/loop.h
--- b/include/linux/loop.h
+++ b/include/linux/loop.h
@@ -18,7 +18,6 @@
 #include <linux/blkdev.h>
 #include <linux/spinlock.h>
 #include <linux/mutex.h>
-#include <linux/rbtree.h>
 
 /* Possible states of device */
 enum {
@@ -72,9 +71,6 @@
 	struct gendisk		*lo_disk;
 	struct list_head	lo_list;
 
-	struct prio_tree_root	prio_root;
-	struct prio_tree_node	*last_insert;
-	struct prio_tree_node	*last_lookup;
 	unsigned int		blkbits;
 };
 

-- 
Jens Axboe