On Thu, Nov 16, 2017 at 5:24 PM, Aneesh Kumar K.V
<aneesh.ku...@linux.vnet.ibm.com> wrote:
> Balbir Singh <bsinghar...@gmail.com> writes:
>
> + address = start;
>> + do {
>> + local_irq_disable();
>> + find_linux_pte(mm->pgd, address, &is_thp, &hshift);
>> + if (!is_thp)
>> + shift = PAGE_SHIFT;
>
> It can still be hugetlb if is_thp is false.
>
Yep, the hshift check needs to be upfront, the version below makes sense.
>> + else if (hshift && !is_thp)
>> + shift = hshift;
>> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>> + else
>> + shift = HPAGE_PMD_SIZE;
>
> That is wrong. I guess it should be shift = HPAGE_PMD_SHIFT. But i am
> not sure we need to make it this complex at all. See below.
>
>> +#else
>> + else {
>> + shift = PAGE_SHIFT;
>> + pr_warn_once("unsupport page size for mm %p,addr
>> %lx\n",
>> + mm, start);
>> + }
>> +#endif
>
> I am still not sure this is correct from a pure page table walking
> point. Why not
>
> if (hshift)
> shift = hshift;
> else
> shift = PAGE_SHIFT;
>
I don't think I care about THP at this point
I'll respin
Balbir
