[PATCH 0/8] Cleanup io_tree arguments in extent read/write path

[PATCH 0/8] Cleanup io_tree arguments in extent read/write path

[David Sterba]

The io_tree is passed to several functions that do extent page
read/write, but there's a lot of redundancy and things can be
simplified.

David Sterba (8):
  btrfs: remove extent_page_data::tree
  btrfs: drop argument tree from submit_extent_page
  btrfs: add assertions for tree == inode->io_tree to extent IO helpers
  btrfs: drop argument tree from btrfs_lock_and_flush_ordered_range
  btrfs: sink argument tree to extent_read_full_page
  btrfs: sink argument tree to __extent_read_full_page
  btrfs: sink arugment tree to contiguous_readpages
  btrfs: sink argument tree to __do_readpage

 fs/btrfs/disk-io.c      |  4 +---
 fs/btrfs/extent_io.c    | 50 ++++++++++++++++-------------------------
 fs/btrfs/extent_io.h    |  4 ++--
 fs/btrfs/file.c         |  2 +-
 fs/btrfs/inode.c        |  6 ++---
 fs/btrfs/ordered-data.c |  8 +++----
 fs/btrfs/ordered-data.h |  3 +--
 7 files changed, 29 insertions(+), 48 deletions(-)

-- 
2.25.0

[PATCH 1/8] btrfs: remove extent_page_data::tree

[David Sterba]

