Re: [PATCH v3 2/3] mm: changes to split_huge_page() to free zero filled tail pages

["Huang, Ying" <ying.huang@intel.com>]

<alexlzhu@fb.com> writes:

[snip]

> diff --git a/mm/migrate.c b/mm/migrate.c
> index c228afba0963..504ea5d7fd43 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -168,13 +168,62 @@ void putback_movable_pages(struct list_head *l)
>  	}
>  }
>  
> +static bool try_to_unmap_clean(struct page_vma_mapped_walk *pvmw, struct page *page)
> +{
> +	void *addr;
> +	bool dirty;
> +	pte_t newpte;
> +
> +	VM_BUG_ON_PAGE(PageCompound(page), page);
> +	VM_BUG_ON_PAGE(!PageAnon(page), page);
> +	VM_BUG_ON_PAGE(!PageLocked(page), page);
> +	VM_BUG_ON_PAGE(pte_present(*pvmw->pte), page);
> +
> +	if (PageMlocked(page) || (pvmw->vma->vm_flags & VM_LOCKED))
> +		return false;
> +
> +	/*
> +	 * The pmd entry mapping the old thp was flushed and the pte mapping
> +	 * this subpage has been non present. Therefore, this subpage is
> +	 * inaccessible. We don't need to remap it if it contains only zeros.
> +	 */
> +	addr = kmap_local_page(page);
> +	dirty = memchr_inv(addr, 0, PAGE_SIZE);
> +	kunmap_local(addr);
> +
> +	if (dirty)
> +		return false;
> +
> +	pte_clear_not_present_full(pvmw->vma->vm_mm, pvmw->address, pvmw->pte, false);
> +
> +	if (userfaultfd_armed(pvmw->vma)) {
> +		newpte = pte_mkspecial(pfn_pte(page_to_pfn(ZERO_PAGE(pvmw->address)),
> +					       pvmw->vma->vm_page_prot));
> +		ptep_clear_flush(pvmw->vma, pvmw->address, pvmw->pte);

Why do we need clear PTE and flush TLB again?  If my understanding were
correct, the TLB has been flushed when unmapping the page.  Otherwise,
we need to flush TLB before checking whether the page is all-zero.

> +		set_pte_at(pvmw->vma->vm_mm, pvmw->address, pvmw->pte, newpte);
> +		dec_mm_counter(pvmw->vma->vm_mm, MM_ANONPAGES);
> +		count_vm_event(THP_SPLIT_REMAP_READONLY_ZERO_PAGE);
> +		return true;
> +	}
> +
> +	dec_mm_counter(pvmw->vma->vm_mm, mm_counter(page));

This can be moved before "if (userfaultfd_armed(pvmw->vma))".

> +	count_vm_event(THP_SPLIT_UNMAP);
> +	return true;
> +}
> +

[snip]

Best Regards,
Huang, Ying