Re: [PATCH] powerpc/64s/radix: Fix unmapping huge vmaps when CONFIG_HUGETLB_PAGE=n
flat view
From: Nicholas Piggin <npiggin@gmail.com>
Date: 2021-11-26 10:56:03
Excerpts from Daniel Axtens's message of November 26, 2021 4:09 pm:
Hi,quoted
pmd_huge is defined out to false when HUGETLB_PAGE is not configured, but the vmap code still installs huge PMDs. This leads to errors encountering bad PMDs when vunmapping because it is not seen as a huge PTE, and the bad PMD check catches it. The end result may not be much more serious than some bad pmd warning messages, because the pmd_none_or_clear_bad() does what we wanted and clears the huge PTE anyway.Huh. So vmap seems to key off arch_vmap_p?d_supported which checks for radix and HAVE_ARCH_HUGE_VMAP.quoted
Fix this by checking pmd_is_leaf(), which checks for a PTE regardless of config options. The whole huge/large/leaf stuff is a tangled mess but that's kernel-wide and not something we can improve much in arch/powerpc code.I guess I'm a bit late to the party here because p?d_is_leaf was added in 2019 in commit d6eacedd1f0e ("powerpc/book3s: Use config independent helpers for page table walk") but why wouldn't we just make pmd_huge() not config dependent?
I guess so it constant folds code if hugetlbfs is not configured (and maybe so !huge kernels would correctly print a bad PMD warning if they got huge PMD in user mappings).
Also, looking at that commit, there are a few places that might still
throw warnings, e.g. find_linux_pte, find_current_mm_pte, pud_page which
seem like they might still throw warnings if they were to encounter a
huge vmap page:
struct page *pud_page(pud_t pud)
{
if (pud_is_leaf(pud)) {
VM_WARN_ON(!pud_huge(pud));Oh, hmm. That is used in vmalloc.c so maybe that warning should be removed as a false positive. Good catch.
Do these functions need special treatment for huge vmappings()?
find_linux_pte etc could be called for vmaps. I'm not sure I see a problem in that function. Thanks, Nick
Apart from those questions, the patch itself makes sense to me and I can follow how it would fix a problem. Reviewed-by: Daniel Axtens <redacted> Kind regards, Daniel