Re: [Bug 216570] New: Pass wrong index value to DEFINE_READAHEAD which leads to almost no readahead of fsverity data

[Andrew Morton <akpm@linux-foundation.org>]

(switched to email.  Please respond via emailed reply-to-all, not via the
bugzilla web interface).

On Tue, 11 Oct 2022 06:52:49 +0000 bugzilla-daemon@kernel.org wrote:

> https://bugzilla.kernel.org/show_bug.cgi?id=216570
> 
>             Bug ID: 216570
>            Summary: Pass wrong index value to DEFINE_READAHEAD which leads
>                     to almost no readahead of fsverity data
>            Product: Memory Management
>            Version: 2.5
>     Kernel Version: 6.0
>           Hardware: All
>                 OS: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: low
>           Priority: P1
>          Component: Other
>           Assignee: akpm@linux-foundation.org
>           Reporter: nicememory@gmail.com
>         Regression: No
> 
> It was introduced by commit 73bb49da50cd460bb3ba31250ed2e7fbf2115edf
> 
> In function ext4_read_merkle_tree_page and f2fs_read_merkle_tree_page,
> parameter index was added by a offset and then passed to
> page_cache_readahead_unbounded.
> For now, index is passed to DEFINE_READAHEAD directly with wrong value.

oops, yes, thanks.  Matthew, could you please take a look?

> 
> Possible fix is not changing the value of parameter index and defining a new
> local variable ra_index to replace all the other reference to index. Like:
> 
> static struct page *ext4_read_merkle_tree_page(struct inode *inode,
>                                                pgoff_t index,
>                                                unsigned long num_ra_pages)
> {
>         pgoff_t ra_index = index + ext4_verity_metadata_pos(inode) >>
> PAGE_SHIFT;
>         DEFINE_READAHEAD(ractl, NULL, NULL, inode->i_mapping, ra_index);
>         struct page *page;
> 
>         page = find_get_page_flags(inode->i_mapping, ra_index, FGP_ACCESSED);
>         if (!page || !PageUptodate(page)) {
>                 if (page)
>                         put_page(page);
>                 else if (num_ra_pages > 1)
>                         page_cache_ra_unbounded(&ractl, num_ra_pages, 0);
>                 page = read_mapping_page(inode->i_mapping, ra_index, NULL);
>         }
>         return page;
> }
> 
> 
> static struct page *f2fs_read_merkle_tree_page(struct inode *inode,
>                                                pgoff_t index,
>                                                unsigned long num_ra_pages)
> {
>         pgoff_t ra_index = index + f2fs_verity_metadata_pos(inode) >>
> PAGE_SHIFT;
>         DEFINE_READAHEAD(ractl, NULL, NULL, inode->i_mapping, ra_index);
>         struct page *page;
> 
>         page = find_get_page_flags(inode->i_mapping, ra_index, FGP_ACCESSED);
>         if (!page || !PageUptodate(page)) {
>                 if (page)
>                         put_page(page);
>                 else if (num_ra_pages > 1)
>                         page_cache_ra_unbounded(&ractl, num_ra_pages, 0);
>                 page = read_mapping_page(inode->i_mapping, ra_index, NULL);
>         }
>         return page;
> }
>