[PATCH 1/1] LoongArch: mm: remove redundant pte occupancy check in __set_fixmap()

[PATCH 1/1] LoongArch: mm: remove redundant pte occupancy check in __set_fixmap()

[george]

From: George Guo <guodongtai@kylinos.cn>

This check could falsely report errors when PTEs are remapped:
arch/loongarch/mm/init.c:204: bad pte 0000000107b601c3.

The check was unnecessary because:
- Fixmap slots are designed to be reusable
- Other architectures (x86/arm64) allow PTE overwrites
- Callers are responsible for proper mapping management

Signed-off-by: George Guo <guodongtai@kylinos.cn>
---
 arch/loongarch/mm/init.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/loongarch/mm/init.c b/arch/loongarch/mm/init.c
index 06f11d9e4ec1..7b7e3b1aa00d 100644
--- a/arch/loongarch/mm/init.c
+++ b/arch/loongarch/mm/init.c
@@ -200,10 +200,6 @@ void __init __set_fixmap(enum fixed_addresses idx,
 	BUG_ON(idx <= FIX_HOLE || idx >= __end_of_fixed_addresses);
 
 	ptep = populate_kernel_pte(addr);
-	if (!pte_none(ptep_get(ptep))) {
-		pte_ERROR(*ptep);
-		return;
-	}
 
 	if (pgprot_val(flags))
 		set_pte(ptep, pfn_pte(phys >> PAGE_SHIFT, flags));
-- 
2.34.1

Re: [PATCH 1/1] LoongArch: mm: remove redundant pte occupancy check in __set_fixmap()

[Yanteng Si]

在 5/8/25 11:05 AM, george 写道:
> From: George Guo <guodongtai@kylinos.cn>
This is unnecessary. Please remove it.

Thanks,
Yanteng
> 
> This check could falsely report errors when PTEs are remapped:
> arch/loongarch/mm/init.c:204: bad pte 0000000107b601c3.
> 
> The check was unnecessary because:
> - Fixmap slots are designed to be reusable
> - Other architectures (x86/arm64) allow PTE overwrites
> - Callers are responsible for proper mapping management
> 
> Signed-off-by: George Guo <guodongtai@kylinos.cn>
> ---
>   arch/loongarch/mm/init.c | 4 ----
>   1 file changed, 4 deletions(-)
> 
> diff --git a/arch/loongarch/mm/init.c b/arch/loongarch/mm/init.c
> index 06f11d9e4ec1..7b7e3b1aa00d 100644
> --- a/arch/loongarch/mm/init.c
> +++ b/arch/loongarch/mm/init.c
> @@ -200,10 +200,6 @@ void __init __set_fixmap(enum fixed_addresses idx,
>   	BUG_ON(idx <= FIX_HOLE || idx >= __end_of_fixed_addresses);
>   
>   	ptep = populate_kernel_pte(addr);
> -	if (!pte_none(ptep_get(ptep))) {
> -		pte_ERROR(*ptep);
> -		return;
> -	}
>   
>   	if (pgprot_val(flags))
>   		set_pte(ptep, pfn_pte(phys >> PAGE_SHIFT, flags));

Re: [PATCH 1/1] LoongArch: mm: remove redundant pte occupancy check in __set_fixmap()

[Andrew Morton]

On Thu, 8 May 2025 13:23:38 +0800 Yanteng Si <si.yanteng@linux.dev> wrote:

> 在 5/8/25 11:05 AM, george 写道:
> > From: George Guo <guodongtai@kylinos.cn>
> This is unnecessary. Please remove it.

Why?

I believe it is necessary.  Without this line, the patch will be
committed as

	From: george <dongtai.guo@linux.dev>

- different display name
- different email address
- email address doesn't match signed-off-by

Re: [PATCH 1/1] LoongArch: mm: remove redundant pte occupancy check in __set_fixmap()

[Yanteng Si]

在 5/8/25 1:42 PM, Andrew Morton 写道:
> On Thu, 8 May 2025 13:23:38 +0800 Yanteng Si <si.yanteng@linux.dev> wrote:
> 
>> 在 5/8/25 11:05 AM, george 写道:
>>> From: George Guo <guodongtai@kylinos.cn>
>> This is unnecessary. Please remove it.
> 
> Why?
> 
> I believe it is necessary.  Without this line, the patch will be
> committed as
> 
> 	From: george <dongtai.guo@linux.dev>
> 
> - different display name
> - different email address
> - email address doesn't match signed-off-by
> 
OK, I see. It is necessary.

Thanks,
Yanteng

Re: [PATCH 1/1] LoongArch: mm: remove redundant pte occupancy check in __set_fixmap()

[Huacai Chen]

Hi, George,

On Thu, May 8, 2025 at 11:05 AM george <dongtai.guo@linux.dev> wrote:
>
> From: George Guo <guodongtai@kylinos.cn>
>
> This check could falsely report errors when PTEs are remapped:
> arch/loongarch/mm/init.c:204: bad pte 0000000107b601c3.
>
> The check was unnecessary because:
> - Fixmap slots are designed to be reusable
> - Other architectures (x86/arm64) allow PTE overwrites
> - Callers are responsible for proper mapping management
>
> Signed-off-by: George Guo <guodongtai@kylinos.cn>
> ---
>  arch/loongarch/mm/init.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/arch/loongarch/mm/init.c b/arch/loongarch/mm/init.c
> index 06f11d9e4ec1..7b7e3b1aa00d 100644
> --- a/arch/loongarch/mm/init.c
> +++ b/arch/loongarch/mm/init.c
> @@ -200,10 +200,6 @@ void __init __set_fixmap(enum fixed_addresses idx,
>         BUG_ON(idx <= FIX_HOLE || idx >= __end_of_fixed_addresses);
>
>         ptep = populate_kernel_pte(addr);
> -       if (!pte_none(ptep_get(ptep))) {
> -               pte_ERROR(*ptep);
> -               return;
> -       }
Resolve what problems? If other patches need this change, please
submit them as a series.

Huacai

>
>         if (pgprot_val(flags))
>                 set_pte(ptep, pfn_pte(phys >> PAGE_SHIFT, flags));
> --
> 2.34.1
>
>

Re: [PATCH 1/1] LoongArch: mm: remove redundant pte occupancy check in __set_fixmap()

[George Guo]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=GB18030, Size: 4360 bytes --]

On Sat, 10 May 2025 15:34:01 +0800
Huacai Chen <chenhuacai@kernel.org> wrote:

> Hi, George,
> 
> On Thu, May 8, 2025 at 11:056§2AM george <dongtai.guo@linux.dev> wrote:
> >
> > From: George Guo <guodongtai@kylinos.cn>
> >
> > This check could falsely report errors when PTEs are remapped:
> > arch/loongarch/mm/init.c:204: bad pte 0000000107b601c3.
> >
> > The check was unnecessary because:
> > - Fixmap slots are designed to be reusable
> > - Other architectures (x86/arm64) allow PTE overwrites
> > - Callers are responsible for proper mapping management
> >
> > Signed-off-by: George Guo <guodongtai@kylinos.cn>
> > ---
> >  arch/loongarch/mm/init.c | 4 ----
> >  1 file changed, 4 deletions(-)
> >
> > diff --git a/arch/loongarch/mm/init.c b/arch/loongarch/mm/init.c
> > index 06f11d9e4ec1..7b7e3b1aa00d 100644
> > --- a/arch/loongarch/mm/init.c
> > +++ b/arch/loongarch/mm/init.c
> > @@ -200,10 +200,6 @@ void __init __set_fixmap(enum fixed_addresses
> > idx, BUG_ON(idx <= FIX_HOLE || idx >= __end_of_fixed_addresses);
> >
> >         ptep = populate_kernel_pte(addr);
> > -       if (!pte_none(ptep_get(ptep))) {
> > -               pte_ERROR(*ptep);
> > -               return;
> > -       }
> Resolve what problems? If other patches need this change, please
> submit them as a series.
Here is a test case to show the bug:
#include <linux/module.h>
#include <linux/vmalloc.h>
#include <linux/mm.h>
#include <asm/io.h>
#include <asm-generic/fixmap.h>

#define ALLOC_SIZE 8192  // 8KB

static char *vmalloc_mem = NULL;

static int __init fixmap_test_init(void)
{
	struct page *page;
	phys_addr_t phys;
	void *addr;
	char c;
	void __iomem *base;
	char *charp;

	pr_info("%s module loaded!\n", THIS_MODULE->name);

	addr = vmalloc_mem = vmalloc(ALLOC_SIZE);
	if (!vmalloc_mem) {
		pr_err("Failed to allocate memory with vmalloc\n");
		return -ENOMEM;
	}

	vmalloc_mem[4099] = 'a';
	vmalloc_mem[0] = 'g';
	pr_info("Allocated vmalloc memory at virtual address: %px,
	vmalloc_mem[0]=%c\n", vmalloc_mem, vmalloc_mem[0]);

	//vrit addr -> phys addr
	page = vmalloc_to_page(addr);
	phys = page_to_phys(page) + offset_in_page(addr);
	pr_info("phys = 0x%lx\n", phys);


	__set_fixmap(FIX_TEXT_POKE0, phys & PAGE_MASK, FIXMAP_PAGE_IO);
	base = (void __iomem *)__fix_to_virt(FIX_TEXT_POKE0);
	base += phys & ~PAGE_MASK;
	charp = (char *)base;
	pr_info("fixmap virt addr=0x%lx,charp[0] = %c\n",base,
	charp[0]);
	__set_fixmap(FIX_TEXT_POKE0, 0, FIXMAP_PAGE_CLEAR);

	return 0;
}

static void __exit fixmap_test_exit(void)
{
	if (vmalloc_mem) {
		vfree(vmalloc_mem);
		pr_info("Freed vmalloc memory\n");
	}

	pr_info("%s module unloaded!\n", THIS_MODULE->name);
}

module_init(fixmap_test_init);
module_exit(fixmap_test_exit);

MODULE_LICENSE("GPL");
MODULE_AUTHOR("George Guo <guodongtai@kylinos.cn>");
MODULE_DESCRIPTION("fixmap test");

And below is the log for test:

arm64:

first insmod:
[10476.162720] fixmap_test module loaded!
[10476.163390] Allocated vmalloc memory at virtual address:
ffff80008008f000, vmalloc_mem[0]=g
[10476.164497] phys = 0x10fab2000
[10476.165106] fixmap virt addr=0xffffffffff5fc000,charp[0] = g

rmmod then insmod again:
[10514.031689] Freed vmalloc memory
[10514.032268] fixmap_test module unloaded!
[10516.391483] fixmap_test module loaded!
[10516.392135] Allocated vmalloc memory at virtual address:
ffff800080097000, vmalloc_mem[0]=g
[10516.393205] phys = 0x10dc2c000
[10516.393663] fixmap virt addr=0xffffffffff5fc000,charp[0] = g


loongarch:

first insmod:
[   43.983300] [   T1399] fixmap test module loaded
[   43.983570] [   T1399] Allocated vmalloc memory at virtual address:
ffff80001b45c000, vmalloc_mem[0]=g
[   43.983973] [   T1399] phys = 0x107b60000
[   43.984535] [   T1399] fixmap virt addr=0xfffffffffffd8000,charp[0]
= g

rmmod then insmod again:
[   52.266230] [   T1417] Freed vmalloc memory
[   55.344169] [   T1421] fixmap test module loaded
[   55.344441] [   T1421] Allocated vmalloc memory at virtual address:
ffff80001b344000, vmalloc_mem[0]=g
[   55.344915] [   T1421] phys = 0x10ae04000
[   55.345503] [   T1421] arch/loongarch/mm/init.c:204: bad pte
0000000107b601c3.
[   55.346087] [   T1421] fixmap virt addr=0xfffffffffffd8000,charp[0]
= \x00

> 
> Huacai
> 
> >
> >         if (pgprot_val(flags))
> >                 set_pte(ptep, pfn_pte(phys >> PAGE_SHIFT, flags));
> > --
> > 2.34.1
> >
> >

[PATCH v2] LoongArch: mm: remove redundant pte occupancy check in __set_fixmap()

[george]

From: George Guo <guodongtai@kylinos.cn>

This check could falsely report errors when PTEs are remapped:
arch/loongarch/mm/init.c:204: bad pte 0000000107b601c3.

The check was unnecessary because:
- Fixmap slots are designed to be reusable
- Callers are responsible for proper mapping management

Changes in v2:
- modify commit messages

Signed-off-by: George Guo <guodongtai@kylinos.cn>
---
 arch/loongarch/mm/init.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/loongarch/mm/init.c b/arch/loongarch/mm/init.c
index c3e4586a7975..626c99b803a3 100644
--- a/arch/loongarch/mm/init.c
+++ b/arch/loongarch/mm/init.c
@@ -192,10 +192,6 @@ void __init __set_fixmap(enum fixed_addresses idx,
 	BUG_ON(idx <= FIX_HOLE || idx >= __end_of_fixed_addresses);
 
 	ptep = populate_kernel_pte(addr);
-	if (!pte_none(ptep_get(ptep))) {
-		pte_ERROR(*ptep);
-		return;
-	}
 
 	if (pgprot_val(flags))
 		set_pte(ptep, pfn_pte(phys >> PAGE_SHIFT, flags));
-- 
2.34.1

Re: [PATCH v2] LoongArch: mm: remove redundant pte occupancy check in __set_fixmap()

[Huacai Chen]

On Thu, Aug 21, 2025 at 11:05 AM george <dongtai.guo@linux.dev> wrote:
>
> From: George Guo <guodongtai@kylinos.cn>
>
> This check could falsely report errors when PTEs are remapped:
> arch/loongarch/mm/init.c:204: bad pte 0000000107b601c3.
>
> The check was unnecessary because:
> - Fixmap slots are designed to be reusable
> - Callers are responsible for proper mapping management
>
> Changes in v2:
> - modify commit messages
>
> Signed-off-by: George Guo <guodongtai@kylinos.cn>
> ---
>  arch/loongarch/mm/init.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/arch/loongarch/mm/init.c b/arch/loongarch/mm/init.c
> index c3e4586a7975..626c99b803a3 100644
> --- a/arch/loongarch/mm/init.c
> +++ b/arch/loongarch/mm/init.c
> @@ -192,10 +192,6 @@ void __init __set_fixmap(enum fixed_addresses idx,
>         BUG_ON(idx <= FIX_HOLE || idx >= __end_of_fixed_addresses);
>
>         ptep = populate_kernel_pte(addr);
> -       if (!pte_none(ptep_get(ptep))) {
> -               pte_ERROR(*ptep);
> -               return;
> -       }
This only makes sense if we have FIX_TEXT_POKE, if without it, fixmap
won't change at runtime.

Huacai

>
>         if (pgprot_val(flags))
>                 set_pte(ptep, pfn_pte(phys >> PAGE_SHIFT, flags));
> --
> 2.34.1
>