On 30.07.24 18:08, Marek Szyprowski wrote:
On 30.07.2024 17:49, David Hildenbrand wrote:
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@xxxxxxxxxx>
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?
Yes, highmem is enabled on those boards and all of them have 1GB+ of
RAM. For other kernel configuration options see
arch/arm/configs/exynos_defconfig.
Yes, pretty sure that's it. virt_to_page() won't work on kmaped pages.
So looks like we cannot easily do the conversion in this patch. :(
We'll have to get hacky in patch #2 instead.
@Andrew, can you drop both patches for now? I'll send a v2 that
essentially does in v2 on top something like:
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index da800e56fe590..c2e330b1eee21 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -963,7 +963,13 @@ static inline spinlock_t *huge_pte_lockptr(struct hstate *h,
* will give us the same result: the per-MM PT lock.
*/
if (huge_page_size(h) < PMD_SIZE)
- return pte_lockptr(mm, pte);
+ /*
+ * pte_lockptr() needs the PMD, which we don't have. Because of
+ * highmem we cannot convert pte_lockptr() to consume a pte.
+ * But as we never have highmem page tables in hugetlb, we can
+ * safely use virt_to_ptdesc() here.
+ */
+ return ptlock_ptr(virt_to_ptdesc(pte));
else if (huge_page_size(h) < PUD_SIZE)
return pmd_lockptr(mm, (pmd_t *) pte);
return pud_lockptr(mm, (pud_t *) pte);
--
Cheers,
David / dhildenb
