mbox series

[0/8] Return head pages from find_get_entry and find_lock_entry

Message ID 20200819184850.24779-1-willy@infradead.org (mailing list archive)
Headers show
Series Return head pages from find_get_entry and find_lock_entry | expand

Message

Matthew Wilcox Aug. 19, 2020, 6:48 p.m. UTC
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(-)

Comments

Matthew Wilcox Aug. 19, 2020, 7:16 p.m. UTC | #1
On Wed, Aug 19, 2020 at 07:06:17PM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: Return head pages from find_get_entry and find_lock_entry
> URL   : https://patchwork.freedesktop.org/series/80818/
> State : failure
> 
> == Summary ==
> 
> CALL    scripts/checksyscalls.sh
>   CALL    scripts/atomic/check-atomics.sh
>   DESCEND  objtool
>   CHK     include/generated/compile.h
>   CC      mm/swap_state.o
> mm/swap_state.c: In function ‘find_get_swap_page’:
> mm/swap_state.c:435:7: error: implicit declaration of function ‘shmem_mapping’; did you mean ‘page_mapping’? [-Werror=implicit-function-declaration]
>   if (!shmem_mapping(mapping))
>        ^~~~~~~~~~~~~
>        page_mapping
> cc1: some warnings being treated as errors
> scripts/Makefile.build:283: recipe for target 'mm/swap_state.o' failed
> make[1]: *** [mm/swap_state.o] Error 1
> Makefile:1789: recipe for target 'mm' failed
> make: *** [mm] Error 2

Thanks!  Do you have the .config for this build?
Matthew Wilcox Aug. 19, 2020, 7:22 p.m. UTC | #2
On Wed, Aug 19, 2020 at 07:06:17PM -0000, Patchwork wrote:
>   CC      mm/swap_state.o
> mm/swap_state.c: In function ‘find_get_swap_page’:
> mm/swap_state.c:435:7: error: implicit declaration of function ‘shmem_mapping’; did you mean ‘page_mapping’? [-Werror=implicit-function-declaration]
>   if (!shmem_mapping(mapping))
>        ^~~~~~~~~~~~~
>        page_mapping
> cc1: some warnings being treated as errors

Never mind the .config; I decided on a way to fix it without seeing that.
It would be useful for future warnings though.
Matthew Wilcox Aug. 20, 2020, 1:29 a.m. UTC | #3
On Wed, Aug 19, 2020 at 07:29:24PM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: Return head pages from find_get_entry and find_lock_entry (rev2)
> URL   : https://patchwork.freedesktop.org/series/80818/
> State : failure
> 
> == Summary ==
> 
> CALL    scripts/checksyscalls.sh
>   CALL    scripts/atomic/check-atomics.sh
>   DESCEND  objtool
>   CHK     include/generated/compile.h
>   CC      mm/memcontrol.o
> mm/memcontrol.c: In function ‘mc_handle_file_pte’:
> mm/memcontrol.c:5548:9: error: implicit declaration of function ‘find_get_swap_page’; did you mean ‘get_swap_page’? [-Werror=implicit-function-declaration]
>   return find_get_swap_page(vma->vm_file->f_mapping,
>          ^~~~~~~~~~~~~~~~~~
>          get_swap_page
> mm/memcontrol.c:5548:9: warning: return makes pointer from integer without a cast [-Wint-conversion]
>   return find_get_swap_page(vma->vm_file->f_mapping,
>          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     linear_page_index(vma, addr));
>     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> cc1: some warnings being treated as errors

This doesn't make sense.  Dave Airlie pointed me at what he believes to
be the config file used [1] and I can't reproduce it.  Is it possible
the build-bot applied only 2/8 and not 1/8?

[1] https://gitlab.freedesktop.org/gfx-ci/i915-infra/-/blob/master/kconfig/debug
William Kucharski Aug. 21, 2020, 5:37 p.m. UTC | #4
> On Aug 19, 2020, at 12:48 PM, Matthew Wilcox (Oracle) <willy@infradead.org> 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 <william.kucharski@oracle.com>