Mike Kravetz <mike.kravetz@xxxxxxxxxx> writes:
> 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.
Yes. I should have added
Suggested-by: "Kirill A. Shutemov" <kirill.shutemov@xxxxxxxxxxxxxxx>
Best Regards,
Huang, Ying
>> 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>
