| Message ID | a54487df38bbbd4f6149b2a7bbe86247fe5c532e.1580925977.git.dsterba@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Cleanup io_tree arguments in extent read/write path | expand |
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
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
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.
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>
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,
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(-)