Re: [PATCH V2] mm: Standardize printing for pgtable entries

[Ryan Roberts <ryan.roberts@arm.com>]

On 08/07/2026 04:28, Anshuman Khandual wrote:
> From: "David Hildenbrand (Arm)" <david@kernel.org>
> 
> Bad page map reporting currently stores page table entry values in an
> unsigned long long and prints them with fixed 64-bit-oriented format
> strings. This is inconsistent across call sites and does not work well for
> architectures where page table entry values are not naturally represented
> as 64-bit values, such as 32-bit or 128-bit entries.
> 
> Introduce a common helper to convert raw page table entry values into a
> fixed-width hexadecimal string based on the actual entry size. Use it for
> bad page map reporting and for dumping the page table walk in
> __print_bad_page_map_pgtable().
> 
> Pass page table entry values to the reporting path as raw bytes together
> with their size, instead of forcing them through an unsigned long long.
> It keeps the printed output consistent and avoids truncation or misleading
> formatting for non-64-bit page table entries.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> This patch applies on v7.2-rc2
> 
> Changes in V2:
> 
> - Dropped space after ":" during print per Matthew
> - Dropped CONFIG_CPU_BIG_ENDIAN per David
> 
> Changes in V1:
> 
> https://lore.kernel.org/all/20260707041703.658021-1-anshuman.khandual@arm.com/
> 
>  mm/memory.c | 98 ++++++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 75 insertions(+), 23 deletions(-)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index ff338c2abe92..a2b63af82792 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -519,9 +519,48 @@ static bool is_bad_page_map_ratelimited(void)
>  	return false;
>  }
>  
> +#define PTVAL_STR_MAX	(32 + 1) /* Max 128-bit value in hex + NUL */

Not sure if it's worth doing something like this?:

#define PTVAL_STR_MAX						\
	(MAX(MAX(MAX(MAX(sizeof(pteval_t), sizeof(pmdval_t)), 	\
	    	     sizeof(pudval_t)), 			\
		 sizeof(p4dval_t)), 				\
    	     sizeof(pgdval_t)) + 1)

Would probably save stack space for 32bit arches?

> +
> +static void ptval_bytes_to_hex_str(char *buf, size_t buf_size, const void *entry, size_t entry_size)
> +{
> +	if (WARN_ON_ONCE(buf_size < entry_size * 2 + 1)) {
> +		snprintf(buf, buf_size, "overflow");
> +		return;
> +	}
> +
> +	switch (entry_size) {
> +	case sizeof(u32):
> +		snprintf(buf, buf_size, "%08x", *(const u32 *)entry);
> +		break;
> +	case sizeof(u64):
> +		snprintf(buf, buf_size, "%016llx", *(const u64 *)entry);
> +		break;
> +#if defined(__SIZEOF_INT128__)
> +	case sizeof(u128):
> +		snprintf(buf, buf_size, "%016llx%016llx",
> +			 (unsigned long long)(*(const u128 *)entry >> 64),
> +			 (unsigned long long)*(const u128 *)entry);
> +		break;
> +#endif
> +	default:
> +		snprintf(buf, buf_size, "unsupported");
> +		break;
> +	}
> +}
> +
> +#define ptval_to_str(buf, val)								\
> +	do {										\
> +		__auto_type __val = (val);						\
> +											\
> +		ptval_bytes_to_hex_str((buf), sizeof(buf), &__val, sizeof(__val));	\
> +	} while (0)

I think arm64 code also does pte printing, which you also need to fix up for
D128 support. Perhaps this could be moved to a header for reuse?

Also not sure if it's worth returning buf so that this pattern would be possible:

