[PATCH] btrfs: remove struct processed_extent

[PATCH] btrfs: remove struct processed_extent

[Christoph Hellwig]

Since commit 4a445b7b6178 ("btrfs: don't merge pages into bio if thei
r page offset is not contiguous"), all pages in buffered I/O bios
are contigous.

Remove the processed_extent machinery and just do a single unlock_extent
for the entire bio, using the bbio-wide information like bbio->inode
where applicable.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/extent_io.c | 115 +++++++------------------------------------
 1 file changed, 19 insertions(+), 96 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index a1adadd5d25ddb..8116be675f301b 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -606,75 +606,6 @@ static void end_bio_extent_writepage(struct btrfs_bio *bbio)
 	bio_put(bio);
 }
 
-/*
- * Record previously processed extent range
- *
- * For endio_readpage_release_extent() to handle a full extent range, reducing
- * the extent io operations.
- */
-struct processed_extent {
-	struct btrfs_inode *inode;
-	/* Start of the range in @inode */
-	u64 start;
-	/* End of the range in @inode */
-	u64 end;
-	bool uptodate;
-};
-
-/*
- * Try to release processed extent range
- *
- * May not release the extent range right now if the current range is
- * contiguous to processed extent.
- *
- * Will release processed extent when any of @inode, @uptodate, the range is
- * no longer contiguous to the processed range.
- *
- * Passing @inode == NULL will force processed extent to be released.
- */
-static void endio_readpage_release_extent(struct processed_extent *processed,
-			      struct btrfs_inode *inode, u64 start, u64 end,
-			      bool uptodate)
-{
-	struct extent_state *cached = NULL;
-	struct extent_io_tree *tree;
-
-	/* The first extent, initialize @processed */
-	if (!processed->inode)
-		goto update;
-
-	/*
-	 * Contiguous to processed extent, just uptodate the end.
-	 *
-	 * Several things to notice:
-	 *
-	 * - bio can be merged as long as on-disk bytenr is contiguous
-	 *   This means we can have page belonging to other inodes, thus need to
-	 *   check if the inode still matches.
-	 * - bvec can contain range beyond current page for multi-page bvec
-	 *   Thus we need to do processed->end + 1 >= start check
-	 */
-	if (processed->inode == inode && processed->uptodate == uptodate &&
-	    processed->end + 1 >= start && end >= processed->end) {
-		processed->end = end;
-		return;
-	}
-
-	tree = &processed->inode->io_tree;
-	/*
-	 * Now we don't have range contiguous to the processed range, release
-	 * the processed range now.
-	 */
-	unlock_extent(tree, processed->start, processed->end, &cached);
-
-update:
-	/* Update processed to current range */
-	processed->inode = inode;
-	processed->start = start;
-	processed->end = end;
-	processed->uptodate = uptodate;
-}
-
 static void begin_page_read(struct btrfs_fs_info *fs_info, struct page *page)
 {
 	ASSERT(PageLocked(page));
@@ -727,27 +658,28 @@ static struct extent_buffer *find_extent_buffer_readpage(
  */
 static void end_bio_extent_readpage(struct btrfs_bio *bbio)
 {
+	struct btrfs_fs_info *fs_info = bbio->inode->root->fs_info;
+	const u32 sectorsize = fs_info->sectorsize;
+	struct bvec_iter_all iter_all;
 	struct bio *bio = &bbio->bio;
+	bool uptodate = !bio->bi_status;
+	int mirror = bbio->mirror_num;
 	struct bio_vec *bvec;
-	struct processed_extent processed = { 0 };
-	/*
-	 * The offset to the beginning of a bio, since one bio can never be
-	 * larger than UINT_MAX, u32 here is enough.
-	 */
-	u32 bio_offset = 0;
-	int mirror;
-	struct bvec_iter_all iter_all;
+	u64 bio_start = 0;
+	u32 bio_len = 0;
 
 	ASSERT(!bio_flagged(bio, BIO_CLONED));
 	bio_for_each_segment_all(bvec, bio, iter_all) {
-		bool uptodate = !bio->bi_status;
 		struct page *page = bvec->bv_page;
 		struct inode *inode = page->mapping->host;
-		struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
-		const u32 sectorsize = fs_info->sectorsize;
-		u64 start;
-		u64 end;
-		u32 len;
+		u64 start = page_offset(page) + bvec->bv_offset;
+		u64 end = start + bvec->bv_len - 1;
+		u32 len = bvec->bv_len;
+
+		if (bio_len)
+			ASSERT(start == bio_start + bio_len);
+		else
+			bio_start = start;
 
 		btrfs_debug(fs_info,
 			"end_bio_extent_readpage: bi_sector=%llu, err=%d, mirror=%u",
@@ -771,15 +703,9 @@ static void end_bio_extent_readpage(struct btrfs_bio *bbio)
 		"incomplete page read with offset %u and length %u",
 				   bvec->bv_offset, bvec->bv_len);
 
-		start = page_offset(page) + bvec->bv_offset;
-		end = start + bvec->bv_len - 1;
-		len = bvec->bv_len;
-
-		mirror = bbio->mirror_num;
 		if (uptodate && !is_data_inode(inode) &&
 		    btrfs_validate_metadata_buffer(bbio, page, start, end, mirror))
 			uptodate = false;
-
 		if (likely(uptodate)) {
 			loff_t i_size = i_size_read(inode);
 			pgoff_t end_index = i_size >> PAGE_SHIFT;
@@ -811,15 +737,12 @@ static void end_bio_extent_readpage(struct btrfs_bio *bbio)
 
 		/* Update page status and unlock. */
 		end_page_read(page, uptodate, start, len);
-		endio_readpage_release_extent(&processed, BTRFS_I(inode),
-					      start, end, PageUptodate(page));
-
-		ASSERT(bio_offset + len > bio_offset);
-		bio_offset += len;
+		bio_len += len;
 
 	}
-	/* Release the last extent */
-	endio_readpage_release_extent(&processed, NULL, 0, 0, false);
+
+	unlock_extent(&bbio->inode->io_tree, bio_start,
+		      bio_start + bio_len - 1, NULL);
 	bio_put(bio);
 }
 
-- 
2.39.2

Re: [PATCH] btrfs: remove struct processed_extent

[Christoph Hellwig]

I'll retract this patch for now.  It conflicts with a submitted
series and actually shows a problem with always compression testing
that I need to look into.