From: Vishal Moola <vishal.moola@gmail.com>
To: Tal Zussman <tz2294@columbia.edu>
Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org>,
Jan Kara <jack@suse.cz>,
Andrew Morton <akpm@linux-foundation.org>,
linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm/filemap: fix page_cache_prev_miss() when no hole is found
Date: Mon, 11 May 2026 13:44:17 +0100 [thread overview]
Message-ID: <agHPIcZHHyzBjcGN@fedora> (raw)
In-Reply-To: <20260510-prev_miss_fix-v1-1-755bb123145a@columbia.edu>
On Sun, May 10, 2026 at 05:54:17PM -0400, Tal Zussman wrote:
> page_cache_prev_miss() is documented to return a value outside the
> searched range when no gap is found. However, the no-gap-found path
> returns xas.xa_index, which after a successful loop is the first index
> in the range. As such, that index is misreported as a gap.
>
> The sole caller, page_cache_sync_ra(), uses the return value to estimate
> the cached run preceding a sequential read. In some cases, the buggy
> return value can undercount the contiguous range by one, shrinking the
> readahead window or pushing borderline requests into the
> small-random-read branch.
>
> Mirror the fix in commit bbcaee20e03e ("readahead: fix return value of
> page_cache_next_miss() when no hole is found"): preserve max_scan in a
> separate variable across the loop and return `index - max_scan` from the
> no-gap-found path.
IMO, this way of fixing it hurts the readability. I'd prefer something
similar to the fix in the original commit. Or...
> - while (max_scan--) {
> + while (nr--) {
> void *entry = xas_prev(&xas);
> if (!entry || xa_is_value(entry))
> - break;
> + return xas.xa_index;
> if (xas.xa_index == ULONG_MAX)
> - break;
> + return ULONG_MAX;
> }
If I understand this correctly, couldn't we just do something like:
if (!max_scan)
return xas.xa_index - 1;
> - return xas.xa_index;
> + return index - max_scan;
> }
> EXPORT_SYMBOL(page_cache_prev_miss);
>
>
> ---
> base-commit: e9dd96806dbc2d50a66770b6a86962bd5d601153
> change-id: 20260510-prev_miss_fix-fcb308472131
>
> Best regards,
> --
> Tal Zussman <tz2294@columbia.edu>
>
next prev parent reply other threads:[~2026-05-11 12:44 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-10 21:54 [PATCH] mm/filemap: fix page_cache_prev_miss() when no hole is found Tal Zussman
2026-05-11 12:44 ` Vishal Moola [this message]
2026-05-11 16:26 ` Jan Kara
2026-05-11 18:15 ` Tal Zussman
2026-05-12 7:38 ` Jan Kara
2026-05-12 8:02 ` Vishal Moola
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=agHPIcZHHyzBjcGN@fedora \
--to=vishal.moola@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=jack@suse.cz \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=tz2294@columbia.edu \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.