char pmd_str[PTVAL_STR_MAX];
pr_err("pmd=%s\n, ptval_to_str(pmd_str, pmd_val(*pmdp));

Although perhaps that's a bit busy and should be discouraged...

Thanks,
Ryan


> +
>  static void __print_bad_page_map_pgtable(struct mm_struct *mm, unsigned long addr)
>  {
> -	unsigned long long pgdv, p4dv, pudv, pmdv;
> +	char pgd_str[PTVAL_STR_MAX];
> +	char p4d_str[PTVAL_STR_MAX];
> +	char pud_str[PTVAL_STR_MAX];
> +	char pmd_str[PTVAL_STR_MAX];
>  	p4d_t p4d, *p4dp;
>  	pud_t pud, *pudp;
>  	pmd_t pmd, *pmdp;
> @@ -532,34 +571,34 @@ static void __print_bad_page_map_pgtable(struct mm_struct *mm, unsigned long add
>  	 * see locking requirements for print_bad_page_map().
>  	 */
>  	pgdp = pgd_offset(mm, addr);
> -	pgdv = pgd_val(*pgdp);
> +	ptval_to_str(pgd_str, pgd_val(*pgdp));
>  
>  	if (!pgd_present(*pgdp) || pgd_leaf(*pgdp)) {
> -		pr_alert("pgd:%08llx\n", pgdv);
> +		pr_alert("pgd:%s\n", pgd_str);
>  		return;
>  	}
>  
>  	p4dp = p4d_offset(pgdp, addr);
>  	p4d = p4dp_get(p4dp);
> -	p4dv = p4d_val(p4d);
> +	ptval_to_str(p4d_str, p4d_val(p4d));
>  
>  	if (!p4d_present(p4d) || p4d_leaf(p4d)) {
> -		pr_alert("pgd:%08llx p4d:%08llx\n", pgdv, p4dv);
> +		pr_alert("pgd:%s p4d:%s\n", pgd_str, p4d_str);
>  		return;
>  	}
>  
>  	pudp = pud_offset(p4dp, addr);
>  	pud = pudp_get(pudp);
> -	pudv = pud_val(pud);
> +	ptval_to_str(pud_str, pud_val(pud));
>  
>  	if (!pud_present(pud) || pud_leaf(pud)) {
> -		pr_alert("pgd:%08llx p4d:%08llx pud:%08llx\n", pgdv, p4dv, pudv);
> +		pr_alert("pgd:%s p4d:%s pud:%s\n", pgd_str, p4d_str, pud_str);
>  		return;
>  	}
>  
>  	pmdp = pmd_offset(pudp, addr);
>  	pmd = pmdp_get(pmdp);
> -	pmdv = pmd_val(pmd);
> +	ptval_to_str(pmd_str, pmd_val(pmd));
>  
>  	/*
>  	 * Dumping the PTE would be nice, but it's tricky with CONFIG_HIGHPTE,
> @@ -567,8 +606,7 @@ static void __print_bad_page_map_pgtable(struct mm_struct *mm, unsigned long add
>  	 * doing another map would be bad. print_bad_page_map() should
>  	 * already take care of printing the PTE.
>  	 */
> -	pr_alert("pgd:%08llx p4d:%08llx pud:%08llx pmd:%08llx\n", pgdv,
> -		 p4dv, pudv, pmdv);
> +	pr_alert("pgd:%s p4d:%s pud:%s pmd:%s\n", pgd_str, p4d_str, pud_str, pmd_str);
>  }
>  
>  /*
> @@ -584,10 +622,11 @@ static void __print_bad_page_map_pgtable(struct mm_struct *mm, unsigned long add
>   * page table lock.
>   */
>  static void print_bad_page_map(struct vm_area_struct *vma,
> -		unsigned long addr, unsigned long long entry, struct page *page,
> -		enum pgtable_level level)
> +		unsigned long addr, const void *entry, size_t entry_size,
> +		struct page *page, enum pgtable_level level)
>  {
>  	struct address_space *mapping;
> +	char entry_str[PTVAL_STR_MAX];
>  	pgoff_t index;
>  
>  	if (is_bad_page_map_ratelimited())
> @@ -596,8 +635,9 @@ static void print_bad_page_map(struct vm_area_struct *vma,
>  	mapping = vma->vm_file ? vma->vm_file->f_mapping : NULL;
>  	index = linear_page_index(vma, addr);
>  
> -	pr_alert("BUG: Bad page map in process %s  %s:%08llx", current->comm,
> -		 pgtable_level_to_str(level), entry);
> +	ptval_bytes_to_hex_str(entry_str, sizeof(entry_str), entry, entry_size);
> +	pr_alert("BUG: Bad page map in process %s  %s:%s", current->comm,
> +		 pgtable_level_to_str(level), entry_str);
>  	__print_bad_page_map_pgtable(vma->vm_mm, addr);
>  	if (page)
>  		dump_page(page, "bad page map");
> @@ -627,8 +667,13 @@ static inline bool pgtable_level_has_pxx_special(enum pgtable_level level)
>  	}
>  }
>  
> -#define print_bad_pte(vma, addr, pte, page) \
> -	print_bad_page_map(vma, addr, pte_val(pte), page, PGTABLE_LEVEL_PTE)
> +static void print_bad_pte(struct vm_area_struct *vma, unsigned long addr,
> +			  pte_t pte, struct page *page)
> +{
> +	__auto_type entry = pte_val(pte);
> +
> +	print_bad_page_map(vma, addr, &entry, sizeof(entry), page, PGTABLE_LEVEL_PTE);
> +}
>  
>  /**
>   * __vm_normal_page() - Get the "struct page" associated with a page table entry.
> @@ -636,8 +681,9 @@ static inline bool pgtable_level_has_pxx_special(enum pgtable_level level)
>   * @addr: The address where the page table entry is mapped.
>   * @pfn: The PFN stored in the page table entry.
>   * @special: Whether the page table entry is marked "special".
> - * @level: The page table level for error reporting purposes only.
>   * @entry: The page table entry value for error reporting purposes only.
> + * @entry_size: The size of @entry.
> + * @level: The page table level for error reporting purposes only.
>   *
>   * "Special" mappings do not wish to be associated with a "struct page" (either
>   * it doesn't exist, or it exists but they don't want to touch it). In this
> @@ -697,7 +743,7 @@ static inline bool pgtable_level_has_pxx_special(enum pgtable_level level)
>   */
>  static inline struct page *__vm_normal_page(struct vm_area_struct *vma,
>  		unsigned long addr, unsigned long pfn, bool special,
> -		unsigned long long entry, enum pgtable_level level)
> +		const void *entry, size_t entry_size, enum pgtable_level level)
>  {
>  	if (pgtable_level_has_pxx_special(level)) {
>  		if (unlikely(special)) {
> @@ -710,7 +756,7 @@ static inline struct page *__vm_normal_page(struct vm_area_struct *vma,
>  			if (is_zero_pfn(pfn) || is_huge_zero_pfn(pfn))
>  				return NULL;
>  
> -			print_bad_page_map(vma, addr, entry, NULL, level);
> +			print_bad_page_map(vma, addr, entry, entry_size, NULL, level);
>  			return NULL;
>  		}
>  		/*
> @@ -741,7 +787,7 @@ static inline struct page *__vm_normal_page(struct vm_area_struct *vma,
>  
>  	if (unlikely(pfn > highest_memmap_pfn)) {
>  		/* Corrupted page table entry. */
> -		print_bad_page_map(vma, addr, entry, NULL, level);
> +		print_bad_page_map(vma, addr, entry, entry_size, NULL, level);
>  		return NULL;
>  	}
>  	/*
> @@ -767,8 +813,10 @@ static inline struct page *__vm_normal_page(struct vm_area_struct *vma,
>  struct page *vm_normal_page(struct vm_area_struct *vma, unsigned long addr,
>  			    pte_t pte)
>  {
> +	__auto_type entry = pte_val(pte);
> +
>  	return __vm_normal_page(vma, addr, pte_pfn(pte), pte_special(pte),
> -				pte_val(pte), PGTABLE_LEVEL_PTE);
> +				&entry, sizeof(entry), PGTABLE_LEVEL_PTE);
>  }
>  
>  /**
> @@ -809,8 +857,10 @@ struct folio *vm_normal_folio(struct vm_area_struct *vma, unsigned long addr,
>  struct page *vm_normal_page_pmd(struct vm_area_struct *vma, unsigned long addr,
>  				pmd_t pmd)
>  {
> +	__auto_type entry = pmd_val(pmd);
> +
>  	return __vm_normal_page(vma, addr, pmd_pfn(pmd), pmd_special(pmd),
> -				pmd_val(pmd), PGTABLE_LEVEL_PMD);
> +				&entry, sizeof(entry), PGTABLE_LEVEL_PMD);
>  }
>  
>  /**
> @@ -850,8 +900,10 @@ struct folio *vm_normal_folio_pmd(struct vm_area_struct *vma,
>  struct page *vm_normal_page_pud(struct vm_area_struct *vma,
>  		unsigned long addr, pud_t pud)
>  {
> +	__auto_type entry = pud_val(pud);
> +
>  	return __vm_normal_page(vma, addr, pud_pfn(pud), pud_special(pud),
> -				pud_val(pud), PGTABLE_LEVEL_PUD);
> +				&entry, sizeof(entry), PGTABLE_LEVEL_PUD);
>  }
>  #endif
>