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 <william.kucharski@xxxxxxxxxx>
> On Dec 7, 2021, at 9:22 PM, Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx> 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
>
>
