Re: [RFC V2 1/6] loongarch/mm: Stop using pte_ERROR()

On Wed, Aug 12, 2026 at 01:27:54PM +0200, David Hildenbrand (Arm) wrote:
> On 8/12/26 13:19, David Hildenbrand (Arm) wrote:
> > On 8/11/26 06:21, Anshuman Khandual wrote:
> >> Directly use pr_err() in __set_fixmap() and drop pte_ERROR() which helps in
> >> eventually dropping pte_ERROR() macro across the tree. In this new printing
> >> __FILE__ and __LINE__ has been dropped because they are always the same and
> >> don't really add any value.
> >>
> >> Cc: Huacai Chen <[email protected]>
> >> Cc: WANG Xuerui <[email protected]>
> >> Cc: [email protected]
> >> Cc: [email protected]
> >> Signed-off-by: Anshuman Khandual <[email protected]>
> >> ---
> >>  arch/loongarch/include/asm/pgtable.h | 2 --
> >>  arch/loongarch/mm/init.c             | 2 +-
> >>  2 files changed, 1 insertion(+), 3 deletions(-)
> >>
> >> diff --git a/arch/loongarch/include/asm/pgtable.h b/arch/loongarch/include/asm/pgtable.h
> >> index 223528c04d73..a32a8fff70c1 100644
> >> --- a/arch/loongarch/include/asm/pgtable.h
> >> +++ b/arch/loongarch/include/asm/pgtable.h
> >> @@ -128,8 +128,6 @@ struct vm_area_struct;
> >>  #define ptep_get(ptep) READ_ONCE(*(ptep))
> >>  #define pmdp_get(pmdp) READ_ONCE(*(pmdp))
> >>  
> >> -#define pte_ERROR(e) \
> >> -	pr_err("%s:%d: bad pte %016lx.\n", __FILE__, __LINE__, pte_val(e))
> >>  #ifndef __PAGETABLE_PMD_FOLDED
> >>  #define pmd_ERROR(e) \
> >>  	pr_err("%s:%d: bad pmd %016lx.\n", __FILE__, __LINE__, pmd_val(e))
> >> diff --git a/arch/loongarch/mm/init.c b/arch/loongarch/mm/init.c
> >> index 3407030f3e7a..be9b127daef2 100644
> >> --- a/arch/loongarch/mm/init.c
> >> +++ b/arch/loongarch/mm/init.c
> >> @@ -203,7 +203,7 @@ void __init __set_fixmap(enum fixed_addresses idx,
> >>  
> >>  	ptep = populate_kernel_pte(addr);
> >>  	if (!pte_none(ptep_get(ptep))) {
> >> -		pte_ERROR(*ptep);
> >> +		pr_err("bad pte %016lx\n", pte_val(*ptep));
> >>  		return;
> >>  	}
> >>  
> > 
> > Ah, I missed that we want to print actual values, I somehow thought that we
> > would be printing pte_none() ...
> > 
> > Maybe we should just be more verbose while at it?

Initially thought that these new prints should be right close to the
original via pxd_ERROR() but I guess making them verbose while doing
the replacements should be indeed acceptable.

> > 
> > pr_err("unexpected set PTE at %ul in __set_fixmap: %016lx\n",
> >        addr, pte_val(*ptep));
> > 
> 
> On second thought, why not simply use ptval_to_str() here as well?

Sure will do the required changes - the following builds successfully.

--- a/arch/loongarch/mm/init.c
+++ b/arch/loongarch/mm/init.c
@@ -197,13 +197,15 @@ void __init __set_fixmap(enum fixed_addresses idx,
                               phys_addr_t phys, pgprot_t flags)
 {
        unsigned long addr = __fix_to_virt(idx);
+       char str[PTVAL_STR_MAX];
        pte_t *ptep;

        BUG_ON(idx <= FIX_HOLE || idx >= __end_of_fixed_addresses);

        ptep = populate_kernel_pte(addr);
        if (!pte_none(ptep_get(ptep))) {
-               pr_err("bad pte %016lx\n", pte_val(*ptep));
+               ptval_to_str(str, pte_val(*ptep));
+               pr_err("unexpected set PTE at %lx in __set_fixmap: %s\n", addr, str);
                return;
        }

> 
> -- 
> Cheers,
> 
> David