2020-08-19 15:10:05

by Matthew Wilcox

[permalink] [raw]
Subject: [PATCH 0/7] Overhaul find_get_entries and pagevec_lookup_entries

This started out as part of the THP patchset, but it's turned into a
nice simplification in its own right. Essentially we end up unifying
find_get_entries() and pagevec_lookup_entries() into one function that's
better than either, and we get rid of a lot of code in the callers as
a result.

I'm running this through xfstests right now, but something similar to
this has already passed xfstests as part of the THP patchset.

I've done my best to avoid off-by-one errors for 'end', but I wouldn't be
surprised if I made a mistake. We're not consistent with whether 'end'
is inclusive or exclusive and I didn't want to make extensive changes
to ensure they were consistent.

Matthew Wilcox (Oracle) (7):
mm: Use pagevec_lookup in shmem_unlock_mapping
mm: Rewrite shmem_seek_hole_data
mm: Add an 'end' parameter to find_get_entries
mm: Add an 'end' parameter to pagevec_lookup_entries
mm: Remove nr_entries parameter from pagevec_lookup_entries
mm: Pass pvec directly to find_get_entries
mm: Remove pagevec_lookup_entries

include/linux/pagemap.h | 3 +-
include/linux/pagevec.h | 4 --
mm/filemap.c | 19 +++++----
mm/shmem.c | 85 ++++++++++++++---------------------------
mm/swap.c | 38 +-----------------
mm/truncate.c | 33 +++-------------
6 files changed, 45 insertions(+), 137 deletions(-)

--
2.28.0


2020-08-19 15:11:33

by Matthew Wilcox

[permalink] [raw]
Subject: [PATCH 7/7] mm: Remove pagevec_lookup_entries

pagevec_lookup_entries() is now just a wrapper around find_get_entries()
so remove it and convert all its callers.

Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
include/linux/pagevec.h | 3 ---
mm/swap.c | 36 ++----------------------------------
mm/truncate.c | 9 ++++-----
3 files changed, 6 insertions(+), 42 deletions(-)

