Re: [PATCH] powerpc/npu: Cleanup MMIO ATSD flushing

[Balbir Singh <bsingharora@gmail.com>]

On Thu, Nov 16, 2017 at 5:24 PM, Aneesh Kumar K.V
<aneesh.kumar@linux.vnet.ibm.com> wrote:
> Balbir Singh <bsingharora@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