On 3/21/25 9:33 PM, Vlastimil Babka wrote:
On 3/21/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.
Thanks for confirming, please state that in the commit log/cover letter too.
Yes, the commit log and cover letter has been improved for this in v2.
I just found the issue by code inspection. Lets drop the fix tags
in v2.
Fixes: tag is fine and correct, just Cc: stable is unnecessary.
Thanks for the hints. The 'Cc: stable' tag has been dropped, but the
'Fixes:' tag is kept in v2, which was posted.
https://lore.kernel.org/linux-mm/20250321120222.1456770-1-gshan@xxxxxxxxxx/T/#t
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.
Thanks,
Gavin
