On Thu, 13 Dec 2018 18:29:08 +0300 Kirill Tkhai <ktkhai@xxxxxxxxxxxxx> wrote:
> This patch adds an optimization for KSM pages almost
> in the same way, that we have for ordinary anonymous
> pages. If there is a write fault in a page, which is
> mapped to an only pte, and it is not related to swap
> cache; the page may be reused without copying its
> content.
>
> [Note, that we do not consider PageSwapCache() pages
> at least for now, since we don't want to complicate
> __get_ksm_page(), which has nice optimization based
> on this (for the migration case). Currenly it is
> spinning on PageSwapCache() pages, waiting for when
> they have unfreezed counters (i.e., for the migration
> finish). But we don't want to make it also spinning
> on swap cache pages, which we try to reuse, since
> there is not a very high probability to reuse them.
> So, for now we do not consider PageSwapCache() pages
> at all.]
>
> So, in reuse_ksm_page() we check for 1)PageSwapCache()
> and 2)page_stable_node(), to skip a page, which KSM
> is currently trying to link to stable tree. Then we
> do page_ref_freeze() to prohibit KSM to merge one more
> page into the page, we are reusing. After that, nobody
> can refer to the reusing page: KSM skips !PageSwapCache()
> pages with zero refcount; and the protection against
> of all other participants is the same as for reused
> ordinary anon pages pte lock, page lock and mmap_sem.
>
> ...
>
> +bool reuse_ksm_page(struct page *page,
> + struct vm_area_struct *vma,
> + unsigned long address)
> +{
> + VM_BUG_ON_PAGE(is_zero_pfn(page_to_pfn(page)), page);
> + VM_BUG_ON_PAGE(!page_mapped(page), page);
> + VM_BUG_ON_PAGE(!PageLocked(page), page);
> +
> + if (PageSwapCache(page) || !page_stable_node(page))
> + return false;
> + /* Prohibit parallel get_ksm_page() */
> + if (!page_ref_freeze(page, 1))
> + return false;
> +
> + page_move_anon_rmap(page, vma);
> + page->index = linear_page_index(vma, address);
> + page_ref_unfreeze(page, 1);
> +
> + return true;
> +}
Can we avoid those BUG_ON()s?
Something like this:
--- a/mm/ksm.c~mm-reuse-only-pte-mapped-ksm-page-in-do_wp_page-fix
+++ a/mm/ksm.c
@@ -2649,9 +2649,14 @@ bool reuse_ksm_page(struct page *page,
struct vm_area_struct *vma,
unsigned long address)
{
- VM_BUG_ON_PAGE(is_zero_pfn(page_to_pfn(page)), page);
- VM_BUG_ON_PAGE(!page_mapped(page), page);
- VM_BUG_ON_PAGE(!PageLocked(page), page);
+#ifdef CONFIG_DEBUG_VM
+ if (WARN_ON(is_zero_pfn(page_to_pfn(page))) ||
+ WARN_ON(!page_mapped(page)) ||
+ WARN_ON(!PageLocked(page))) {
+ dump_page(page, "reuse_ksm_page");
+ return false;
+ }
+#endif
if (PageSwapCache(page) || !page_stable_node(page))
return false;
We don't have a VM_WARN_ON_PAGE() and we can't provide one because the
VM_foo() macros don't return a value. It's irritating and I keep
forgetting why we ended up doing them this way.
