* Re: [Bug 216570] New: Pass wrong index value to DEFINE_READAHEAD which leads to almost no readahead of fsverity data
[not found] <bug-216570-27@https.bugzilla.kernel.org/>
@ 2022-10-12 0:21 ` Andrew Morton
2022-10-12 1:32 ` Matthew Wilcox
0 siblings, 1 reply; 2+ messages in thread
From: Andrew Morton @ 2022-10-12 0:21 UTC (permalink / raw)
To: nicememory; +Cc: bugzilla-daemon, Matthew Wilcox, linux-mm
(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;
> }
>
^ permalink raw reply [flat|nested] 2+ messages in thread