On 08/04/2025 19:40, Matthew Wilcox wrote: > On Tue, Apr 08, 2025 at 09:54:42AM -0700, Dave Hansen wrote: >> On 4/8/25 09:37, Matthew Wilcox wrote: >>> On Tue, Apr 08, 2025 at 08:22:47AM -0700, Dave Hansen wrote: >>>> Are there any tests for folio_test_pgtable() at free_page() time? If we >>>> had that, it would make it less likely that another free_page() user >>>> could sneak in without calling the destructor. >>> It's hidden, but yes: >>> >>> static inline bool page_expected_state(struct page *page, >>> unsigned long check_flags) >>> { >>> if (unlikely(atomic_read(&page->_mapcount) != -1)) >>> return false; >>> >>> PageTable uses page_type which aliases with mapcount, so this check >>> covers "PageTable is still set when the last refcount to it is put". >> Huh, so shouldn't we have ended up in bad_page() for these, other than: >> >> pagetable_dtor(virt_to_ptdesc(pmd)); >> free_page((unsigned long)pmd); > I think at this point in Kevin's series, we don't call the ctor for > these pages, so we never set PageTable() on them. I could be wrong;
Correct, that's why I added this patch early in the series (the next patch adds the ctor call in pte_alloc_one_kernel()). The BUG() in v1 was indeed triggered by a page_expected_state() check [1]. > as Kevin says, this is all very twisty and confusing with exceptions and > exceptions to exceptions. This series should reduce the confusion. I hope so! - Kevin [1] https://lore.kernel.org/oe-lkp/202503211612.e11bd73f-...@intel.com/