This patch seris started out as part of the THP patch set, but it has
some nice effects along the way and it seems worth splitting it out and
submitting separately.
Currently find_get_entry() and find_lock_entry() return the page
corresponding to the requested index, but the first thing most callers do
is find the head page, which we just threw away. As part of auditing
all the callers, I found some misuses of the APIs and some plain
inefficiencies that I've fixed.
The diffstat is unflattering, but I added more kernel-doc.
Matthew Wilcox (Oracle) (8):
mm: Factor find_get_swap_page out of mincore_page
mm: Use find_get_swap_page in memcontrol
mm: Optimise madvise WILLNEED
proc: Optimise smaps for shmem entries
i915: Use find_lock_page instead of find_lock_entry
mm: Convert find_get_entry to return the head page
mm: Return head page from find_lock_entry
mm: Hoist find_subpage call further up in pagecache_get_page
drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 4 +--
fs/proc/task_mmu.c | 8 +----
include/linux/pagemap.h | 16 +++++++--
include/linux/swap.h | 7 ++++
mm/filemap.c | 41 +++++++++++------------
mm/madvise.c | 21 +++++++-----
mm/memcontrol.c | 25 ++------------
mm/mincore.c | 28 ++--------------
mm/shmem.c | 15 +++++----
mm/swap_state.c | 31 +++++++++++++++++
10 files changed, 98 insertions(+), 98 deletions(-)
--
2.28.0
This avoids a call to compound_head().
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
mm/filemap.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/mm/filemap.c b/mm/filemap.c
index 6594baae7cd2..8c354277108d 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1667,7 +1667,6 @@ struct page *pagecache_get_page(struct address_space *mapping, pgoff_t index,
page = NULL;
if (!page)
goto no_page;
- page = find_subpage(page, index);
if (fgp_flags & FGP_LOCK) {
if (fgp_flags & FGP_NOWAIT) {
@@ -1680,12 +1679,12 @@ struct page *pagecache_get_page(struct address_space *mapping, pgoff_t index,
}
/* Has the page been truncated? */
- if (unlikely(compound_head(page)->mapping != mapping)) {
+ if (unlikely(page->mapping != mapping)) {
unlock_page(page);
put_page(page);
goto repeat;
}
- VM_BUG_ON_PAGE(page->index != index, page);
+ VM_BUG_ON_PAGE(!thp_valid_index(page, index), page);
}
if (fgp_flags & FGP_ACCESSED)
@@ -1695,6 +1694,7 @@ struct page *pagecache_get_page(struct address_space *mapping, pgoff_t index,
if (page_is_idle(page))
clear_page_idle(page);
}
+ page = find_subpage(page, index);
no_page:
if (!page && (fgp_flags & FGP_CREAT)) {
--
2.28.0
Avoid bumping the refcount on pages when we're only interested in the
swap entries.
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
fs/proc/task_mmu.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 5066b0251ed8..e42d9e5e9a3c 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -520,16 +520,10 @@ static void smaps_pte_entry(pte_t *pte, unsigned long addr,
page = device_private_entry_to_page(swpent);
} else if (unlikely(IS_ENABLED(CONFIG_SHMEM) && mss->check_shmem_swap
&& pte_none(*pte))) {
- page = find_get_entry(vma->vm_file->f_mapping,
+ page = xa_load(&vma->vm_file->f_mapping->i_pages,
linear_page_index(vma, addr));
- if (!page)
- return;
-
if (xa_is_value(page))
mss->swap += PAGE_SIZE;
- else
- put_page(page);
-
return;
}
--
2.28.0
Convert the one caller of find_lock_entry() to cope with a head page
being returned instead of the subpage for the index.
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
include/linux/pagemap.h | 12 ++++++++++++
mm/filemap.c | 25 +++++++++++--------------
mm/shmem.c | 15 ++++++++-------
3 files changed, 31 insertions(+), 21 deletions(-)
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 8261c64f592d..de8f3758ceaa 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -371,6 +371,18 @@ static inline struct page *grab_cache_page_nowait(struct address_space *mapping,
mapping_gfp_mask(mapping));
}
+/*
+ * Is this index for one of the subpages of this page?
+ * This should always be called for a locked, non-hugetlbfs page.
+ */
+static inline bool thp_valid_index(struct page *head, pgoff_t index)
+{
+ VM_BUG_ON_PAGE(PageHuge(head), head);
+ VM_BUG_ON_PAGE(!PageLocked(head), head);
+
+ return head->index == (index & ~(thp_nr_pages(head) - 1));
+}
+
/*
* Given the page we found in the page cache, return the page corresponding
* to this index in the file
diff --git a/mm/filemap.c b/mm/filemap.c
index 5a87e1bdddd6..6594baae7cd2 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1594,37 +1594,34 @@ struct page *find_get_entry(struct address_space *mapping, pgoff_t index)
}
/**
- * find_lock_entry - locate, pin and lock a page cache entry
- * @mapping: the address_space to search
- * @offset: the page cache index
+ * find_lock_entry - Locate and lock a page cache entry.
+ * @mapping: The address_space to search.
+ * @index: The page cache index.
*
- * Looks up the page cache slot at @mapping & @offset. If there is a
- * page cache page, it is returned locked and with an increased
- * refcount.
+ * Looks up the page at @mapping & @index. If there is a page in the
+ * cache, the head page is returned locked and with an increased refcount.
*
* If the slot holds a shadow entry of a previously evicted page, or a
* swap entry from shmem/tmpfs, it is returned.
*
- * find_lock_entry() may sleep.
- *
- * Return: the found page or shadow entry, %NULL if nothing is found.
+ * Context: May sleep.
+ * Return: The head page or shadow entry, %NULL if nothing is found.
*/
-struct page *find_lock_entry(struct address_space *mapping, pgoff_t offset)
+struct page *find_lock_entry(struct address_space *mapping, pgoff_t index)
{
struct page *page;
repeat:
- page = find_get_entry(mapping, offset);
+ page = find_get_entry(mapping, index);
if (page && !xa_is_value(page)) {
lock_page(page);
/* Has the page been truncated? */
- if (unlikely(page_mapping(page) != mapping)) {
+ if (unlikely(page->mapping != mapping)) {
unlock_page(page);
put_page(page);
goto repeat;
}
- page = find_subpage(page, offset);
- VM_BUG_ON_PAGE(page_to_pgoff(page) != offset, page);
+ VM_BUG_ON_PAGE(!thp_valid_index(page, index), page);
}
return page;
}
diff --git a/mm/shmem.c b/mm/shmem.c
index 271548ca20f3..bbb43e9d35df 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -1834,8 +1834,9 @@ static int shmem_getpage_gfp(struct inode *inode, pgoff_t index,
page = NULL;
}
if (page || sgp == SGP_READ) {
- *pagep = page;
- return 0;
+ if (page && PageTransHuge(page))
+ hindex = round_down(index, HPAGE_PMD_NR);
+ goto out;
}
/*
@@ -1961,14 +1962,13 @@ static int shmem_getpage_gfp(struct inode *inode, pgoff_t index,
* it now, lest undo on failure cancel our earlier guarantee.
*/
if (sgp != SGP_WRITE && !PageUptodate(page)) {
- struct page *head = compound_head(page);
int i;
- for (i = 0; i < compound_nr(head); i++) {
- clear_highpage(head + i);
- flush_dcache_page(head + i);
+ for (i = 0; i < compound_nr(page); i++) {
+ clear_highpage(page + i);
+ flush_dcache_page(page + i);
}
- SetPageUptodate(head);
+ SetPageUptodate(page);
}
/* Perhaps the file has been truncated since we checked */
@@ -1984,6 +1984,7 @@ static int shmem_getpage_gfp(struct inode *inode, pgoff_t index,
error = -EINVAL;
goto unlock;
}
+out:
*pagep = page + index - hindex;
return 0;
--
2.28.0
> On Aug 19, 2020, at 12:48 PM, Matthew Wilcox (Oracle) <[email protected]> wrote:
>
> This patch seris started out as part of the THP patch set, but it has
> some nice effects along the way and it seems worth splitting it out and
> submitting separately.
>
> Currently find_get_entry() and find_lock_entry() return the page
> corresponding to the requested index, but the first thing most callers do
> is find the head page, which we just threw away. As part of auditing
> all the callers, I found some misuses of the APIs and some plain
> inefficiencies that I've fixed.
>
> The diffstat is unflattering, but I added more kernel-doc.
>
> Matthew Wilcox (Oracle) (8):
> mm: Factor find_get_swap_page out of mincore_page
> mm: Use find_get_swap_page in memcontrol
> mm: Optimise madvise WILLNEED
> proc: Optimise smaps for shmem entries
> i915: Use find_lock_page instead of find_lock_entry
> mm: Convert find_get_entry to return the head page
> mm: Return head page from find_lock_entry
> mm: Hoist find_subpage call further up in pagecache_get_page
>
> drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 4 +--
> fs/proc/task_mmu.c | 8 +----
> include/linux/pagemap.h | 16 +++++++--
> include/linux/swap.h | 7 ++++
> mm/filemap.c | 41 +++++++++++------------
> mm/madvise.c | 21 +++++++-----
> mm/memcontrol.c | 25 ++------------
> mm/mincore.c | 28 ++--------------
> mm/shmem.c | 15 +++++----
> mm/swap_state.c | 31 +++++++++++++++++
> 10 files changed, 98 insertions(+), 98 deletions(-)
For the series:
Reviewed-by: William Kucharski <[email protected]>
On Wed, Aug 19, 2020 at 07:48:46PM +0100, Matthew Wilcox (Oracle) wrote:
> Avoid bumping the refcount on pages when we're only interested in the
> swap entries.
>
> Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
Acked-by: Johannes Weiner <[email protected]>