| Message ID | f1232d0ffc8ae7389206d4cc9210afc77cfcacac.1723096922.git.wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: reduce extent map lookup overhead for data write | expand |
On Thu, Aug 8, 2024 at 7:06 AM Qu Wenruo <wqu@suse.com> wrote: > > Unlike data read, we never really utilize the cached extent_map for data > write. > > This means we will do the costly extent map lookup for each sector we > need to write back. > > Change it to utilize the same function, __get_extent_map(), just like > the data read path, to reduce the overhead of extent map lookup. > > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > fs/btrfs/extent_io.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index d4ad98488c03..9492cd9d4f04 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -1357,7 +1357,8 @@ static int submit_one_sector(struct btrfs_inode *inode, > /* @filepos >= i_size case should be handled by the caller. */ > ASSERT(filepos < i_size); > > - em = btrfs_get_extent(inode, NULL, filepos, sectorsize); > + em = __get_extent_map(&inode->vfs_inode, NULL, filepos, sectorsize, > + &bio_ctrl->em_cached); > if (IS_ERR(em)) > return PTR_ERR_OR_ZERO(em); > > @@ -2320,6 +2321,7 @@ void extent_write_locked_range(struct inode *inode, const struct folio *locked_f > cur = cur_end + 1; > } > > + free_extent_map(bio_ctrl.em_cached); > submit_write_bio(&bio_ctrl, found_error ? ret : 0); > } > > @@ -2338,6 +2340,7 @@ int btrfs_writepages(struct address_space *mapping, struct writeback_control *wb > */ > btrfs_zoned_data_reloc_lock(BTRFS_I(inode)); > ret = extent_write_cache_pages(mapping, &bio_ctrl); > + free_extent_map(bio_ctrl.em_cached); > submit_write_bio(&bio_ctrl, ret); Same comment as in the first patch, instead of duplicating free_extent_map() calls before submit_write_bio() calls, just move the free_extent_map() to inside of submit_write_bio() and then also set ->em_cached to NULL after the free. This helps avoid use-after-free issues and someone forgetting a free call in the future. Thanks. > btrfs_zoned_data_reloc_unlock(BTRFS_I(inode)); > return ret; > -- > 2.45.2 > >
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index d4ad98488c03..9492cd9d4f04 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -1357,7 +1357,8 @@ static int submit_one_sector(struct btrfs_inode *inode, /* @filepos >= i_size case should be handled by the caller. */ ASSERT(filepos < i_size); - em = btrfs_get_extent(inode, NULL, filepos, sectorsize); + em = __get_extent_map(&inode->vfs_inode, NULL, filepos, sectorsize, + &bio_ctrl->em_cached); if (IS_ERR(em)) return PTR_ERR_OR_ZERO(em); @@ -2320,6 +2321,7 @@ void extent_write_locked_range(struct inode *inode, const struct folio *locked_f cur = cur_end + 1; } + free_extent_map(bio_ctrl.em_cached); submit_write_bio(&bio_ctrl, found_error ? ret : 0); } @@ -2338,6 +2340,7 @@ int btrfs_writepages(struct address_space *mapping, struct writeback_control *wb */ btrfs_zoned_data_reloc_lock(BTRFS_I(inode)); ret = extent_write_cache_pages(mapping, &bio_ctrl); + free_extent_map(bio_ctrl.em_cached); submit_write_bio(&bio_ctrl, ret); btrfs_zoned_data_reloc_unlock(BTRFS_I(inode)); return ret;
Unlike data read, we never really utilize the cached extent_map for data write. This means we will do the costly extent map lookup for each sector we need to write back. Change it to utilize the same function, __get_extent_map(), just like the data read path, to reduce the overhead of extent map lookup. Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/extent_io.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)