| Message ID | 20230309090526.332550-13-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/20] btrfs: mark extent_buffer_under_io static | expand |
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Looking at the resulting code, I hope one of the subsequent
patches is merging write_one_subpage_eb() and write_one_eb()
as they're getting quite similar.
On Thu, Mar 09, 2023 at 02:00:32PM +0000, Johannes Thumshirn wrote: > Looks good, > Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> > > Looking at the resulting code, I hope one of the subsequent > patches is merging write_one_subpage_eb() and write_one_eb() > as they're getting quite similar. The final patch will do that.
On 2023/3/9 17:05, Christoph Hellwig wrote: > The btrfs_bio_ctrl machinery is overkill for writing extent_buffers > as we always operate on PAGE SIZE chunks (or one smaller one for the > subpage case) that are contigous and are guaranteed to fit into a > single bio. Replace it with open coded btrfs_bio_alloc, __bio_add_page > and btrfs_submit_bio calls. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Qu Wenruo <wqu@suse.com> Does this also mean, the only benefit of manually merging continuous pages into a larger bio, compared to multiple smaller adjacent bios, is just memory saving? Thanks, Qu > --- > fs/btrfs/extent_io.c | 40 ++++++++++++++++++++-------------------- > 1 file changed, 20 insertions(+), 20 deletions(-) > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index 1fc50d1402cef8..1d7f48190ee9b9 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -121,9 +121,6 @@ static void submit_one_bio(struct btrfs_bio_ctrl *bio_ctrl) > /* Caller should ensure the bio has at least some range added */ > ASSERT(bbio->bio.bi_iter.bi_size); > > - if (!is_data_inode(&bbio->inode->vfs_inode)) > - bbio->bio.bi_opf |= REQ_META; > - > if (btrfs_op(&bbio->bio) == BTRFS_MAP_READ && > bio_ctrl->compress_type != BTRFS_COMPRESS_NONE) > btrfs_submit_compressed_read(bbio); > @@ -1897,11 +1894,7 @@ static void write_one_subpage_eb(struct extent_buffer *eb, > struct btrfs_fs_info *fs_info = eb->fs_info; > struct page *page = eb->pages[0]; > bool no_dirty_ebs = false; > - struct btrfs_bio_ctrl bio_ctrl = { > - .wbc = wbc, > - .opf = REQ_OP_WRITE | wbc_to_write_flags(wbc), > - .end_io_func = end_bio_subpage_eb_writepage, > - }; > + struct btrfs_bio *bbio; > > prepare_eb_write(eb); > > @@ -1915,10 +1908,16 @@ static void write_one_subpage_eb(struct extent_buffer *eb, > if (no_dirty_ebs) > clear_page_dirty_for_io(page); > > - submit_extent_page(&bio_ctrl, eb->start, page, eb->len, > - eb->start - page_offset(page)); > + bbio = btrfs_bio_alloc(INLINE_EXTENT_BUFFER_PAGES, > + REQ_OP_WRITE | REQ_META | wbc_to_write_flags(wbc), > + BTRFS_I(eb->fs_info->btree_inode), > + end_bio_subpage_eb_writepage, NULL); > + bbio->bio.bi_iter.bi_sector = eb->start >> SECTOR_SHIFT; > + bbio->file_offset = eb->start; > + __bio_add_page(&bbio->bio, page, eb->len, eb->start - page_offset(page)); > unlock_page(page); > - submit_one_bio(&bio_ctrl); > + btrfs_submit_bio(bbio, 0); > + > /* > * Submission finished without problem, if no range of the page is > * dirty anymore, we have submitted a page. Update nr_written in wbc. > @@ -1930,16 +1929,18 @@ static void write_one_subpage_eb(struct extent_buffer *eb, > static noinline_for_stack void write_one_eb(struct extent_buffer *eb, > struct writeback_control *wbc) > { > - u64 disk_bytenr = eb->start; > + struct btrfs_bio *bbio; > int i, num_pages; > - struct btrfs_bio_ctrl bio_ctrl = { > - .wbc = wbc, > - .opf = REQ_OP_WRITE | wbc_to_write_flags(wbc), > - .end_io_func = end_bio_extent_buffer_writepage, > - }; > > prepare_eb_write(eb); > > + bbio = btrfs_bio_alloc(INLINE_EXTENT_BUFFER_PAGES, > + REQ_OP_WRITE | REQ_META | wbc_to_write_flags(wbc), > + BTRFS_I(eb->fs_info->btree_inode), > + end_bio_extent_buffer_writepage, NULL); > + bbio->bio.bi_iter.bi_sector = eb->start >> SECTOR_SHIFT; > + bbio->file_offset = eb->start; > + > num_pages = num_extent_pages(eb); > for (i = 0; i < num_pages; i++) { > struct page *p = eb->pages[i]; > @@ -1947,12 +1948,11 @@ static noinline_for_stack void write_one_eb(struct extent_buffer *eb, > lock_page(p); > clear_page_dirty_for_io(p); > set_page_writeback(p); > - submit_extent_page(&bio_ctrl, disk_bytenr, p, PAGE_SIZE, 0); > - disk_bytenr += PAGE_SIZE; > + __bio_add_page(&bbio->bio, p, PAGE_SIZE, 0); > wbc->nr_to_write--; > unlock_page(p); > } > - submit_one_bio(&bio_ctrl); > + btrfs_submit_bio(bbio, 0); > } > > /*
On Fri, Mar 10, 2023 at 04:34:10PM +0800, Qu Wenruo wrote: > Does this also mean, the only benefit of manually merging continuous pages > into a larger bio, compared to multiple smaller adjacent bios, is just > memory saving? That is the only significant saving. There also is a very minimal amount of CPU cycles saved as well.
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index 1fc50d1402cef8..1d7f48190ee9b9 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -121,9 +121,6 @@ static void submit_one_bio(struct btrfs_bio_ctrl *bio_ctrl) /* Caller should ensure the bio has at least some range added */ ASSERT(bbio->bio.bi_iter.bi_size); - if (!is_data_inode(&bbio->inode->vfs_inode)) - bbio->bio.bi_opf |= REQ_META; - if (btrfs_op(&bbio->bio) == BTRFS_MAP_READ && bio_ctrl->compress_type != BTRFS_COMPRESS_NONE) btrfs_submit_compressed_read(bbio); @@ -1897,11 +1894,7 @@ static void write_one_subpage_eb(struct extent_buffer *eb, struct btrfs_fs_info *fs_info = eb->fs_info; struct page *page = eb->pages[0]; bool no_dirty_ebs = false; - struct btrfs_bio_ctrl bio_ctrl = { - .wbc = wbc, - .opf = REQ_OP_WRITE | wbc_to_write_flags(wbc), - .end_io_func = end_bio_subpage_eb_writepage, - }; + struct btrfs_bio *bbio; prepare_eb_write(eb); @@ -1915,10 +1908,16 @@ static void write_one_subpage_eb(struct extent_buffer *eb, if (no_dirty_ebs) clear_page_dirty_for_io(page); - submit_extent_page(&bio_ctrl, eb->start, page, eb->len, - eb->start - page_offset(page)); + bbio = btrfs_bio_alloc(INLINE_EXTENT_BUFFER_PAGES, + REQ_OP_WRITE | REQ_META | wbc_to_write_flags(wbc), + BTRFS_I(eb->fs_info->btree_inode), + end_bio_subpage_eb_writepage, NULL); + bbio->bio.bi_iter.bi_sector = eb->start >> SECTOR_SHIFT; + bbio->file_offset = eb->start; + __bio_add_page(&bbio->bio, page, eb->len, eb->start - page_offset(page)); unlock_page(page); - submit_one_bio(&bio_ctrl); + btrfs_submit_bio(bbio, 0); + /* * Submission finished without problem, if no range of the page is * dirty anymore, we have submitted a page. Update nr_written in wbc. @@ -1930,16 +1929,18 @@ static void write_one_subpage_eb(struct extent_buffer *eb, static noinline_for_stack void write_one_eb(struct extent_buffer *eb, struct writeback_control *wbc) { - u64 disk_bytenr = eb->start; + struct btrfs_bio *bbio; int i, num_pages; - struct btrfs_bio_ctrl bio_ctrl = { - .wbc = wbc, - .opf = REQ_OP_WRITE | wbc_to_write_flags(wbc), - .end_io_func = end_bio_extent_buffer_writepage, - }; prepare_eb_write(eb); + bbio = btrfs_bio_alloc(INLINE_EXTENT_BUFFER_PAGES, + REQ_OP_WRITE | REQ_META | wbc_to_write_flags(wbc), + BTRFS_I(eb->fs_info->btree_inode), + end_bio_extent_buffer_writepage, NULL); + bbio->bio.bi_iter.bi_sector = eb->start >> SECTOR_SHIFT; + bbio->file_offset = eb->start; + num_pages = num_extent_pages(eb); for (i = 0; i < num_pages; i++) { struct page *p = eb->pages[i]; @@ -1947,12 +1948,11 @@ static noinline_for_stack void write_one_eb(struct extent_buffer *eb, lock_page(p); clear_page_dirty_for_io(p); set_page_writeback(p); - submit_extent_page(&bio_ctrl, disk_bytenr, p, PAGE_SIZE, 0); - disk_bytenr += PAGE_SIZE; + __bio_add_page(&bbio->bio, p, PAGE_SIZE, 0); wbc->nr_to_write--; unlock_page(p); } - submit_one_bio(&bio_ctrl); + btrfs_submit_bio(bbio, 0); } /*
The btrfs_bio_ctrl machinery is overkill for writing extent_buffers as we always operate on PAGE SIZE chunks (or one smaller one for the subpage case) that are contigous and are guaranteed to fit into a single bio. Replace it with open coded btrfs_bio_alloc, __bio_add_page and btrfs_submit_bio calls. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/btrfs/extent_io.c | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-)