| Message ID | 20230814175208.810785-1-willy@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: Convert btrfs_read_merkle_tree_page() to use a folio | expand |
On Mon, Aug 14, 2023 at 06:52:08PM +0100, Matthew Wilcox (Oracle) wrote: > Remove a number of hidden calls to compound_head() by using a folio > throughout. Also follow core kernel code style by adding the folio to > the page cache immediately after allocation instead of doing the read > first, then adding it to the page cache. This ordering makes subsequent > readers block waiting for the first reader instead of duplicating the > work only to throw it away when they find out they lost the race. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> The btrfs fs-verity fstests had bitrotted (some boring issues with btrfs-progs) that I cleaned up locally and was able to test your change. Code LGTM as well, thanks for the update. Reviewed-by: Boris Burkov <boris@bur.io> > --- > fs/btrfs/verity.c | 62 +++++++++++++++++++++++------------------------ > 1 file changed, 31 insertions(+), 31 deletions(-) > > diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c > index c5ff16f9e9fa..04d85db9bb58 100644 > --- a/fs/btrfs/verity.c > +++ b/fs/btrfs/verity.c > @@ -715,7 +715,7 @@ static struct page *btrfs_read_merkle_tree_page(struct inode *inode, > pgoff_t index, > unsigned long num_ra_pages) > { > - struct page *page; > + struct folio *folio; > u64 off = (u64)index << PAGE_SHIFT; > loff_t merkle_pos = merkle_file_pos(inode); > int ret; > @@ -726,29 +726,38 @@ static struct page *btrfs_read_merkle_tree_page(struct inode *inode, > return ERR_PTR(-EFBIG); > index += merkle_pos >> PAGE_SHIFT; > again: > - page = find_get_page_flags(inode->i_mapping, index, FGP_ACCESSED); > - if (page) { > - if (PageUptodate(page)) > - return page; > + folio = __filemap_get_folio(inode->i_mapping, index, FGP_ACCESSED, 0); > + if (!IS_ERR(folio)) { > + if (folio_test_uptodate(folio)) > + goto out; > > - lock_page(page); > + folio_lock(folio); > /* > - * We only insert uptodate pages, so !Uptodate has to be > - * an error > + * If it's not uptodate after we have the lock, we got a > + * read error. > */ > - if (!PageUptodate(page)) { > - unlock_page(page); > - put_page(page); > + if (!folio_test_uptodate(folio)) { > + folio_unlock(folio); > + folio_put(folio); > return ERR_PTR(-EIO); > } > - unlock_page(page); > - return page; > + folio_unlock(folio); > + goto out; > } > > - page = __page_cache_alloc(mapping_gfp_constraint(inode->i_mapping, ~__GFP_FS)); > - if (!page) > + folio = filemap_alloc_folio(mapping_gfp_constraint(inode->i_mapping, ~__GFP_FS), 0); > + if (!folio) > return ERR_PTR(-ENOMEM); > > + ret = filemap_add_folio(inode->i_mapping, folio, index, GFP_NOFS); > + if (ret) { > + folio_put(folio); > + /* Did someone else insert a folio here? */ > + if (ret == -EEXIST) > + goto again; > + return ERR_PTR(ret); > + } > + > /* > * Merkle item keys are indexed from byte 0 in the merkle tree. > * They have the form: > @@ -756,28 +765,19 @@ static struct page *btrfs_read_merkle_tree_page(struct inode *inode, > * [ inode objectid, BTRFS_MERKLE_ITEM_KEY, offset in bytes ] > */ > ret = read_key_bytes(BTRFS_I(inode), BTRFS_VERITY_MERKLE_ITEM_KEY, off, > - page_address(page), PAGE_SIZE, page); > + folio_address(folio), PAGE_SIZE, &folio->page); > if (ret < 0) { > - put_page(page); > + folio_put(folio); > return ERR_PTR(ret); > } > if (ret < PAGE_SIZE) > - memzero_page(page, ret, PAGE_SIZE - ret); > + folio_zero_segment(folio, ret, PAGE_SIZE); > > - SetPageUptodate(page); > - ret = add_to_page_cache_lru(page, inode->i_mapping, index, GFP_NOFS); > + folio_mark_uptodate(folio); > + folio_unlock(folio); > > - if (!ret) { > - /* Inserted and ready for fsverity */ > - unlock_page(page); > - } else { > - put_page(page); > - /* Did someone race us into inserting this page? */ > - if (ret == -EEXIST) > - goto again; > - page = ERR_PTR(ret); > - } > - return page; > +out: > + return folio_file_page(folio, index); > } > > /* > -- > 2.40.1 >
On Mon, Aug 14, 2023 at 06:52:08PM +0100, Matthew Wilcox (Oracle) wrote: > Remove a number of hidden calls to compound_head() by using a folio > throughout. Also follow core kernel code style by adding the folio to > the page cache immediately after allocation instead of doing the read > first, then adding it to the page cache. This ordering makes subsequent > readers block waiting for the first reader instead of duplicating the > work only to throw it away when they find out they lost the race. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> Added to btrfs tree, thanks.
diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c index c5ff16f9e9fa..04d85db9bb58 100644 --- a/fs/btrfs/verity.c +++ b/fs/btrfs/verity.c @@ -715,7 +715,7 @@ static struct page *btrfs_read_merkle_tree_page(struct inode *inode, pgoff_t index, unsigned long num_ra_pages) { - struct page *page; + struct folio *folio; u64 off = (u64)index << PAGE_SHIFT; loff_t merkle_pos = merkle_file_pos(inode); int ret; @@ -726,29 +726,38 @@ static struct page *btrfs_read_merkle_tree_page(struct inode *inode, return ERR_PTR(-EFBIG); index += merkle_pos >> PAGE_SHIFT; again: - page = find_get_page_flags(inode->i_mapping, index, FGP_ACCESSED); - if (page) { - if (PageUptodate(page)) - return page; + folio = __filemap_get_folio(inode->i_mapping, index, FGP_ACCESSED, 0); + if (!IS_ERR(folio)) { + if (folio_test_uptodate(folio)) + goto out; - lock_page(page); + folio_lock(folio); /* - * We only insert uptodate pages, so !Uptodate has to be - * an error + * If it's not uptodate after we have the lock, we got a + * read error. */ - if (!PageUptodate(page)) { - unlock_page(page); - put_page(page); + if (!folio_test_uptodate(folio)) { + folio_unlock(folio); + folio_put(folio); return ERR_PTR(-EIO); } - unlock_page(page); - return page; + folio_unlock(folio); + goto out; } - page = __page_cache_alloc(mapping_gfp_constraint(inode->i_mapping, ~__GFP_FS)); - if (!page) + folio = filemap_alloc_folio(mapping_gfp_constraint(inode->i_mapping, ~__GFP_FS), 0); + if (!folio) return ERR_PTR(-ENOMEM); + ret = filemap_add_folio(inode->i_mapping, folio, index, GFP_NOFS); + if (ret) { + folio_put(folio); + /* Did someone else insert a folio here? */ + if (ret == -EEXIST) + goto again; + return ERR_PTR(ret); + } + /* * Merkle item keys are indexed from byte 0 in the merkle tree. * They have the form: @@ -756,28 +765,19 @@ static struct page *btrfs_read_merkle_tree_page(struct inode *inode, * [ inode objectid, BTRFS_MERKLE_ITEM_KEY, offset in bytes ] */ ret = read_key_bytes(BTRFS_I(inode), BTRFS_VERITY_MERKLE_ITEM_KEY, off, - page_address(page), PAGE_SIZE, page); + folio_address(folio), PAGE_SIZE, &folio->page); if (ret < 0) { - put_page(page); + folio_put(folio); return ERR_PTR(ret); } if (ret < PAGE_SIZE) - memzero_page(page, ret, PAGE_SIZE - ret); + folio_zero_segment(folio, ret, PAGE_SIZE); - SetPageUptodate(page); - ret = add_to_page_cache_lru(page, inode->i_mapping, index, GFP_NOFS); + folio_mark_uptodate(folio); + folio_unlock(folio); - if (!ret) { - /* Inserted and ready for fsverity */ - unlock_page(page); - } else { - put_page(page); - /* Did someone race us into inserting this page? */ - if (ret == -EEXIST) - goto again; - page = ERR_PTR(ret); - } - return page; +out: + return folio_file_page(folio, index); } /*
Remove a number of hidden calls to compound_head() by using a folio throughout. Also follow core kernel code style by adding the folio to the page cache immediately after allocation instead of doing the read first, then adding it to the page cache. This ordering makes subsequent readers block waiting for the first reader instead of duplicating the work only to throw it away when they find out they lost the race. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/btrfs/verity.c | 62 +++++++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 31 deletions(-)