On 2024/11/22 19:12, David Hildenbrand wrote:
> Ever since kernel commit 4ffca5a96678c ("mm: support only one page_type
> per page"), page types are no longer flags (PG_slab, PG_offline, ...)
> stored in page->_mapcount, but values stored in the top byte.
>
> Because we currently try deriving flags from the mapcount values to
> check for slab and hugetlb pages, we get:
> (1) "false positives" from isSlab(), making us not detect free (buddy)
> pages and offline pages anymore.
> (2) "false positives" when detecting hugetlb pages.
>
> In the common case we now simply dump all memory, and fail to exclude
> offline and free (buddy) pages, assuming they are all slab pages,
> which is bad.
>
> We should just consistently compare the page->_mapcount with the unmodified
> PAGE_*_MAPCOUNT_VALUE, like we already did for free (buddy) and offline
> pages already. This also works for older kernels, because the kernel never
> supported having multiple page types set on a single page, so there was
> never the need to derive flags from the PAGE_*_MAPCOUNT_VALUE.
>
> It is worth noting that the lower 24bit of the page->_mapcount field
> can be used while a page type is set. This is, however, currently not
> the case for any of the involved page types (slab, buddy, offline,
Hi David,
thank you for the patch and information, it's very helpful. I had
thought that we should derive flags as it was implemented as flags..
The patch looks good and tested ok with 6.12 and older kernels, applied.
https://github.com/makedumpfile/makedumpfile/commit/72c3414949bd2e9284c8ba7ceaf109a5a34ac2e1
Thanks,
Kazu
> hugetlb). In the future, the kernel could either just tell us the types,
> or provide a mask to be applied to the page->_mapcount ("bits to ignore")
> when comparing the values. But, there might be bigger changes coming up
> with the "memdesc" work in the kernel, where the type would not longer
> be stored in page->_mapcount ... so for now we can keep it simple.
>
> Link: https://github.com/makedumpfile/makedumpfile/issues/16
> Cc: Masamitsu Yamazaki <yamazaki-msmt@xxxxxxx>
> Cc: Kazuhito Hagio <k-hagio-ab@xxxxxxx>
> Cc: Matthew Wilcox <willy@xxxxxxxxxxxxx>
> Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
> ---
> makedumpfile.c | 6 ++----
> makedumpfile.h | 2 --
> 2 files changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/makedumpfile.c b/makedumpfile.c
> index b356eb3..bad3c48 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -280,8 +280,7 @@ isSlab(unsigned long flags, unsigned int _mapcount)
> {
> /* Linux 6.10 and later */
> if (NUMBER(PAGE_SLAB_MAPCOUNT_VALUE) != NOT_FOUND_NUMBER) {
> - unsigned int PG_slab = ~NUMBER(PAGE_SLAB_MAPCOUNT_VALUE);
> - if ((_mapcount & (PAGE_TYPE_BASE | PG_slab)) == PAGE_TYPE_BASE)
> + if (_mapcount == (int)NUMBER(PAGE_SLAB_MAPCOUNT_VALUE))
> return TRUE;
> }
>
> @@ -6549,11 +6548,10 @@ __exclude_unnecessary_pages(unsigned long mem_map,
> */
> if (NUMBER(PAGE_HUGETLB_MAPCOUNT_VALUE) != NOT_FOUND_NUMBER) {
> unsigned long _flags_1 = ULONG(addr + OFFSET(page.flags));
> - unsigned int PG_hugetlb = ~NUMBER(PAGE_HUGETLB_MAPCOUNT_VALUE);
>
> compound_order = _flags_1 & 0xff;
>
> - if ((_mapcount & (PAGE_TYPE_BASE | PG_hugetlb)) == PAGE_TYPE_BASE)
> + if (_mapcount == (int)NUMBER(PAGE_HUGETLB_MAPCOUNT_VALUE))
> compound_dtor = IS_HUGETLB;
>
> goto check_order;
> diff --git a/makedumpfile.h b/makedumpfile.h
> index 7ed566d..2b3495e 100644
> --- a/makedumpfile.h
> +++ b/makedumpfile.h
> @@ -164,8 +164,6 @@ test_bit(int nr, unsigned long addr)
> #define isAnon(mapping, flags, _mapcount) \
> (((unsigned long)mapping & PAGE_MAPPING_ANON) != 0 && !isSlab(flags, _mapcount))
>
> -#define PAGE_TYPE_BASE (0xf0000000)
> -
> #define PTOB(X) (((unsigned long long)(X)) << PAGESHIFT())
> #define BTOP(X) (((unsigned long long)(X)) >> PAGESHIFT())
>
