On Mon, Apr 17, 2023 at 11:50:42PM -0700, Eric Biggers wrote:
> Hi Matthew,
>
> On Fri, Mar 24, 2023 at 06:01:29PM +0000, Matthew Wilcox (Oracle) wrote:
> > This is an implementation of fsverity_operations read_merkle_tree_page,
> > so it must still return the precise page asked for, but we can use the
> > folio API to reduce the number of conversions between folios & pages.
> >
> > Signed-off-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
> > ---
> > fs/ext4/verity.c | 14 +++++++-------
> > 1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/fs/ext4/verity.c b/fs/ext4/verity.c
> > index afe847c967a4..3b01247066dd 100644
> > --- a/fs/ext4/verity.c
> > +++ b/fs/ext4/verity.c
> > @@ -361,21 +361,21 @@ static struct page *ext4_read_merkle_tree_page(struct inode *inode,
> > pgoff_t index,
> > unsigned long num_ra_pages)
> > {
> > - struct page *page;
> > + struct folio *folio;
> >
> > index += ext4_verity_metadata_pos(inode) >> PAGE_SHIFT;
> >
> > - page = find_get_page_flags(inode->i_mapping, index, FGP_ACCESSED);
> > - if (!page || !PageUptodate(page)) {
> > + folio = __filemap_get_folio(inode->i_mapping, index, FGP_ACCESSED, 0);
> > + if (!folio || !folio_test_uptodate(folio)) {
> > DEFINE_READAHEAD(ractl, NULL, NULL, inode->i_mapping, index);
> >
> > - if (page)
> > - put_page(page);
> > + if (folio)
> > + folio_put(folio);
> > else if (num_ra_pages > 1)
> > page_cache_ra_unbounded(&ractl, num_ra_pages, 0);
> > - page = read_mapping_page(inode->i_mapping, index, NULL);
> > + folio = read_mapping_folio(inode->i_mapping, index, NULL);
> > }
> > - return page;
> > + return folio_file_page(folio, index);
>
> This is not working at all, since it dereferences ERR_PTR(-ENOENT). I think it
> needs:
Argh. Christoph changed the return value of __filemap_get_folio().
> folio = __filemap_get_folio(inode->i_mapping, index, FGP_ACCESSED, 0);
> - if (!folio || !folio_test_uptodate(folio)) {
> + if (folio == ERR_PTR(-ENOENT) || !folio_test_uptodate(folio)) {
This should be "if (IS_ERR(folio) || !folio_test_uptodate(folio)) {"
But we can't carry this change in Ted's tree because it doesn't have
Christoph's change. And we can't carry it in Andrew's tree because it
doesn't have my ext4 change.
> DEFINE_READAHEAD(ractl, NULL, NULL, inode->i_mapping, index);
>
> - if (folio)
> + if (!IS_ERR(folio))
> folio_put(folio);
> else if (num_ra_pages > 1)
> page_cache_ra_unbounded(&ractl, num_ra_pages, 0);
> folio = read_mapping_folio(inode->i_mapping, index, NULL);
> }
> + if (IS_ERR(folio))
> + return ERR_CAST(folio);
return &folio->page;
> return folio_file_page(folio, index);
> }
>