All functions that set up extent_page_data::tree set it to the inode
io_tree. That's passed down the callstack that accesses either the same
inode or its pages. In the end submit_extent_page can pull the tree out
of the page and we don't have to store it in the structure.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent_io.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index d91a48d73e8f..753fc92ad348 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -122,7 +122,6 @@ struct tree_entry {
 
 struct extent_page_data {
 	struct bio *bio;
-	struct extent_io_tree *tree;
 	/* tells writepage not to lock the state bits for this range
 	 * it still does the unlocking
 	 */
@@ -2967,6 +2966,7 @@ static int submit_extent_page(unsigned int opf, struct extent_io_tree *tree,
 	sector_t sector = offset >> 9;
 
 	ASSERT(bio_ret);
+	ASSERT(tree == &BTRFS_I(page->mapping->host)->io_tree);
 
 	if (*bio_ret) {
 		bool contig;
@@ -3434,7 +3434,7 @@ static noinline_for_stack int __extent_writepage_io(struct inode *inode,
 				 unsigned long nr_written,
 				 int *nr_ret)
 {
-	struct extent_io_tree *tree = epd->tree;
+	struct extent_io_tree *tree = &BTRFS_I(inode)->io_tree;
 	u64 start = page_offset(page);
 	u64 page_end = start + PAGE_SIZE - 1;
 	u64 end;
@@ -3908,11 +3908,9 @@ static noinline_for_stack int write_one_eb(struct extent_buffer *eb,
 int btree_write_cache_pages(struct address_space *mapping,
 				   struct writeback_control *wbc)
 {
-	struct extent_io_tree *tree = &BTRFS_I(mapping->host)->io_tree;
 	struct extent_buffer *eb, *prev_eb = NULL;
 	struct extent_page_data epd = {
 		.bio = NULL,
-		.tree = tree,
 		.extent_locked = 0,
 		.sync_io = wbc->sync_mode == WB_SYNC_ALL,
 	};
@@ -4201,7 +4199,6 @@ int extent_write_full_page(struct page *page, struct writeback_control *wbc)
 	int ret;
 	struct extent_page_data epd = {
 		.bio = NULL,
-		.tree = &BTRFS_I(page->mapping->host)->io_tree,
 		.extent_locked = 0,
 		.sync_io = wbc->sync_mode == WB_SYNC_ALL,
 	};
@@ -4223,14 +4220,12 @@ int extent_write_locked_range(struct inode *inode, u64 start, u64 end,
 {
 	int ret = 0;
 	struct address_space *mapping = inode->i_mapping;
-	struct extent_io_tree *tree = &BTRFS_I(inode)->io_tree;
 	struct page *page;
 	unsigned long nr_pages = (end - start + PAGE_SIZE) >>
 		PAGE_SHIFT;
 
 	struct extent_page_data epd = {
 		.bio = NULL,
-		.tree = tree,
 		.extent_locked = 1,
 		.sync_io = mode == WB_SYNC_ALL,
 	};
@@ -4274,7 +4269,6 @@ int extent_writepages(struct address_space *mapping,
 	int ret = 0;
 	struct extent_page_data epd = {
 		.bio = NULL,
-		.tree = &BTRFS_I(mapping->host)->io_tree,
 		.extent_locked = 0,
 		.sync_io = wbc->sync_mode == WB_SYNC_ALL,
 	};
-- 
2.25.0

Re: [PATCH 1/8] btrfs: remove extent_page_data::tree

[Anand Jain]

On 2/6/20 2:09 AM, David Sterba wrote:
> All functions that set up extent_page_data::tree set it to the inode
> io_tree. That's passed down the callstack that accesses either the same
> inode or its pages. In the end submit_extent_page can pull the tree out
> of the page and we don't have to store it in the structure.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Re: [PATCH 1/8] btrfs: remove extent_page_data::tree

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

[PATCH 2/8] btrfs: drop argument tree from submit_extent_page

[David Sterba]

Now that we're sure the tree from argument is same as the one we can get
from the page's inode io_tree, drop the redundant argument.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent_io.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 753fc92ad348..6640336dd9ba 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2936,7 +2936,6 @@ struct bio *btrfs_bio_clone_partial(struct bio *orig, int offset, int size)
 
 /*
  * @opf:	bio REQ_OP_* and REQ_* flags as one value
- * @tree:	tree so we can call our merge_bio hook
  * @wbc:	optional writeback control for io accounting
  * @page:	page to add to the bio
  * @pg_offset:	offset of the new bio or to check whether we are adding
@@ -2949,7 +2948,7 @@ struct bio *btrfs_bio_clone_partial(struct bio *orig, int offset, int size)
  * @prev_bio_flags:  flags of previous bio to see if we can merge the current one
  * @bio_flags:	flags of the current bio to see if we can merge them
  */
-static int submit_extent_page(unsigned int opf, struct extent_io_tree *tree,
+static int submit_extent_page(unsigned int opf,
 			      struct writeback_control *wbc,
 			      struct page *page, u64 offset,
 			      size_t size, unsigned long pg_offset,
@@ -2964,9 +2963,9 @@ static int submit_extent_page(unsigned int opf, struct extent_io_tree *tree,
 	struct bio *bio;
 	size_t page_size = min_t(size_t, size, PAGE_SIZE);
 	sector_t sector = offset >> 9;
+	struct extent_io_tree *tree = &BTRFS_I(page->mapping->host)->io_tree;
 
 	ASSERT(bio_ret);
-	ASSERT(tree == &BTRFS_I(page->mapping->host)->io_tree);
 
 	if (*bio_ret) {
 		bool contig;
@@ -3253,7 +3252,7 @@ static int __do_readpage(struct extent_io_tree *tree,
 			continue;
 		}
 
-		ret = submit_extent_page(REQ_OP_READ | read_flags, tree, NULL,
+		ret = submit_extent_page(REQ_OP_READ | read_flags, NULL,
 					 page, offset, disk_io_size,
 					 pg_offset, bio,
 					 end_bio_extent_readpage, mirror_num,
@@ -3520,7 +3519,7 @@ static noinline_for_stack int __extent_writepage_io(struct inode *inode,
 			       page->index, cur, end);
 		}
 
-		ret = submit_extent_page(REQ_OP_WRITE | write_flags, tree, wbc,
+		ret = submit_extent_page(REQ_OP_WRITE | write_flags, wbc,
 					 page, offset, iosize, pg_offset,
 					 &epd->bio,
 					 end_bio_extent_writepage,
@@ -3842,7 +3841,6 @@ static noinline_for_stack int write_one_eb(struct extent_buffer *eb,
 			struct extent_page_data *epd)
 {
 	struct btrfs_fs_info *fs_info = eb->fs_info;
-	struct extent_io_tree *tree = &BTRFS_I(fs_info->btree_inode)->io_tree;
 	u64 offset = eb->start;
 	u32 nritems;
 	int i, num_pages;
@@ -3875,7 +3873,7 @@ static noinline_for_stack int write_one_eb(struct extent_buffer *eb,
 
 		clear_page_dirty_for_io(p);
 		set_page_writeback(p);
-		ret = submit_extent_page(REQ_OP_WRITE | write_flags, tree, wbc,
+		ret = submit_extent_page(REQ_OP_WRITE | write_flags, wbc,
 					 p, offset, PAGE_SIZE, 0,
 					 &epd->bio,
 					 end_bio_extent_buffer_writepage,
-- 
2.25.0

Re: [PATCH 2/8] btrfs: drop argument tree from submit_extent_page

[Anand Jain]

On 2/6/20 2:09 AM, David Sterba wrote:
> Now that we're sure the tree from argument is same as the one we can get
> from the page's inode io_tree,


> drop the redundant argument.

  I think there is/was a plan to drop the btree inode? should we need
  this argument if the plan is still on? or any idea if it still can
  be implemented without this argument?

Thanks, Anand

Re: [PATCH 2/8] btrfs: drop argument tree from submit_extent_page

[David Sterba]

On Thu, Feb 06, 2020 at 01:58:22PM +0800, Anand Jain wrote:
> On 2/6/20 2:09 AM, David Sterba wrote:
> > Now that we're sure the tree from argument is same as the one we can get
> > from the page's inode io_tree,
> 
> 
> > drop the redundant argument.
> 
>   I think there is/was a plan to drop the btree inode? should we need
>   this argument if the plan is still on? or any idea if it still can
>   be implemented without this argument?

That's a question for the one implementing the btree inode removal. As
there are several possible ways how to implement it, with different
trade-offs and such, I can't forsee if this particula parameter will be
useful or not. And keeping things around for theoretical needs of future
patches has proven to not work so we've been removing such artifacts.

The exception is of course for patchsets that are in active development
and would have to revert the cleanups.

Re: [PATCH 2/8] btrfs: drop argument tree from submit_extent_page

[Anand Jain]

On 2/6/20 9:47 PM, David Sterba wrote:
> On Thu, Feb 06, 2020 at 01:58:22PM +0800, Anand Jain wrote:
>> On 2/6/20 2:09 AM, David Sterba wrote:
>>> Now that we're sure the tree from argument is same as the one we can get
>>> from the page's inode io_tree,
>>
>>
>>> drop the redundant argument.
>>
>>    I think there is/was a plan to drop the btree inode? should we need
>>    this argument if the plan is still on? or any idea if it still can
>>    be implemented without this argument?
> 
> That's a question for the one implementing the btree inode removal. As
> there are several possible ways how to implement it, with different
> trade-offs and such, I can't forsee if this particula parameter will be
> useful or not. And keeping things around for theoretical needs of future
> patches has proven to not work so we've been removing such artifacts.
> 
> The exception is of course for patchsets that are in active development
> and would have to revert the cleanups.
> 

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Re: [PATCH 2/8] btrfs: drop argument tree from submit_extent_page

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

[PATCH 3/8] btrfs: add assertions for tree == inode->io_tree to extent IO helpers

[David Sterba]

Add assertions to all helpers that get tree as argument and verify that
it's the same that can be obtained from the inode or from its pages. In
followup patches the redundant arguments and assertions will be removed
one by one.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent_io.c    | 10 ++++++++++
 fs/btrfs/ordered-data.c |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 6640336dd9ba..e9d116ecf5a1 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3097,6 +3097,8 @@ static int __do_readpage(struct extent_io_tree *tree,
 	size_t blocksize = inode->i_sb->s_blocksize;
 	unsigned long this_bio_flag = 0;
 
+	ASSERT(tree == &BTRFS_I(inode)->io_tree);
+
 	set_page_extent_mapped(page);
 
 	if (!PageUptodate(page)) {
@@ -3290,6 +3292,8 @@ static inline void contiguous_readpages(struct extent_io_tree *tree,
 	struct btrfs_inode *inode = BTRFS_I(pages[0]->mapping->host);
 	int index;
 
+	ASSERT(tree == &inode->io_tree);
+
 	btrfs_lock_and_flush_ordered_range(tree, inode, start, end, NULL);
 
 	for (index = 0; index < nr_pages; index++) {
@@ -3311,6 +3315,8 @@ static int __extent_read_full_page(struct extent_io_tree *tree,
 	u64 end = start + PAGE_SIZE - 1;
 	int ret;
 
+	ASSERT(tree == &inode->io_tree);
+
 	btrfs_lock_and_flush_ordered_range(tree, inode, start, end, NULL);
 
 	ret = __do_readpage(tree, page, get_extent, NULL, bio, mirror_num,
@@ -3325,6 +3331,8 @@ int extent_read_full_page(struct extent_io_tree *tree, struct page *page,
 	unsigned long bio_flags = 0;
 	int ret;
 
+	ASSERT(tree == &BTRFS_I(page->mapping->host)->io_tree);
+
 	ret = __extent_read_full_page(tree, page, get_extent, &bio, mirror_num,
 				      &bio_flags, 0);
 	if (bio)
@@ -5413,6 +5421,8 @@ int read_extent_buffer_pages(struct extent_buffer *eb, int wait, int mirror_num)
 	if (test_bit(EXTENT_BUFFER_UPTODATE, &eb->bflags))
 		return 0;
 
+	ASSERT(tree == &BTRFS_I(eb->pages[0]->mapping->host)->io_tree);
+
 	num_pages = num_extent_pages(eb);
 	for (i = 0; i < num_pages; i++) {
 		page = eb->pages[i];
diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index 45048e5f76b0..ad471a2fba93 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -849,6 +849,8 @@ void btrfs_lock_and_flush_ordered_range(struct extent_io_tree *tree,
 	struct extent_state *cache = NULL;
 	struct extent_state **cachedp = &cache;
 
+	ASSERT(tree == &inode->io_tree);
+
 	if (cached_state)
 		cachedp = cached_state;
 
-- 
2.25.0

Re: [PATCH 3/8] btrfs: add assertions for tree == inode->io_tree to extent IO helpers

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

[PATCH 4/8] btrfs: drop argument tree from btrfs_lock_and_flush_ordered_range

[David Sterba]

The tree pointer can be safely read from the inode so we can drop the
redundant argument from btrfs_lock_and_flush_ordered_range.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent_io.c    |  4 ++--
 fs/btrfs/file.c         |  2 +-
 fs/btrfs/inode.c        |  2 +-
 fs/btrfs/ordered-data.c | 10 +++-------
 fs/btrfs/ordered-data.h |  3 +--
 5 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index e9d116ecf5a1..a0a80a151085 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3294,7 +3294,7 @@ static inline void contiguous_readpages(struct extent_io_tree *tree,
 
 	ASSERT(tree == &inode->io_tree);
 
-	btrfs_lock_and_flush_ordered_range(tree, inode, start, end, NULL);
+	btrfs_lock_and_flush_ordered_range(inode, start, end, NULL);
 
 	for (index = 0; index < nr_pages; index++) {
 		__do_readpage(tree, pages[index], btrfs_get_extent, em_cached,
@@ -3317,7 +3317,7 @@ static int __extent_read_full_page(struct extent_io_tree *tree,
 
 	ASSERT(tree == &inode->io_tree);
 
-	btrfs_lock_and_flush_ordered_range(tree, inode, start, end, NULL);
+	btrfs_lock_and_flush_ordered_range(inode, start, end, NULL);
 
 	ret = __do_readpage(tree, page, get_extent, NULL, bio, mirror_num,
 			    bio_flags, read_flags, NULL);
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 746d569d234e..b03651ea1896 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1560,7 +1560,7 @@ static noinline int check_can_nocow(struct btrfs_inode *inode, loff_t pos,
 	lockend = round_up(pos + *write_bytes,
 			   fs_info->sectorsize) - 1;
 
-	btrfs_lock_and_flush_ordered_range(&inode->io_tree, inode, lockstart,
+	btrfs_lock_and_flush_ordered_range(inode, lockstart,
 					   lockend, NULL);
 
 	num_bytes = lockend - lockstart + 1;
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 5c897d8c9506..29b06c109137 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -4611,7 +4611,7 @@ int btrfs_cont_expand(struct inode *inode, loff_t oldsize, loff_t size)
 	if (size <= hole_start)
 		return 0;
 
-	btrfs_lock_and_flush_ordered_range(io_tree, BTRFS_I(inode), hole_start,
+	btrfs_lock_and_flush_ordered_range(BTRFS_I(inode), hole_start,
 					   block_end - 1, &cached_state);
 	cur_offset = hole_start;
 	while (1) {
diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index ad471a2fba93..d3f2f274e28d 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -830,7 +830,6 @@ int btrfs_find_ordered_sum(struct inode *inode, u64 offset, u64 disk_bytenr,
  * btrfs_flush_ordered_range - Lock the passed range and ensures all pending
  * ordered extents in it are run to completion.
  *
- * @tree:         IO tree used for locking out other users of the range
  * @inode:        Inode whose ordered tree is to be searched
  * @start:        Beginning of range to flush
  * @end:          Last byte of range to lock
@@ -840,8 +839,7 @@ int btrfs_find_ordered_sum(struct inode *inode, u64 offset, u64 disk_bytenr,
  * This function always returns with the given range locked, ensuring after it's
  * called no order extent can be pending.
  */
-void btrfs_lock_and_flush_ordered_range(struct extent_io_tree *tree,
-					struct btrfs_inode *inode, u64 start,
+void btrfs_lock_and_flush_ordered_range(struct btrfs_inode *inode, u64 start,
 					u64 end,
 					struct extent_state **cached_state)
 {
@@ -849,13 +847,11 @@ void btrfs_lock_and_flush_ordered_range(struct extent_io_tree *tree,
 	struct extent_state *cache = NULL;
 	struct extent_state **cachedp = &cache;
 
-	ASSERT(tree == &inode->io_tree);
-
 	if (cached_state)
 		cachedp = cached_state;
 
 	while (1) {
-		lock_extent_bits(tree, start, end, cachedp);
+		lock_extent_bits(&inode->io_tree, start, end, cachedp);
 		ordered = btrfs_lookup_ordered_range(inode, start,
 						     end - start + 1);
 		if (!ordered) {
@@ -868,7 +864,7 @@ void btrfs_lock_and_flush_ordered_range(struct extent_io_tree *tree,
 				refcount_dec(&cache->refs);
 			break;
 		}
-		unlock_extent_cached(tree, start, end, cachedp);
+		unlock_extent_cached(&inode->io_tree, start, end, cachedp);
 		btrfs_start_ordered_extent(&inode->vfs_inode, ordered, 1);
 		btrfs_put_ordered_extent(ordered);
 	}
diff --git a/fs/btrfs/ordered-data.h b/fs/btrfs/ordered-data.h
index a46f319d9ae0..c01c9698250b 100644
--- a/fs/btrfs/ordered-data.h
+++ b/fs/btrfs/ordered-data.h
@@ -183,8 +183,7 @@ u64 btrfs_wait_ordered_extents(struct btrfs_root *root, u64 nr,
 			       const u64 range_start, const u64 range_len);
 void btrfs_wait_ordered_roots(struct btrfs_fs_info *fs_info, u64 nr,
 			      const u64 range_start, const u64 range_len);
-void btrfs_lock_and_flush_ordered_range(struct extent_io_tree *tree,
-					struct btrfs_inode *inode, u64 start,
+void btrfs_lock_and_flush_ordered_range(struct btrfs_inode *inode, u64 start,
 					u64 end,
 					struct extent_state **cached_state);
 int __init ordered_data_init(void);
-- 
2.25.0

Re: [PATCH 4/8] btrfs: drop argument tree from btrfs_lock_and_flush_ordered_range

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

[PATCH 5/8] btrfs: sink argument tree to extent_read_full_page

[David Sterba]

The tree pointer can be safely read from the page's inode, use it and
drop the redundant argument.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/disk-io.c   | 4 +---
 fs/btrfs/extent_io.c | 7 +++----
 fs/btrfs/extent_io.h | 4 ++--
 fs/btrfs/inode.c     | 4 +---
 4 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 28622de9e642..a1e5b9511993 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -972,9 +972,7 @@ static int btree_writepages(struct address_space *mapping,
 
 static int btree_readpage(struct file *file, struct page *page)
 {
-	struct extent_io_tree *tree;
-	tree = &BTRFS_I(page->mapping->host)->io_tree;
-	return extent_read_full_page(tree, page, btree_get_extent, 0);
+	return extent_read_full_page(page, btree_get_extent, 0);
 }
 
 static int btree_releasepage(struct page *page, gfp_t gfp_flags)
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index a0a80a151085..97e613e72ed4 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3324,15 +3324,14 @@ static int __extent_read_full_page(struct extent_io_tree *tree,
 	return ret;
 }
 
-int extent_read_full_page(struct extent_io_tree *tree, struct page *page,
-			    get_extent_t *get_extent, int mirror_num)
+int extent_read_full_page(struct page *page, get_extent_t *get_extent,
+			  int mirror_num)
 {
 	struct bio *bio = NULL;
 	unsigned long bio_flags = 0;
+	struct extent_io_tree *tree = &BTRFS_I(page->mapping->host)->io_tree;
 	int ret;
 
-	ASSERT(tree == &BTRFS_I(page->mapping->host)->io_tree);
-
 	ret = __extent_read_full_page(tree, page, get_extent, &bio, mirror_num,
 				      &bio_flags, 0);
 	if (bio)
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index 5d205bbaafdc..234622101230 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -189,8 +189,8 @@ typedef struct extent_map *(get_extent_t)(struct btrfs_inode *inode,
 int try_release_extent_mapping(struct page *page, gfp_t mask);
 int try_release_extent_buffer(struct page *page);
 
-int extent_read_full_page(struct extent_io_tree *tree, struct page *page,
-			  get_extent_t *get_extent, int mirror_num);
+int extent_read_full_page(struct page *page, get_extent_t *get_extent,
+			  int mirror_num);
 int extent_write_full_page(struct page *page, struct writeback_control *wbc);
 int extent_write_locked_range(struct inode *inode, u64 start, u64 end,
 			      int mode);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 29b06c109137..64e3c62cc0df 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8256,9 +8256,7 @@ static int btrfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 
 int btrfs_readpage(struct file *file, struct page *page)
 {
-	struct extent_io_tree *tree;
-	tree = &BTRFS_I(page->mapping->host)->io_tree;
-	return extent_read_full_page(tree, page, btrfs_get_extent, 0);
+	return extent_read_full_page(page, btrfs_get_extent, 0);
 }
 
 static int btrfs_writepage(struct page *page, struct writeback_control *wbc)
-- 
2.25.0

Re: [PATCH 5/8] btrfs: sink argument tree to extent_read_full_page

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

[PATCH 6/8] btrfs: sink argument tree to __extent_read_full_page

[David Sterba]

The tree pointer can be safely read from the inode, use it and drop the
redundant argument.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent_io.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 97e613e72ed4..015993ccf333 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3303,8 +3303,7 @@ static inline void contiguous_readpages(struct extent_io_tree *tree,
 	}
 }
 
-static int __extent_read_full_page(struct extent_io_tree *tree,
-				   struct page *page,
+static int __extent_read_full_page(struct page *page,
 				   get_extent_t *get_extent,
 				   struct bio **bio, int mirror_num,
 				   unsigned long *bio_flags,
@@ -3313,10 +3312,9 @@ static int __extent_read_full_page(struct extent_io_tree *tree,
 	struct btrfs_inode *inode = BTRFS_I(page->mapping->host);
 	u64 start = page_offset(page);
 	u64 end = start + PAGE_SIZE - 1;
+	struct extent_io_tree *tree = &inode->io_tree;
 	int ret;
 
-	ASSERT(tree == &inode->io_tree);
-
 	btrfs_lock_and_flush_ordered_range(inode, start, end, NULL);
 
 	ret = __do_readpage(tree, page, get_extent, NULL, bio, mirror_num,
@@ -3329,10 +3327,9 @@ int extent_read_full_page(struct page *page, get_extent_t *get_extent,
 {
 	struct bio *bio = NULL;
 	unsigned long bio_flags = 0;
-	struct extent_io_tree *tree = &BTRFS_I(page->mapping->host)->io_tree;
 	int ret;
 
-	ret = __extent_read_full_page(tree, page, get_extent, &bio, mirror_num,
+	ret = __extent_read_full_page(page, get_extent, &bio, mirror_num,
 				      &bio_flags, 0);
 	if (bio)
 		ret = submit_one_bio(bio, mirror_num, bio_flags);
@@ -5415,13 +5412,10 @@ int read_extent_buffer_pages(struct extent_buffer *eb, int wait, int mirror_num)
 	unsigned long num_reads = 0;
 	struct bio *bio = NULL;
 	unsigned long bio_flags = 0;
-	struct extent_io_tree *tree = &BTRFS_I(eb->fs_info->btree_inode)->io_tree;
 
 	if (test_bit(EXTENT_BUFFER_UPTODATE, &eb->bflags))
 		return 0;
 
-	ASSERT(tree == &BTRFS_I(eb->pages[0]->mapping->host)->io_tree);
-
 	num_pages = num_extent_pages(eb);
 	for (i = 0; i < num_pages; i++) {
 		page = eb->pages[i];
@@ -5465,7 +5459,7 @@ int read_extent_buffer_pages(struct extent_buffer *eb, int wait, int mirror_num)
 			}
 
 			ClearPageError(page);
-			err = __extent_read_full_page(tree, page,
+			err = __extent_read_full_page(page,
 						      btree_get_extent, &bio,
 						      mirror_num, &bio_flags,
 						      REQ_META);
-- 
2.25.0

Re: [PATCH 6/8] btrfs: sink argument tree to __extent_read_full_page

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

[PATCH 7/8] btrfs: sink arugment tree to contiguous_readpages

[David Sterba]

The tree pointer can be safely read from the inode, use it and drop the
redundant argument.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent_io.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 015993ccf333..157a0cf0b492 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3281,8 +3281,7 @@ static int __do_readpage(struct extent_io_tree *tree,
 	return ret;
 }
 
-static inline void contiguous_readpages(struct extent_io_tree *tree,
-					     struct page *pages[], int nr_pages,
+static inline void contiguous_readpages(struct page *pages[], int nr_pages,
 					     u64 start, u64 end,
 					     struct extent_map **em_cached,
 					     struct bio **bio,
@@ -3290,10 +3289,9 @@ static inline void contiguous_readpages(struct extent_io_tree *tree,
 					     u64 *prev_em_start)
 {
 	struct btrfs_inode *inode = BTRFS_I(pages[0]->mapping->host);
+	struct extent_io_tree *tree = &inode->io_tree;
 	int index;
 
-	ASSERT(tree == &inode->io_tree);
-
 	btrfs_lock_and_flush_ordered_range(inode, start, end, NULL);
 
 	for (index = 0; index < nr_pages; index++) {
@@ -4292,7 +4290,6 @@ int extent_readpages(struct address_space *mapping, struct list_head *pages,
 	unsigned long bio_flags = 0;
 	struct page *pagepool[16];
 	struct extent_map *em_cached = NULL;
-	struct extent_io_tree *tree = &BTRFS_I(mapping->host)->io_tree;
 	int nr = 0;
 	u64 prev_em_start = (u64)-1;
 
@@ -4319,7 +4316,7 @@ int extent_readpages(struct address_space *mapping, struct list_head *pages,
 
 			ASSERT(contig_start + nr * PAGE_SIZE - 1 == contig_end);
 
-			contiguous_readpages(tree, pagepool, nr, contig_start,
+			contiguous_readpages(pagepool, nr, contig_start,
 				     contig_end, &em_cached, &bio, &bio_flags,
 				     &prev_em_start);
 		}
-- 
2.25.0

Re: [PATCH 7/8] btrfs: sink arugment tree to contiguous_readpages

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

[PATCH 8/8] btrfs: sink argument tree to __do_readpage

[David Sterba]

The tree pointer can be safely read from the inode, use it and drop the
redundant argument.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent_io.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 157a0cf0b492..46f0e52bce8f 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3072,8 +3072,7 @@ __get_extent_map(struct inode *inode, struct page *page, size_t pg_offset,
  * XXX JDM: This needs looking at to ensure proper page locking
  * return 0 on success, otherwise return error
  */
-static int __do_readpage(struct extent_io_tree *tree,
-			 struct page *page,
+static int __do_readpage(struct page *page,
 			 get_extent_t *get_extent,
 			 struct extent_map **em_cached,
 			 struct bio **bio, int mirror_num,
@@ -3096,8 +3095,7 @@ static int __do_readpage(struct extent_io_tree *tree,
 	size_t disk_io_size;
 	size_t blocksize = inode->i_sb->s_blocksize;
 	unsigned long this_bio_flag = 0;
-
-	ASSERT(tree == &BTRFS_I(inode)->io_tree);
+	struct extent_io_tree *tree = &BTRFS_I(inode)->io_tree;
 
 	set_page_extent_mapped(page);
 
@@ -3289,13 +3287,12 @@ static inline void contiguous_readpages(struct page *pages[], int nr_pages,
 					     u64 *prev_em_start)
 {
 	struct btrfs_inode *inode = BTRFS_I(pages[0]->mapping->host);
-	struct extent_io_tree *tree = &inode->io_tree;
 	int index;
 
 	btrfs_lock_and_flush_ordered_range(inode, start, end, NULL);
 
 	for (index = 0; index < nr_pages; index++) {
-		__do_readpage(tree, pages[index], btrfs_get_extent, em_cached,
+		__do_readpage(pages[index], btrfs_get_extent, em_cached,
 				bio, 0, bio_flags, REQ_RAHEAD, prev_em_start);
 		put_page(pages[index]);
 	}
@@ -3310,12 +3307,11 @@ static int __extent_read_full_page(struct page *page,
 	struct btrfs_inode *inode = BTRFS_I(page->mapping->host);
 	u64 start = page_offset(page);
 	u64 end = start + PAGE_SIZE - 1;
-	struct extent_io_tree *tree = &inode->io_tree;
 	int ret;
 
 	btrfs_lock_and_flush_ordered_range(inode, start, end, NULL);
 
-	ret = __do_readpage(tree, page, get_extent, NULL, bio, mirror_num,
+	ret = __do_readpage(page, get_extent, NULL, bio, mirror_num,
 			    bio_flags, read_flags, NULL);
 	return ret;
 }
-- 
2.25.0

Re: [PATCH 8/8] btrfs: sink argument tree to __do_readpage

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 0/8] Cleanup io_tree arguments in extent read/write path

[Nikolay Borisov]

On 5.02.20 г. 20:09 ч., David Sterba wrote:
> The io_tree is passed to several functions that do extent page
> read/write, but there's a lot of redundancy and things can be
> simplified.
> 
> David Sterba (8):
>   btrfs: remove extent_page_data::tree
>   btrfs: drop argument tree from submit_extent_page
>   btrfs: add assertions for tree == inode->io_tree to extent IO helpers
>   btrfs: drop argument tree from btrfs_lock_and_flush_ordered_range
>   btrfs: sink argument tree to extent_read_full_page
>   btrfs: sink argument tree to __extent_read_full_page
>   btrfs: sink arugment tree to contiguous_readpages
>   btrfs: sink argument tree to __do_readpage
> 
>  fs/btrfs/disk-io.c      |  4 +---
>  fs/btrfs/extent_io.c    | 50 ++++++++++++++++-------------------------
>  fs/btrfs/extent_io.h    |  4 ++--
>  fs/btrfs/file.c         |  2 +-
>  fs/btrfs/inode.c        |  6 ++---
>  fs/btrfs/ordered-data.c |  8 +++----
>  fs/btrfs/ordered-data.h |  3 +--
>  7 files changed, 29 insertions(+), 48 deletions(-)
> 


Reviewed-by: Nikolay Borisov <nborisov@suse.com>