Re: [PATCH 00/48] Folios for 5.17
From: William Kucharski <hidden>
Date: 2021-12-26 22:26:43
Also in:
linux-fsdevel
Consolidated multiple review comments into one email, the majority are nits at
best:
[PATCH 04/48]:
An obnoxiously pendantic English grammar nit:
+ * lock). This can also be called from mark_buffer_dirty(), which I
The period should be inside the paren, e.g.: "lock.)"
[PATCH 05/48]:
+ unsigned char aux[3];
I'd like to see an explanation of why this is "3."
+static inline void folio_batch_init(struct folio_batch *fbatch)
+{
+ fbatch->nr = 0;
+}
+
+static inline unsigned int folio_batch_count(struct folio_batch *fbatch)
+{
+ return fbatch->nr;
+}
+
+static inline unsigned int fbatch_space(struct folio_batch *fbatch)
+{
+ return PAGEVEC_SIZE - fbatch->nr;
+}
+
+/**
+ * folio_batch_add() - Add a folio to a batch.
+ * @fbatch: The folio batch.
+ * @folio: The folio to add.
+ *
+ * The folio is added to the end of the batch.
+ * The batch must have previously been initialised using folio_batch_init().
+ *
+ * Return: The number of slots still available.
+ */
+static inline unsigned folio_batch_add(struct folio_batch *fbatch,
+ struct folio *folio)
+{
+ fbatch->folios[fbatch->nr++] = folio;
Is there any need to validate fbatch in these inlines?
[PATCH 07/48]:
+ xas_for_each(&xas, folio, ULONG_MAX) { \
unsigned left; \
- if (xas_retry(&xas, head)) \
+ size_t offset = offset_in_folio(folio, start + __off); \
+ if (xas_retry(&xas, folio)) \
continue; \
- if (WARN_ON(xa_is_value(head))) \
+ if (WARN_ON(xa_is_value(folio))) \
break; \
- if (WARN_ON(PageHuge(head))) \
+ if (WARN_ON(folio_test_hugetlb(folio))) \
break; \
- for (j = (head->index < index) ? index - head->index : 0; \
- j < thp_nr_pages(head); j++) { \
- void *kaddr = kmap_local_page(head + j); \
- base = kaddr + offset; \
- len = PAGE_SIZE - offset; \
+ while (offset < folio_size(folio)) { \
Since offset is not actually used until after a bunch of error conditions
may exit or restart the loop, and isn't used at all in between, defer
its calculation until just before its first use in the "while."
[PATCH 25/48]:
Whether you use your follow-on patch sent to Christop or defer it until later
as mentioned in your followup email, either approach is fine with me.
Otherwise it all looks good.
For the series:
Reviewed-by: William Kucharski <redacted>
On Dec 7, 2021, at 9:22 PM, Matthew Wilcox (Oracle) [off-list ref] wrote: I was trying to get all the way to adding large folios to the page cache, but I ran out of time. I think the changes in this batch of patches are worth adding, even without large folio support for filesystems other than tmpfs. The big change here is the last patch which switches from storing large folios in the page cache as 2^N identical entries to using the XArray support for multi-index entries. As the changelog says, this fixes a bug that can occur when doing (eg) msync() or sync_file_range(). Some parts of this have been sent before. The first patch is already in Andrew's tree, but I included it here so the build bots don't freak out about not being able to apply this patch series. The folio_batch (replacement for pagevec) is new. I also fixed a few bugs. This all passes xfstests with no new failures on both xfs and tmpfs. I intend to put all this into for-next tomorrow. Matthew Wilcox (Oracle) (48): filemap: Remove PageHWPoison check from next_uptodate_page() fs/writeback: Convert inode_switch_wbs_work_fn to folios mm/doc: Add documentation for folio_test_uptodate mm/writeback: Improve __folio_mark_dirty() comment pagevec: Add folio_batch iov_iter: Add copy_folio_to_iter() iov_iter: Convert iter_xarray to use folios mm: Add folio_test_pmd_mappable() filemap: Add folio_put_wait_locked() filemap: Convert page_cache_delete to take a folio filemap: Add filemap_unaccount_folio() filemap: Convert tracing of page cache operations to folio filemap: Add filemap_remove_folio and __filemap_remove_folio filemap: Convert find_get_entry to return a folio filemap: Remove thp_contains() filemap: Convert filemap_get_read_batch to use folios filemap: Convert find_get_pages_contig to folios filemap: Convert filemap_read_page to take a folio filemap: Convert filemap_create_page to folio filemap: Convert filemap_range_uptodate to folios readahead: Convert page_cache_async_ra() to take a folio readahead: Convert page_cache_ra_unbounded to folios filemap: Convert do_async_mmap_readahead to take a folio filemap: Convert filemap_fault to folio filemap: Add read_cache_folio and read_mapping_folio filemap: Convert filemap_get_pages to use folios filemap: Convert page_cache_delete_batch to folios filemap: Use folios in next_uptodate_page filemap: Use a folio in filemap_map_pages filemap: Use a folio in filemap_page_mkwrite filemap: Add filemap_release_folio() truncate: Add truncate_cleanup_folio() mm: Add unmap_mapping_folio() shmem: Convert part of shmem_undo_range() to use a folio truncate,shmem: Add truncate_inode_folio() truncate: Skip known-truncated indices truncate: Convert invalidate_inode_pages2_range() to use a folio truncate: Add invalidate_complete_folio2() filemap: Convert filemap_read() to use a folio filemap: Convert filemap_get_read_batch() to use a folio_batch filemap: Return only folios from find_get_entries() mm: Convert find_lock_entries() to use a folio_batch mm: Remove pagevec_remove_exceptionals() fs: Convert vfs_dedupe_file_range_compare to folios truncate: Convert invalidate_inode_pages2_range to folios truncate,shmem: Handle truncates that split large folios XArray: Add xas_advance() mm: Use multi-index entries in the page cache fs/fs-writeback.c | 24 +- fs/remap_range.c | 116 ++-- include/linux/huge_mm.h | 14 + include/linux/mm.h | 68 +-- include/linux/page-flags.h | 13 +- include/linux/pagemap.h | 59 +- include/linux/pagevec.h | 61 ++- include/linux/uio.h | 7 + include/linux/xarray.h | 18 + include/trace/events/filemap.h | 32 +- lib/iov_iter.c | 29 +- lib/xarray.c | 6 +- mm/filemap.c | 967 ++++++++++++++++----------------- mm/folio-compat.c | 11 + mm/huge_memory.c | 20 +- mm/internal.h | 35 +- mm/khugepaged.c | 12 +- mm/memory.c | 27 +- mm/migrate.c | 29 +- mm/page-writeback.c | 6 +- mm/readahead.c | 24 +- mm/shmem.c | 174 +++--- mm/swap.c | 26 +- mm/truncate.c | 305 ++++++----- 24 files changed, 1100 insertions(+), 983 deletions(-) -- 2.33.0