Re: [PATCH -mm] mm: refactor page index/offset getters

[Johannes Weiner <hannes@cmpxchg.org>]

On Tue, Jul 15, 2014 at 12:41:12PM -0400, Naoya Horiguchi wrote:
> @@ -399,28 +399,24 @@ static inline struct page *read_mapping_page(struct address_space *mapping,
>  }
>  
>  /*
> - * Get the offset in PAGE_SIZE.
> + * Return the 4kB page offset of the given page.
>   * (TODO: hugepage should have ->index in PAGE_SIZE)
>   */
> -static inline pgoff_t page_to_pgoff(struct page *page)
> +static inline pgoff_t page_pgoff(struct page *page)
>  {
> -	if (unlikely(PageHeadHuge(page)))
> -		return page->index << compound_order(page);
> -	else
> -		return page->index << (PAGE_CACHE_SHIFT - PAGE_SHIFT);
> +	if (unlikely(PageHuge(page))) {
> +		VM_BUG_ON_PAGE(PageTail(page), page);
> +		return page_index(page) << compound_order(page);
> +	} else
> +		return page_index(page) << (PAGE_CACHE_SHIFT - PAGE_SHIFT);
>  }

I just bisected the VM refusing to swap and triggering OOM kills to
this patch, which is likely the same bug you reported a couple days
back when you had this patch in your private tree.

Changing page->index to page_index() makes this function return the
swap offset rather than the virtual PFN, but rmap uses this to index
into virtual address space.  Thus, swapcache pages can no longer be
found from try_to_unmap() and reclaim fails.

We can't simply change it back to page->index, however, because the
swapout path, which requires the swap offset, also uses this function
through page_offset().  Virtual address space functions and page cache
address space functions can't use the same helpers, and the helpers
should likely be named distinctly so that they are not confused and
it's clear what is being asked.  Plus, the patch forced every fs using
page_offset() to suddenly check PageHuge(), which is a function call.

How about

o page_offset() for use by filesystems, based on page->index

o page_virt_pgoff() for use on virtual memory math, based on
  page->index and respecting PageHuge()

o page_mapping_pgoff() for use by swapping and when working on
  mappings that could be swapper_space.

o page_mapping_offset() likewise, just in bytes

Hm?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>