diff --git a/include/linux/pagevec.h b/include/linux/pagevec.h
index ce77724a2ab7..a45bea4b4d08 100644
--- a/include/linux/pagevec.h
+++ b/include/linux/pagevec.h
@@ -25,9 +25,6 @@ struct pagevec {

void __pagevec_release(struct pagevec *pvec);
void __pagevec_lru_add(struct pagevec *pvec);
-unsigned pagevec_lookup_entries(struct pagevec *pvec,
- struct address_space *mapping, pgoff_t start, pgoff_t end,
- pgoff_t *indices);
void pagevec_remove_exceptionals(struct pagevec *pvec);
unsigned pagevec_lookup_range(struct pagevec *pvec,
struct address_space *mapping,
diff --git a/mm/swap.c b/mm/swap.c
index 40b23300d353..cc0d648f79d4 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -1031,44 +1031,12 @@ void __pagevec_lru_add(struct pagevec *pvec)
pagevec_lru_move_fn(pvec, __pagevec_lru_add_fn, NULL);
}

-/**
- * pagevec_lookup_entries - gang pagecache lookup
- * @pvec: Where the resulting entries are placed
- * @mapping: The address_space to search
- * @start: The starting entry index
- * @end: The highest index to return (inclusive).
- * @nr_entries: The maximum number of pages
- * @indices: The cache indices corresponding to the entries in @pvec
- *
- * pagevec_lookup_entries() will search for and return a group of up
- * to @nr_pages pages and shadow entries in the mapping. All
- * entries are placed in @pvec. pagevec_lookup_entries() takes a
- * reference against actual pages in @pvec.
- *
- * The search returns a group of mapping-contiguous entries with
- * ascending indexes. There may be holes in the indices due to
- * not-present entries.
- *
- * Only one subpage of a Transparent Huge Page is returned in one call:
- * allowing truncate_inode_pages_range() to evict the whole THP without
- * cycling through a pagevec of extra references.
- *
- * pagevec_lookup_entries() returns the number of entries which were
- * found.
- */
-unsigned pagevec_lookup_entries(struct pagevec *pvec,
- struct address_space *mapping, pgoff_t start, pgoff_t end,
- pgoff_t *indices)
-{
- return find_get_entries(mapping, start, end, pvec, indices);
-}
-
/**
* pagevec_remove_exceptionals - pagevec exceptionals pruning
* @pvec: The pagevec to prune
*
- * pagevec_lookup_entries() fills both pages and exceptional radix
- * tree entries into the pagevec. This function prunes all
+ * find_get_entries() fills both pages and XArray value entries (aka
+ * exceptional entries) into the pagevec. This function prunes all
* exceptionals from @pvec without leaving holes, so that it can be
* passed on to page-only pagevec operations.
*/
diff --git a/mm/truncate.c b/mm/truncate.c
index 96a45ba28042..90b07884db9f 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -326,8 +326,7 @@ void truncate_inode_pages_range(struct address_space *mapping,

pagevec_init(&pvec);
index = start;
- while (pagevec_lookup_entries(&pvec, mapping, index, end - 1,
- indices)) {
+ while (find_get_entries(mapping, index, end - 1, &pvec, indices)) {
/*
* Pagevec array has exceptional entries and we may also fail
* to lock some pages. So we store pages that can be deleted
@@ -410,7 +409,7 @@ void truncate_inode_pages_range(struct address_space *mapping,
index = start;
for ( ; ; ) {
cond_resched();
- if (!pagevec_lookup_entries(&pvec, mapping, index, end - 1,
+ if (!find_get_entries(mapping, index, end - 1, &pvec,
indices)) {
/* If all gone from start onwards, we're done */
if (index == start)
@@ -540,7 +539,7 @@ unsigned long invalidate_mapping_pages(struct address_space *mapping,
int i;

pagevec_init(&pvec);
- while (pagevec_lookup_entries(&pvec, mapping, index, end, indices)) {
+ while (find_get_entries(mapping, index, end, &pvec, indices)) {
for (i = 0; i < pagevec_count(&pvec); i++) {
struct page *page = pvec.pages[i];

@@ -679,7 +678,7 @@ int invalidate_inode_pages2_range(struct address_space *mapping,

pagevec_init(&pvec);
index = start;
- while (pagevec_lookup_entries(&pvec, mapping, index, end, indices)) {
+ while (find_get_entries(mapping, index, end, &pvec, indices)) {
for (i = 0; i < pagevec_count(&pvec); i++) {
struct page *page = pvec.pages[i];

--
2.28.0

2020-08-22 02:39:45

by William Kucharski

[permalink] [raw]
Subject: Re: [PATCH 0/7] Overhaul find_get_entries and pagevec_lookup_entries



> On Aug 19, 2020, at 9:05 AM, Matthew Wilcox (Oracle) <[email protected]> wrote:
>
> This started out as part of the THP patchset, but it's turned into a
> nice simplification in its own right. Essentially we end up unifying
> find_get_entries() and pagevec_lookup_entries() into one function that's
> better than either, and we get rid of a lot of code in the callers as
> a result.
>
> I'm running this through xfstests right now, but something similar to
> this has already passed xfstests as part of the THP patchset.
>
> I've done my best to avoid off-by-one errors for 'end', but I wouldn't be
> surprised if I made a mistake. We're not consistent with whether 'end'
> is inclusive or exclusive and I didn't want to make extensive changes
> to ensure they were consistent.
>
> Matthew Wilcox (Oracle) (7):
> mm: Use pagevec_lookup in shmem_unlock_mapping
> mm: Rewrite shmem_seek_hole_data
> mm: Add an 'end' parameter to find_get_entries
> mm: Add an 'end' parameter to pagevec_lookup_entries
> mm: Remove nr_entries parameter from pagevec_lookup_entries
> mm: Pass pvec directly to find_get_entries
> mm: Remove pagevec_lookup_entries
>
> include/linux/pagemap.h | 3 +-
> include/linux/pagevec.h | 4 --
> mm/filemap.c | 19 +++++----
> mm/shmem.c | 85 ++++++++++++++---------------------------
> mm/swap.c | 38 +-----------------
> mm/truncate.c | 33 +++-------------
> 6 files changed, 45 insertions(+), 137 deletions(-)

Very nice cleanups and the code makes more sense, thanks.

Reviewed-by: William Kucharski <[email protected]>