Re: [PATCH v1 5/5] mm/shmem: optimize file read with folio batching
From: Chi Zhiling
Date: Mon May 25 2026 - 05:49:29 EST
On 5/25/26 4:37 PM, Baolin Wang wrote:
On 5/20/26 6:15 PM, Chi Zhiling wrote:
From: Chi Zhiling <chizhiling@xxxxxxxxxx>
Optimize shmem file read by using filemap_get_folios_contig() to
batch fetch contiguous folios from the page cache, reducing the
overhead of repeated shmem_get_folio() calls.
When the folio batch is exhausted, attempt to refill it with
filemap_get_folios_contig(). If no folios are found (hole or swapped
out pages), fall back to shmem_get_folio() to handle these cases
individually.
Additionally:
- Defer folio_put() until the batch is exhausted or on exit
- Add folio_test_uptodate() check before copying to ensure data
validity
Signed-off-by: Chi Zhiling <chizhiling@xxxxxxxxxx>
---
mm/shmem.c | 50 ++++++++++++++++++++++++++++++++++++--------------
1 file changed, 36 insertions(+), 14 deletions(-)
diff --git a/mm/shmem.c b/mm/shmem.c
index 96ea5a4c0aff..e0eacc23cccd 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3355,11 +3355,14 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
struct file *file = iocb->ki_filp;
struct inode *inode = file_inode(file);
struct address_space *mapping = inode->i_mapping;
+ struct folio_batch fbatch;
pgoff_t index;
unsigned long offset;
int error = 0;
ssize_t retval = 0;
+ folio_batch_init(&fbatch);
+
for (;;) {
struct folio *folio = NULL;
struct page *page = NULL;
@@ -3372,18 +3375,38 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
break;
index = iocb->ki_pos >> PAGE_SHIFT;
- error = shmem_get_folio(inode, index, 0, &folio, SGP_READ);
- if (error) {
- if (error == -EINVAL)
- error = 0;
- break;
+fetch:
+ folio = folio_batch_next(&fbatch);
+ if (!folio) {
+ pgoff_t start = index;
+ pgoff_t end = (iocb->ki_pos + to->count - 1) >> PAGE_SHIFT;
+
+ if (folio_batch_count(&fbatch)) {
+ for (int i = 0; i < folio_batch_count(&fbatch); i++)
+ folio_put(fbatch.folios[i]);
+ folio_batch_reinit(&fbatch);
+ }
+
+ filemap_get_folios_contig(inode->i_mapping, &start, end, &fbatch);
+ if (folio_batch_count(&fbatch))
+ goto fetch;
+
+ error = shmem_get_folio(inode, index, 0, &folio, SGP_READ);
+ if (unlikely(error)) {
+ if (error == -EINVAL)
+ error = 0;
+ break;
+ }
+ if (folio) {
+ folio_unlock(folio);
+ folio_batch_add(&fbatch, folio);
+ fbatch.i++;
+ }
}
- if (folio) {
- folio_unlock(folio);
+ if (folio) {
page = folio_file_page(folio, index);
if (PageHWPoison(page)) {
- folio_put(folio);
error = -EIO;
break;
}
I haven't tested it yet, but I'm sure this will break the fallback_page_copy mode.
You're fetching from the batch at folio granularity here, but in fallback_page_copy mode, we still copy the data at page granularity.
Yes, that's a bug, Sashika has already reported it. I’ll fix it in the next version.
Sashika also reported another potential issue below, but I think that is a misunderstanding, because a folio fetched from the swap cache is already marked Uptodate before being added to the page cache. What do you think?
~~~
Sashika:
Can this lockless check cause readers to receive spurious zeros instead
of the actual data?
Since filemap_get_folios_contig() fetches locklessly, a folio that is
currently being allocated and initialized, or swapped in by another
thread, might not yet be marked uptodate.
By checking folio_test_uptodate() without holding the folio lock, this falls
through to the zero-fill paths if the folio isn't uptodate yet. The original
code avoided this by using shmem_get_folio() which waits for the folio lock
and I/O completion.
~~~
link: https://sashiko.dev/#/patchset/20260520101538.58745-1-chizhiling@xxxxxxx
Thanks,