On Wed, Aug 19, 2020 at 07:48:44PM +0100, Matthew Wilcox (Oracle) wrote:
> The current code does not protect against swapoff of the underlying
> swap device, so this is a bug fix as well as a worthwhile reduction in
> code complexity.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
> ---
> mm/memcontrol.c | 25 ++-----------------------
> 1 file changed, 2 insertions(+), 23 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index b807952b4d43..4075f214a841 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -5539,35 +5539,14 @@ static struct page *mc_handle_swap_pte(struct vm_area_struct *vma,
> static struct page *mc_handle_file_pte(struct vm_area_struct *vma,
> unsigned long addr, pte_t ptent, swp_entry_t *entry)
> {
> - struct page *page = NULL;
> - struct address_space *mapping;
> - pgoff_t pgoff;
> -
> if (!vma->vm_file) /* anonymous vma */
> return NULL;
> if (!(mc.flags & MOVE_FILE))
> return NULL;
>
> - mapping = vma->vm_file->f_mapping;
> - pgoff = linear_page_index(vma, addr);
> -
> /* page is moved even if it's not RSS of this task(page-faulted). */
> -#ifdef CONFIG_SWAP
> - /* shmem/tmpfs may report page out on swap: account for that too. */
> - if (shmem_mapping(mapping)) {
> - page = find_get_entry(mapping, pgoff);
> - if (xa_is_value(page)) {
> - swp_entry_t swp = radix_to_swp_entry(page);
> - *entry = swp;
> - page = find_get_page(swap_address_space(swp),
> - swp_offset(swp));
> - }
> - } else
> - page = find_get_page(mapping, pgoff);
> -#else
> - page = find_get_page(mapping, pgoff);
> -#endif
> - return page;
> + return find_get_swap_page(vma->vm_file->f_mapping,
> + linear_page_index(vma, addr));
The refactor makes sense to me, but the name is confusing. We're not
looking for a swap page, we're primarily looking for a file page in
the page cache mapping that's handed in. Only in the special case
where it's a shmem mapping and there is a swap entry do we consult the
auxiliary swap cache.
How about find_get_page_or_swapcache()? find_get_page_shmemswap()?
Maybe you have a better idea. It's a fairly specialized operation that
isn't widely used, so a longer name isn't a bad thing IMO.
