Re: [PATCH -mm] mm, madvise, THP: Use THP aligned address in madvise_free_huge_pmd()

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

Zi Yan <zi.yan@cs.rutgers.edu> writes:

> This cannot happen.
>
> Two address parameters are passed: addr and next.
> If a??addra?? is not aligned and a??nexta?? is aligned or the end of madvise range, which might not be aligned,
> either way next - addr < HPAGE_PMD_SIZE.
>
> This means the code in a??if (next - addr != HPAGE_PMD_SIZE)a??, which is above your second hunk,
> will split the THP between a??addra?? and a??nexta?? and get out as long as a??addra?? is not aligned.
> Thus, the code in your second hunk should always get aligned a??addra??.
>
> Let me know if I miss anything.

Yes, you are right!  Thanks for pointing this out.

Sorry for bothering, please ignore this patch.

Best Regards,
Huang, Ying

> a??
> Best Regards,
> Yan Zi
>
> On 14 Mar 2018, at 21:18, Huang, Ying wrote:
>
>> From: Huang Ying <ying.huang@intel.com>
>>
>> The address argument passed in madvise_free_huge_pmd() may be not THP
>> aligned.  But some THP operations like pmdp_invalidate(),
>> set_pmd_at(), and tlb_remove_pmd_tlb_entry() need the address to be
>> THP aligned.  Fix this via using THP aligned address for these
>> functions in madvise_free_huge_pmd().
>>
>> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
>> Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>> Cc: Michal Hocko <mhocko@suse.com>
>> Cc: Minchan Kim <minchan@kernel.org>
>> Cc: Shaohua Li <shli@kernel.org>
>> Cc: Zi Yan <zi.yan@cs.rutgers.edu>
>> Cc: jglisse@redhat.com
>> Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>> ---
>>  mm/huge_memory.c | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 0cc62405de9c..c5e1bfb08bd7 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -1617,6 +1617,7 @@ bool madvise_free_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
>>  	struct page *page;
>>  	struct mm_struct *mm = tlb->mm;
>>  	bool ret = false;
>> +	unsigned long haddr = addr & HPAGE_PMD_MASK;
>>
>>  	tlb_remove_check_page_size_change(tlb, HPAGE_PMD_SIZE);
>>
>> @@ -1663,12 +1664,12 @@ bool madvise_free_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
>>  	unlock_page(page);
>>
>>  	if (pmd_young(orig_pmd) || pmd_dirty(orig_pmd)) {
>> -		pmdp_invalidate(vma, addr, pmd);
>> +		pmdp_invalidate(vma, haddr, pmd);
>>  		orig_pmd = pmd_mkold(orig_pmd);
>>  		orig_pmd = pmd_mkclean(orig_pmd);
>>
>> -		set_pmd_at(mm, addr, pmd, orig_pmd);
>> -		tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
>> +		set_pmd_at(mm, haddr, pmd, orig_pmd);
>> +		tlb_remove_pmd_tlb_entry(tlb, pmd, haddr);
>>  	}
>>
>>  	mark_page_lazyfree(page);
>> -- 
>> 2.16.1