From: Chi Zhiling When reading small amounts of data from the page cache, only a single folio is typically returned from filemap_read_get_batch(). In this case, calling xas_advance() or xas_next() after adding the folio to the batch is unnecessary and only introduces extra branches. The same issue exists for large reads, where one additional xarray walk is always performed before termination. Quit the loop once we get the last folio in the range, so the final redundant xarray advancement can be avoided. The xas_next() does not update xa_index when xas->xa_node is set to XAS_RESTART, so the put and retry path would not update xa_index, hence the warning should therefore never trigger. During the 4k reads test, the overhead of this function dropped from 2.91% to 2.53%. Signed-off-by: Chi Zhiling --- mm/filemap.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/mm/filemap.c b/mm/filemap.c index 4e636647100c..d54450e529bd 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -2458,12 +2458,16 @@ static void filemap_get_read_batch(struct address_space *mapping, { XA_STATE(xas, &mapping->i_pages, index); struct folio *folio; + pgoff_t next; + + if (unlikely(index > max)) + return; rcu_read_lock(); for (folio = xas_load(&xas); folio; folio = xas_next(&xas)) { if (xas_retry(&xas, folio)) continue; - if (xas.xa_index > max || xa_is_value(folio)) + if (xa_is_value(folio) || WARN_ON(xas.xa_index > max)) break; if (xa_is_sibling(folio)) break; @@ -2479,7 +2483,11 @@ static void filemap_get_read_batch(struct address_space *mapping, break; if (folio_test_readahead(folio)) break; - xas_advance(&xas, folio_next_index(folio) - 1); + + next = folio_next_index(folio); + if (next > max) + break; + xas_advance(&xas, next - 1); continue; put_folio: folio_put(folio); -- 2.43.0 From: Chi Zhiling Apply the same optimization used in filemap_get_read_batch() by moving the boundary check from the loop condition to before xas_advance(), avoiding an unnecessary xarray lookup and reducing branches in the fast path. Signed-off-by: Chi Zhiling --- mm/filemap.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/mm/filemap.c b/mm/filemap.c index d54450e529bd..6bc717f205a7 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -2258,11 +2258,13 @@ unsigned filemap_get_folios_contig(struct address_space *mapping, XA_STATE(xas, &mapping->i_pages, *start); unsigned long nr; struct folio *folio; + pgoff_t next; - rcu_read_lock(); + if (unlikely(*start > end)) + return 0; - for (folio = xas_load(&xas); folio && xas.xa_index <= end; - folio = xas_next(&xas)) { + rcu_read_lock(); + for (folio = xas_load(&xas); folio; folio = xas_next(&xas)) { if (xas_retry(&xas, folio)) continue; /* @@ -2270,11 +2272,11 @@ unsigned filemap_get_folios_contig(struct address_space *mapping, * No current caller is looking for DAX entries. */ if (xa_is_value(folio)) - goto update_start; + break; /* If we landed in the middle of a THP, continue at its end. */ if (xa_is_sibling(folio)) - goto update_start; + break; if (!folio_try_get(folio)) goto retry; @@ -2282,30 +2284,28 @@ unsigned filemap_get_folios_contig(struct address_space *mapping, if (unlikely(folio != xas_reload(&xas))) goto put_folio; - if (!folio_batch_add(fbatch, folio)) { - nr = folio_nr_pages(folio); - *start = folio->index + nr; - goto out; - } - xas_advance(&xas, folio_next_index(folio) - 1); + if (!folio_batch_add(fbatch, folio)) + break; + + next = folio_next_index(folio); + if (next > end) + break; + xas_advance(&xas, next - 1); continue; + put_folio: folio_put(folio); - retry: xas_reset(&xas); } + rcu_read_unlock(); -update_start: nr = folio_batch_count(fbatch); - if (nr) { folio = fbatch->folios[nr - 1]; *start = folio_next_index(folio); } -out: - rcu_read_unlock(); - return folio_batch_count(fbatch); + return nr; } EXPORT_SYMBOL(filemap_get_folios_contig); -- 2.43.0 From: Chi Zhiling This is a prep patch for shmem folio batching in the read path, where non-uptodate folios need to be handled in the main iteration loop. A large non-uptodate folio should be treated as a hole. Currently, holes larger than PAGE_SIZE cannot be handled because ZERO_PAGE is limited to a single page. Add copy_zero_to_iter() as a wrapper to support copying larger zero ranges to the iterator. Signed-off-by: Chi Zhiling --- mm/shmem.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/mm/shmem.c b/mm/shmem.c index 3b5dc21b323c..b08118fbd275 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -3339,6 +3339,20 @@ shmem_write_end(const struct kiocb *iocb, struct address_space *mapping, return copied; } +static size_t copy_zero_to_iter(size_t bytes, struct iov_iter *i) +{ + struct folio *zero = largest_zero_folio(); + unsigned long nr, ret, written = 0; + + do { + nr = min(bytes - written, folio_size(zero)); + ret = copy_folio_to_iter(zero, 0, nr, i); + written += ret; + } while (written < bytes && ret == nr); + + return written; +} + static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to) { struct file *file = iocb->ki_filp; @@ -3433,7 +3447,7 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to) * clear_user() not so much, that it is noticeably * faster to copy the zero page instead of clearing. */ - ret = copy_page_to_iter(ZERO_PAGE(0), offset, nr, to); + ret = copy_zero_to_iter(nr, to); } else { /* * But submitting the same page twice in a row to -- 2.43.0 From: Chi Zhiling This is perp patch for folio batch support which removing the fallback to page-copy mode from shmem reads. The existing hwpoison fallback would require fetching the same folio multiple times to read its each pages, which complicates the batching model. Instead, move hwpoison handling into copy_each_pages_to_iter(), allowing the error path to be centralized. This simplifies the shmem fast read path and avoids special-case handling that conflicts with folio batching support. Signed-off-by: Chi Zhiling --- mm/shmem.c | 63 +++++++++++++++++++++++++++++------------------------- 1 file changed, 34 insertions(+), 29 deletions(-) diff --git a/mm/shmem.c b/mm/shmem.c index b08118fbd275..cac355685e49 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -3353,6 +3353,30 @@ static size_t copy_zero_to_iter(size_t bytes, struct iov_iter *i) return written; } +static size_t copy_pages_to_iter(struct folio *folio, size_t offset, + size_t bytes, struct iov_iter *i, int *error) +{ + struct page *page; + unsigned long off, nr, ret, written = 0; + + do { + page = folio_page(folio, offset >> PAGE_SHIFT); + if (PageHWPoison(page)) { + *error = -EIO; + break; + } + + off = offset_in_page(offset); + nr = min(PAGE_SIZE - off, bytes - written); + + ret = copy_page_to_iter(page, off, nr, i); + offset += ret; + written += ret; + } while (written < bytes && ret == nr); + + return written; +} + static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to) { struct file *file = iocb->ki_filp; @@ -3365,10 +3389,8 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to) for (;;) { struct folio *folio = NULL; - struct page *page = NULL; unsigned long nr, ret; loff_t end_offset, i_size = i_size_read(inode); - bool fallback_page_copy = false; size_t fsize; if (unlikely(iocb->ki_pos >= i_size)) @@ -3376,25 +3398,13 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to) index = iocb->ki_pos >> PAGE_SHIFT; error = shmem_get_folio(inode, index, 0, &folio, SGP_READ); + if (folio) + folio_unlock(folio); if (error) { if (error == -EINVAL) error = 0; break; } - if (folio) { - folio_unlock(folio); - - page = folio_file_page(folio, index); - if (PageHWPoison(page)) { - folio_put(folio); - error = -EIO; - break; - } - - if (folio_test_large(folio) && - folio_test_has_hwpoisoned(folio)) - fallback_page_copy = true; - } /* * We must evaluate after, since reads (unlike writes) @@ -3406,12 +3416,9 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to) folio_put(folio); break; } - end_offset = min_t(loff_t, i_size, iocb->ki_pos + to->count); - if (folio && likely(!fallback_page_copy)) - fsize = folio_size(folio); - else - fsize = PAGE_SIZE; + fsize = folio ? folio_size(folio) : PAGE_SIZE; offset = iocb->ki_pos & (fsize - 1); + end_offset = min_t(loff_t, i_size, iocb->ki_pos + to->count); nr = min_t(loff_t, end_offset - iocb->ki_pos, fsize - offset); if (folio) { @@ -3420,12 +3427,8 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to) * virtual addresses, take care about potential aliasing * before reading the page on the kernel side. */ - if (mapping_writably_mapped(mapping)) { - if (likely(!fallback_page_copy)) - flush_dcache_folio(folio); - else - flush_dcache_page(page); - } + if (mapping_writably_mapped(mapping)) + flush_dcache_folio(folio); /* * Mark the folio accessed if we read the beginning. @@ -3436,10 +3439,10 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to) * Ok, we have the page, and it's up-to-date, so * now we can copy it to user space... */ - if (likely(!fallback_page_copy)) + if (likely(!folio_contain_hwpoisoned_page(folio))) ret = copy_folio_to_iter(folio, offset, nr, to); else - ret = copy_page_to_iter(page, offset, nr, to); + ret = copy_pages_to_iter(folio, offset, nr, to, &error); folio_put(folio); } else if (user_backed_iter(to)) { /* @@ -3462,6 +3465,8 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to) if (!iov_iter_count(to)) break; + if (unlikely(error)) + break; if (ret < nr) { error = -EFAULT; break; -- 2.43.0 From: Chi Zhiling 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. This patch checks the uptodate flag without holding the folio lock, so it may observe a non-uptodate state on a locked folio that is still being initialized. This is safe because only zero-filled data can be copied to the user buffer in that scenario. A non-uptodate folio in the swap cache cannot be added to the shmem page cache. This creates a semantic conflict, as shmem zeroes the folio out, but the swap cache would fill it by reading from the swap backing store. Signed-off-by: Chi Zhiling --- mm/shmem.c | 57 ++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 42 insertions(+), 15 deletions(-) diff --git a/mm/shmem.c b/mm/shmem.c index cac355685e49..61937582f08c 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -891,6 +891,14 @@ int shmem_add_to_page_cache(struct folio *folio, VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio); VM_BUG_ON_FOLIO(!folio_test_swapbacked(folio), folio); + /* + * Don't add a non-uptodate folio that is in swap cache to page + * cache, since shmem will zero it instead of reading from swap + * backing. + */ + VM_BUG_ON_FOLIO(folio_test_swapcache(folio) && + !folio_test_uptodate(folio), folio); + folio_ref_add(folio, nr); folio->mapping = mapping; folio->index = index; @@ -3382,11 +3390,13 @@ 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; - pgoff_t index; + struct folio_batch fbatch; unsigned long offset; int error = 0; ssize_t retval = 0; + folio_batch_init(&fbatch); + for (;;) { struct folio *folio = NULL; unsigned long nr, ret; @@ -3395,15 +3405,33 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to) if (unlikely(iocb->ki_pos >= i_size)) break; +fetch: + folio = folio_batch_next(&fbatch); + if (!folio) { + pgoff_t start = iocb->ki_pos >> PAGE_SHIFT; + 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); + } - index = iocb->ki_pos >> PAGE_SHIFT; - error = shmem_get_folio(inode, index, 0, &folio, SGP_READ); - if (folio) - folio_unlock(folio); - if (error) { - if (error == -EINVAL) - error = 0; - break; + filemap_get_folios_contig(inode->i_mapping, &start, end, &fbatch); + if (folio_batch_count(&fbatch)) + goto fetch; + + error = shmem_get_folio(inode, start, 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++; + } } /* @@ -3411,17 +3439,15 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to) * are called without i_rwsem protection against truncate */ i_size = i_size_read(inode); - if (unlikely(iocb->ki_pos >= i_size)) { - if (folio) - folio_put(folio); + if (unlikely(iocb->ki_pos >= i_size)) break; - } + fsize = folio ? folio_size(folio) : PAGE_SIZE; offset = iocb->ki_pos & (fsize - 1); end_offset = min_t(loff_t, i_size, iocb->ki_pos + to->count); nr = min_t(loff_t, end_offset - iocb->ki_pos, fsize - offset); - if (folio) { + if (folio && folio_test_uptodate(folio)) { /* * If users can be writing to this page using arbitrary * virtual addresses, take care about potential aliasing @@ -3443,7 +3469,6 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to) ret = copy_folio_to_iter(folio, offset, nr, to); else ret = copy_pages_to_iter(folio, offset, nr, to, &error); - folio_put(folio); } else if (user_backed_iter(to)) { /* * Copy to user tends to be so well optimized, but @@ -3474,6 +3499,8 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to) cond_resched(); } + for (int i = 0; i < folio_batch_count(&fbatch); i++) + folio_put(fbatch.folios[i]); file_accessed(file); return retval ? retval : error; } -- 2.43.0