On 21.03.25 12:25, Gavin Shan wrote:
On 3/21/25 8:11 PM, David Hildenbrand wrote:
On 21.03.25 10:23, Vlastimil Babka wrote:
On 3/21/25 06:31, Gavin Shan wrote:
Found by code inspection. There are two places where the parameter
passed to page_mapcount_is_type() is (page->__mapcount), which is
correct since it should be one more than the value, as explained in
the comments to page_mapcount_is_type(): (a) page_has_type() in
page-flags.h (b) __dump_folio() in mm/debug.c
IIUC you are right. Luckily thanks to the the PGTY_mapcount_underflow limit,
this off-by-one error doesn't currently cause visible issues i.e.
misclassifications legitimate mapcount as page type and vice versa, right?
We'd have to have a mapcount underflown severely right to the limit to make
that off-by-one error cross it?
Agreed. Likely not stable material because it isn't actually fixing anything (because of the safety gaps).
Yes, it shouldn't cause any visible impacts so far due to the gap.
I just found the issue by code inspection. Lets drop the fix tags
in v2.
I wonder if a more future-proof solution would be to redefine
page_mapcount_is_type() instead to not subtract. But I'll leave that to willy.
With upcoming changes around that, likely best to leave that alone. I expect page_mapcount_is_type() to completely vanish.
+1 to remove page_mapcount_is_type(). After Willy confirms, I can post
an extra series to do it if needed.
I think we should only do that one Willy splits struct folio off from,
struct page, storing the type elsewhere. For now, we should likely just
leave it as is.
--
Cheers,
David / dhildenb
