* [PATCH] btrfs: Convert btrfs_read_merkle_tree_page() to use a folio
@ 2023-08-14 17:52 Matthew Wilcox (Oracle)
2023-08-14 19:06 ` Boris Burkov
2023-09-06 18:03 ` David Sterba
0 siblings, 2 replies; 3+ messages in thread
From: Matthew Wilcox (Oracle) @ 2023-08-14 17:52 UTC (permalink / raw)
To: Chris Mason, Josef Bacik, David Sterba
Cc: Matthew Wilcox (Oracle), linux-btrfs
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(-)
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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] btrfs: Convert btrfs_read_merkle_tree_page() to use a folio
2023-08-14 17:52 [PATCH] btrfs: Convert btrfs_read_merkle_tree_page() to use a folio Matthew Wilcox (Oracle)
@ 2023-08-14 19:06 ` Boris Burkov
2023-09-06 18:03 ` David Sterba
1 sibling, 0 replies; 3+ messages in thread
From: Boris Burkov @ 2023-08-14 19:06 UTC (permalink / raw)
To: Matthew Wilcox (Oracle)
Cc: Chris Mason, Josef Bacik, David Sterba, linux-btrfs
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
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] btrfs: Convert btrfs_read_merkle_tree_page() to use a folio
2023-08-14 17:52 [PATCH] btrfs: Convert btrfs_read_merkle_tree_page() to use a folio Matthew Wilcox (Oracle)
2023-08-14 19:06 ` Boris Burkov
@ 2023-09-06 18:03 ` David Sterba
1 sibling, 0 replies; 3+ messages in thread
From: David Sterba @ 2023-09-06 18:03 UTC (permalink / raw)
To: Matthew Wilcox (Oracle)
Cc: Chris Mason, Josef Bacik, David Sterba, linux-btrfs
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.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-09-06 18:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-14 17:52 [PATCH] btrfs: Convert btrfs_read_merkle_tree_page() to use a folio Matthew Wilcox (Oracle)
2023-08-14 19:06 ` Boris Burkov
2023-09-06 18:03 ` David Sterba
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox