On 06/06/2017 07:58 PM, Will Deacon wrote:
> When migrating a transparent hugepage, migrate_misplaced_transhuge_page
> guards itself against a concurrent fastgup of the page by checking that
> the page count is equal to 2 before and after installing the new pmd.
>
> If the page count changes, then the pmd is reverted back to the original
> entry, however there is a small window where the new (possibly writable)
> pmd is installed and the underlying page could be written by userspace.
> Restoring the old pmd could therefore result in loss of data.
>
> This patch fixes the problem by freezing the page count whilst updating
> the page tables, which protects against a concurrent fastgup without the
> need to restore the old pmd in the failure case (since the page count can
> no longer change under our feet).
>
> Cc: Mel Gorman <mgor...@suse.de>
> Signed-off-by: Will Deacon <will.dea...@arm.com>
> ---
> mm/migrate.c | 15 ++-------------
> 1 file changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 89a0a1707f4c..8b21f1b1ec6e 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -1913,7 +1913,6 @@ int migrate_misplaced_transhuge_page(struct mm_struct
> *mm,
> int page_lru = page_is_file_cache(page);
> unsigned long mmun_start = address & HPAGE_PMD_MASK;
> unsigned long mmun_end = mmun_start + HPAGE_PMD_SIZE;
> - pmd_t orig_entry;
>
> /*
> * Rate-limit the amount of data that is being migrated to a node.
> @@ -1956,8 +1955,7 @@ int migrate_misplaced_transhuge_page(struct mm_struct
> *mm,
> /* Recheck the target PMD */
> mmu_notifier_invalidate_range_start(mm, mmun_start, mmun_end);
> ptl = pmd_lock(mm, pmd);
> - if (unlikely(!pmd_same(*pmd, entry) || page_count(page) != 2)) {
> -fail_putback:
> + if (unlikely(!pmd_same(*pmd, entry) || !page_ref_freeze(page, 2))) {
> spin_unlock(ptl);
> mmu_notifier_invalidate_range_end(mm, mmun_start, mmun_end);
>
> @@ -1979,7 +1977,6 @@ int migrate_misplaced_transhuge_page(struct mm_struct
> *mm,
> goto out_unlock;
> }
>
> - orig_entry = *pmd;
> entry = mk_huge_pmd(new_page, vma->vm_page_prot);
> entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma);
>
> @@ -1996,15 +1993,7 @@ int migrate_misplaced_transhuge_page(struct mm_struct
> *mm,
There's a comment above this:
/*
* Clear the old entry under pagetable lock and establish the new PTE.
* Any parallel GUP will either observe the old page blocking on the
* page lock, block on the page table lock or observe the new page.
* The SetPageUptodate on the new page and page_add_new_anon_rmap
* guarantee the copy is visible before the pagetable update.
*/
Is it still correct? Didn't the freezing prevent some of the cases above?
> set_pmd_at(mm, mmun_start, pmd, entry);
> update_mmu_cache_pmd(vma, address, &entry);
>
> - if (page_count(page) != 2) {
BTW, how did the old code recognize that page count would increase and then
decrease back?
> - set_pmd_at(mm, mmun_start, pmd, orig_entry);
> - flush_pmd_tlb_range(vma, mmun_start, mmun_end);
> - mmu_notifier_invalidate_range(mm, mmun_start, mmun_end);
> - update_mmu_cache_pmd(vma, address, &entry);
> - page_remove_rmap(new_page, true);
> - goto fail_putback;
> - }
> -
> + page_ref_unfreeze(page, 2);
> mlock_migrate_page(new_page, page);
> page_remove_rmap(page, true);
> set_page_owner_migrate_reason(new_page, MR_NUMA_MISPLACED);
>
