Hi Matthew,
On 2/13/23 16:50, Matthew Wilcox wrote:
On Mon, Feb 13, 2023 at 04:28:05PM +0100, Alexandre Ghiti wrote:
The issue lies here: before using pteval in pfn_folio(), we should test it.
The following patch fixes the issue for me:
Thanks for debugging it. I'd rather see this written as ...
pte_t pteval = *_pte;
+ unsigned long pfn;
+ if (pte_none(pteval))
+ continue;
+ pfn = pte_pfn(pteval);
+ if (is_zero_pfn(pfn))
+ continue;
+ folio = pfn_folio(pfn);
+ if (folio_test_large(folio))
+ continue;
release_pte_folio(folio);
makes sense?
Sure, that's fine by me, I can send that or I'll add my tested-by on
what you send, whatever suits you.
Alex
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index eb38bd1b1b2f..fef3414b481b 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -514,10 +514,12 @@ static void release_pte_pages(pte_t *pte, pte_t *_pte,
while (--_pte >= pte) {
pte_t pteval = *_pte;
- folio = pfn_folio(pte_pfn(pteval));
- if (!pte_none(pteval) && !is_zero_pfn(pte_pfn(pteval)) &&
- !folio_test_large(folio))
- release_pte_folio(folio);
+ if (!pte_none(pteval) && !is_zero_pfn(pte_pfn(pteval))) {
+ folio = pfn_folio(pte_pfn(pteval));
+
+ if (!folio_test_large(folio))
+ release_pte_folio(folio);
+ }
}
list_for_each_entry_safe(folio, tmp, compound_pagelist, lru) {
@Marek: could you give it a try?
I can send a separate patch if needed, let me know.
Thanks,
Alex
if (!pte_none(pteval) && !is_zero_pfn(pte_pfn(pteval)) &&
- !PageCompound(page))
- release_pte_page(page);
+ !folio_test_large(folio))
+ release_pte_folio(folio);
}
- list_for_each_entry_safe(page, tmp, compound_pagelist, lru) {
- list_del(&page->lru);
- release_pte_page(page);
+ list_for_each_entry_safe(folio, tmp, compound_pagelist, lru) {
+ list_del(&folio->lru);
+ release_pte_folio(folio);
}
}
Best regards
