Re: [PATCH v1 1/2] mm: let pte_lockptr() consume a pte_t pointer

[David Hildenbrand <david@redhat.com>]

On 30.07.24 17:45, David Hildenbrand wrote:
> On 30.07.24 17:30, Marek Szyprowski wrote:
>> On 25.07.2024 20:39, David Hildenbrand wrote:
>>> pte_lockptr() is the only *_lockptr() function that doesn't consume
>>> what would be expected: it consumes a pmd_t pointer instead of a pte_t
>>> pointer.
>>>
>>> Let's change that. The two callers in pgtable-generic.c are easily
>>> adjusted. Adjust khugepaged.c:retract_page_tables() to simply do a
>>> pte_offset_map_nolock() to obtain the lock, even though we won't actually
>>> be traversing the page table.
>>>
>>> This makes the code more similar to the other variants and avoids other
>>> hacks to make the new pte_lockptr() version happy. pte_lockptr() users
>>> reside now only in  pgtable-generic.c.
>>>
>>> Maybe, using pte_offset_map_nolock() is the right thing to do because
>>> the PTE table could have been removed in the meantime? At least it sounds
>>> more future proof if we ever have other means of page table reclaim.
>>>
>>> It's not quite clear if holding the PTE table lock is really required:
>>> what if someone else obtains the lock just after we unlock it? But we'll
>>> leave that as is for now, maybe there are good reasons.
>>>
>>> This is a preparation for adapting hugetlb page table locking logic to
>>> take the same locks as core-mm page table walkers would.
>>>
>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>
>> This patch landed in today's linux-next as commit e98970a1d2d4 ("mm: let
>> pte_lockptr() consume a pte_t pointer"). Unfortunately it causes the
>> following issue on most of my ARM 32bit based test boards:
>>
> 
> That is ... rather surprising.
> 
> The issue below seems to point at __pte_offset_map_lock(), where we
> essentially convert from
> 
> ptlock_ptr(page_ptdesc(pmd_page(*pmd)));
> 
> to
> 
> ptlock_ptr(virt_to_ptdesc(pte));

I'm wondering, is highmem involved here such that the PTE would be 
kmap'ed and virt_to_page() would not do what we would expect it to do?

-- 
Cheers,

David / dhildenb