Messages in this thread | ![/](/images/icornerl.gif) | | Date | Wed, 31 Jan 2024 12:13:05 +0100 | Subject | Re: [PATCH v1 9/9] mm/memory: optimize unmap/zap with PTE-mapped THP | From | David Hildenbrand <> |
| |
>>>> - folio_remove_rmap_pte(folio, page, vma); >>>> + folio_remove_rmap_ptes(folio, page, nr, vma); >>>> + >>>> + /* Only sanity-check the first page in a batch. */ >>>> if (unlikely(page_mapcount(page) < 0)) >>>> print_bad_pte(vma, addr, ptent, page); >>> >>> Is there a case for either removing this all together or moving it into >>> folio_remove_rmap_ptes()? It seems odd to only check some pages. >>> >> >> I really wanted to avoid another nasty loop here. >> >> In my thinking, for 4k folios, or when zapping subpages of large folios, we >> still perform the exact same checks. Only when batching we don't. So if there is >> some problem, there are ways to get it triggered. And these problems are barely >> ever seen. >> >> folio_remove_rmap_ptes() feels like the better place -- especially because the >> delayed-rmap handling is effectively unchecked. But in there, we cannot >> "print_bad_pte()". >> >> [background: if we had a total mapcount -- iow cheap folio_mapcount(), I'd check >> here that the total mapcount does not underflow, instead of checking per-subpage] > > All good points... perhaps extend the comment to describe how this could be > solved in future with cheap total_mapcount()? Or in the commit log if you prefer?
I'll add more meat to the cover letter, thanks!
-- Cheers,
David / dhildenb
| ![\](/images/icornerr.gif) |