On 08/12/2020 07:22 PM, Aneesh Kumar K.V wrote:
> On 8/12/20 7:04 PM, Anshuman Khandual wrote:
>>
>>
>> On 08/12/2020 06:46 PM, Aneesh Kumar K.V wrote:
>>> On 8/12/20 6:33 PM, Anshuman Khandual wrote:
>>>>
>>>>
>>>> On 08/12/2020 12:03 PM, Aneesh Kumar K.V wrote:
>>>>> The seems to be missing quite a lot of details w.r.t allocating
>>>>> the correct pgtable_t page (huge_pte_alloc()), holding the right
>>>>> lock (huge_pte_lock()) etc. The vma used is also not a hugetlb VMA.
>>>>>
>>>>> ppc64 do have runtime checks within CONFIG_DEBUG_VM for most of these.
>>>>> Hence disable the test on ppc64.
>>>>
>>>> This test is free from any platform specific #ifdefs which should
>>>> never be broken. If hugetlb_advanced_tests() does not work or is
>>>> not detailed enough for ppc64, then it would be great if you could
>>>> suggest some improvements so that it works for all enabled platforms.
>>>>
>>>>
>>>
>>> As mentioned the test is broken. For hugetlb, the pgtable_t pages should be
>>> allocated by huge_pte_alloc(). We need to hold huget_pte_lock() beforeĀ
>>> updating huge tlb pte. That takes hugepage size, which is mostly derived
>>> out of vma. Hence vma need to be a hugetlb vma. Some of the functions also
>>> depend on hstate. Also we should use set_huge_pte_at() when setting up
>>> hugetlb pte entries. I was tempted to remove that test completely marking
>>> it broken. But avoided that by marking it broken on only PPC64.
>>
>> The test is not broken, hugetlb helpers on multiple platforms dont complain
>> about
>> this at all. The tests here emulate 'enough' MM objects required for the
>> helpers
>> on enabled platforms, to perform the primary task i.e page table
>> transformation it
>> is expected to do. The test does not claim to emulate a perfect MM
>> environment for
>> a given subsystem's (like HugeTLB) arch helpers. Now in this case, the MM
>> objects
>> being emulated for the HugeTLB advanced tests does not seem to be sufficient
>> for
>> ppc64 but it can be improved. But that does not mean it is broken in it's
>> current
>> form for other platforms.
>>
>
> There is nothing ppc64 specific here. It is just that we have CONFIG_DEBUG_VM
> based checks for different possibly wrong usages of these functions. This was
> done because we have different page sizes, two different translations to
> support and we want to avoid any wrong usage. IMHO expecting hugetlb page
> table helpers to work with a non hugetlb VMA andĀ without holding hugeTLB pte
> lock is a clear violation of hugetlb interface.
Do you have a modified version of the test with HugeTLB marked VMA and with pte
lock
held, which works on ppc664 ?