On 05/23/2018 05:58 PM, Huang, Ying wrote:
> From: Huang Ying <ying.huang@xxxxxxxxx>
>
> To take better advantage of general huge page copying optimization,
> the target subpage address will be passed to hugetlb_cow(), then
> copy_user_huge_page(). So we will use both target subpage address and
> huge page size aligned address in hugetlb_cow(). To distinguish
> between them, "haddr" is used for huge page size aligned address to be
> consistent with Transparent Huge Page naming convention.
>
> Now, only huge page size aligned address is used in hugetlb_cow(), so
> the "address" is renamed to "haddr" in hugetlb_cow() in this patch.
> Next patch will use target subpage address in hugetlb_cow() too.
>
> The patch is just code cleanup without any functionality changes.
>
> Signed-off-by: "Huang, Ying" <ying.huang@xxxxxxxxx>
> Suggested-by: Mike Kravetz <mike.kravetz@xxxxxxxxxx>
> Suggested-by: Michal Hocko <mhocko@xxxxxxxx>
I believe Kirill may have been the one who suggested using haddr to be
consistent with usage in huge_memory.c.
> Cc: David Rientjes <rientjes@xxxxxxxxxx>
> Cc: Andrea Arcangeli <aarcange@xxxxxxxxxx>
> Cc: "Kirill A. Shutemov" <kirill.shutemov@xxxxxxxxxxxxxxx>
> Cc: Andi Kleen <andi.kleen@xxxxxxxxx>
> Cc: Jan Kara <jack@xxxxxxx>
> Cc: Matthew Wilcox <mawilcox@xxxxxxxxxxxxx>
> Cc: Hugh Dickins <hughd@xxxxxxxxxx>
> Cc: Minchan Kim <minchan@xxxxxxxxxx>
> Cc: Shaohua Li <shli@xxxxxx>
> Cc: Christopher Lameter <cl@xxxxxxxxx>
> Cc: "Aneesh Kumar K.V" <aneesh.kumar@xxxxxxxxxxxxxxxxxx>
> Cc: Punit Agrawal <punit.agrawal@xxxxxxx>
> Cc: Anshuman Khandual <khandual@xxxxxxxxxxxxxxxxxx>
> ---
> mm/hugetlb.c | 26 ++++++++++++--------------
> 1 file changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 696befffe6f7..ad3bec2ed269 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -3500,7 +3500,7 @@ static void unmap_ref_private(struct mm_struct *mm, struct vm_area_struct *vma,
> * Keep the pte_same checks anyway to make transition from the mutex easier.
> */
> static int hugetlb_cow(struct mm_struct *mm, struct vm_area_struct *vma,
> - unsigned long address, pte_t *ptep,
> + unsigned long haddr, pte_t *ptep,
> struct page *pagecache_page, spinlock_t *ptl)
> {
> pte_t pte;
> @@ -3518,7 +3518,7 @@ static int hugetlb_cow(struct mm_struct *mm, struct vm_area_struct *vma,
> * and just make the page writable */
> if (page_mapcount(old_page) == 1 && PageAnon(old_page)) {
> page_move_anon_rmap(old_page, vma);
> - set_huge_ptep_writable(vma, address, ptep);
> + set_huge_ptep_writable(vma, haddr, ptep);
> return 0;
> }
>
> @@ -3542,7 +3542,7 @@ static int hugetlb_cow(struct mm_struct *mm, struct vm_area_struct *vma,
> * be acquired again before returning to the caller, as expected.
> */
> spin_unlock(ptl);
> - new_page = alloc_huge_page(vma, address, outside_reserve);
> + new_page = alloc_huge_page(vma, haddr, outside_reserve);
>
> if (IS_ERR(new_page)) {
> /*
> @@ -3555,11 +3555,10 @@ static int hugetlb_cow(struct mm_struct *mm, struct vm_area_struct *vma,
> if (outside_reserve) {
> put_page(old_page);
> BUG_ON(huge_pte_none(pte));
> - unmap_ref_private(mm, vma, old_page, address);
> + unmap_ref_private(mm, vma, old_page, haddr);
> BUG_ON(huge_pte_none(pte));
> spin_lock(ptl);
> - ptep = huge_pte_offset(mm, address & huge_page_mask(h),
> - huge_page_size(h));
> + ptep = huge_pte_offset(mm, haddr, huge_page_size(h));
Ha! The name change points out an obviously unnecessary conversion in
the existing code. Yes, hugetlb_cow is always passed a hpage aligned
address today.
> if (likely(ptep &&
> pte_same(huge_ptep_get(ptep), pte)))
> goto retry_avoidcopy;
> @@ -3584,12 +3583,12 @@ static int hugetlb_cow(struct mm_struct *mm, struct vm_area_struct *vma,
> goto out_release_all;
> }
>
> - copy_user_huge_page(new_page, old_page, address, vma,
> + copy_user_huge_page(new_page, old_page, haddr, vma,
> pages_per_huge_page(h));
> __SetPageUptodate(new_page);
> set_page_huge_active(new_page);
>
> - mmun_start = address & huge_page_mask(h);
> + mmun_start = haddr;
And another one.
> mmun_end = mmun_start + huge_page_size(h);
> mmu_notifier_invalidate_range_start(mm, mmun_start, mmun_end);
>
> @@ -3598,25 +3597,24 @@ static int hugetlb_cow(struct mm_struct *mm, struct vm_area_struct *vma,
> * before the page tables are altered
> */
> spin_lock(ptl);
> - ptep = huge_pte_offset(mm, address & huge_page_mask(h),
> - huge_page_size(h));
> + ptep = huge_pte_offset(mm, haddr, huge_page_size(h));
And yet another.
Reviewed-by: Mike Kravetz <mike.kravetz@xxxxxxxxxx>
--
Mike Kravetz
> if (likely(ptep && pte_same(huge_ptep_get(ptep), pte))) {
> ClearPagePrivate(new_page);
>
> /* Break COW */
> - huge_ptep_clear_flush(vma, address, ptep);
> + huge_ptep_clear_flush(vma, haddr, ptep);
> mmu_notifier_invalidate_range(mm, mmun_start, mmun_end);
> - set_huge_pte_at(mm, address, ptep,
> + set_huge_pte_at(mm, haddr, ptep,
> make_huge_pte(vma, new_page, 1));
> page_remove_rmap(old_page, true);
> - hugepage_add_new_anon_rmap(new_page, vma, address);
> + hugepage_add_new_anon_rmap(new_page, vma, haddr);
> /* Make the old page be freed below */
> new_page = old_page;
> }
> spin_unlock(ptl);
> mmu_notifier_invalidate_range_end(mm, mmun_start, mmun_end);
> out_release_all:
> - restore_reserve_on_error(h, vma, address, new_page);
> + restore_reserve_on_error(h, vma, haddr, new_page);
> put_page(new_page);
> out_release_old:
> put_page(old_page);
>
