LinuxLists.cc - [PATCH v5 0/2] riscv: Use PUD/P4D/PGD pages for the linear mapping

2023-01-25 08:12:37

Subject: [PATCH v5 0/2] riscv: Use PUD/P4D/PGD pages for the linear mapping

This patchset intends to improve tlb utilization by using hugepages for
the linear mapping.

v5:
- Fix nommu builds by getting rid of riscv_pfn_base in patch 1, thanks
Conor
- Add RB from Andre

v4:
- Rebase on top of v6.2-rc3, as noted by Conor
- Add Acked-by Rob

v3:
- Change the comment about initrd_start VA conversion so that it fits
ARM64 and RISCV64 (and others in the future if needed), as suggested
by Rob

v2:
- Add a comment on why RISCV64 does not need to set initrd_start/end that
early in the boot process, as asked by Rob

Alexandre Ghiti (2):
riscv: Get rid of riscv_pfn_base variable
riscv: Use PUD/P4D/PGD pages for the linear mapping

arch/riscv/include/asm/page.h | 19 +++++++++++++++++--
arch/riscv/mm/init.c | 28 ++++++++++++++++++----------
arch/riscv/mm/physaddr.c | 16 ++++++++++++++++
drivers/of/fdt.c | 11 ++++++-----
4 files changed, 57 insertions(+), 17 deletions(-)

--
2.37.2

2023-01-25 08:13:24

by Alexandre Ghiti

[permalink] [raw]

Subject: [PATCH v5 1/2] riscv: Get rid of riscv_pfn_base variable

Use directly phys_ram_base instead, riscv_pfn_base is just the pfn of
the address contained in phys_ram_base.

Even if there is no functional change intended in this patch, actually
setting phys_ram_base that early changes the behaviour of
kernel_mapping_pa_to_va during the early boot: phys_ram_base used to be
zero before this patch and now it is set to the physical start address of
the kernel. But it does not break the conversion of a kernel physical
address into a virtual address since kernel_mapping_pa_to_va should only
be used on kernel physical addresses, i.e. addresses greater than the
physical start address of the kernel.

Signed-off-by: Alexandre Ghiti <[email protected]>
---
arch/riscv/include/asm/page.h | 3 +--
arch/riscv/mm/init.c | 6 +-----
2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
index 9f432c1b5289..728eee53152a 100644
--- a/arch/riscv/include/asm/page.h
+++ b/arch/riscv/include/asm/page.h
@@ -91,8 +91,7 @@ typedef struct page *pgtable_t;
#endif

#ifdef CONFIG_MMU
-extern unsigned long riscv_pfn_base;
-#define ARCH_PFN_OFFSET (riscv_pfn_base)
+#define ARCH_PFN_OFFSET (PFN_DOWN(phys_ram_base))
#else
#define ARCH_PFN_OFFSET (PAGE_OFFSET >> PAGE_SHIFT)
#endif /* CONFIG_MMU */
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 478d6763a01a..225a7d2b65cc 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -271,9 +271,6 @@ static void __init setup_bootmem(void)
#ifdef CONFIG_MMU
struct pt_alloc_ops pt_ops __initdata;

-unsigned long riscv_pfn_base __ro_after_init;
-EXPORT_SYMBOL(riscv_pfn_base);
-
pgd_t swapper_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
pgd_t trampoline_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
static pte_t fixmap_pte[PTRS_PER_PTE] __page_aligned_bss;
@@ -285,7 +282,6 @@ static pmd_t __maybe_unused early_dtb_pmd[PTRS_PER_PMD] __initdata __aligned(PAG

#ifdef CONFIG_XIP_KERNEL
#define pt_ops (*(struct pt_alloc_ops *)XIP_FIXUP(&pt_ops))
-#define riscv_pfn_base (*(unsigned long *)XIP_FIXUP(&riscv_pfn_base))
#define trampoline_pg_dir ((pgd_t *)XIP_FIXUP(trampoline_pg_dir))
#define fixmap_pte ((pte_t *)XIP_FIXUP(fixmap_pte))
#define early_pg_dir ((pgd_t *)XIP_FIXUP(early_pg_dir))
@@ -985,7 +981,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
kernel_map.va_pa_offset = PAGE_OFFSET - kernel_map.phys_addr;
kernel_map.va_kernel_pa_offset = kernel_map.virt_addr - kernel_map.phys_addr;

- riscv_pfn_base = PFN_DOWN(kernel_map.phys_addr);
+ phys_ram_base = kernel_map.phys_addr;

/*
* The default maximal physical memory size is KERN_VIRT_SIZE for 32-bit
--
2.37.2

2023-01-25 08:14:27

by Alexandre Ghiti

[permalink] [raw]

Subject: [PATCH v5 2/2] riscv: Use PUD/P4D/PGD pages for the linear mapping

During the early page table creation, we used to set the mapping for
PAGE_OFFSET to the kernel load address: but the kernel load address is
always offseted by PMD_SIZE which makes it impossible to use PUD/P4D/PGD
pages as this physical address is not aligned on PUD/P4D/PGD size (whereas
PAGE_OFFSET is).

But actually we don't have to establish this mapping (ie set va_pa_offset)
that early in the boot process because:

- first, setup_vm installs a temporary kernel mapping and among other
things, discovers the system memory,
- then, setup_vm_final creates the final kernel mapping and takes
advantage of the discovered system memory to create the linear
mapping.

During the first phase, we don't know the start of the system memory and
then until the second phase is finished, we can't use the linear mapping at
all and phys_to_virt/virt_to_phys translations must not be used because it
would result in a different translation from the 'real' one once the final
mapping is installed.

So here we simply delay the initialization of va_pa_offset to after the
system memory discovery. But to make sure noone uses the linear mapping
before, we add some guard in the DEBUG_VIRTUAL config.

Finally we can use PUD/P4D/PGD hugepages when possible, which will result
in a better TLB utilization.

Note that we rely on the firmware to protect itself using PMP.

Signed-off-by: Alexandre Ghiti <[email protected]>
Acked-by: Rob Herring <[email protected]> # DT bits
Reviewed-by: Andrew Jones <[email protected]>
---
arch/riscv/include/asm/page.h | 16 ++++++++++++++++
arch/riscv/mm/init.c | 24 ++++++++++++++++++------
arch/riscv/mm/physaddr.c | 16 ++++++++++++++++
drivers/of/fdt.c | 11 ++++++-----
4 files changed, 56 insertions(+), 11 deletions(-)

diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
index 728eee53152a..bd7b9dda1e4f 100644
--- a/arch/riscv/include/asm/page.h
+++ b/arch/riscv/include/asm/page.h
@@ -90,6 +90,14 @@ typedef struct page *pgtable_t;
#define PTE_FMT "%08lx"
#endif

+#ifdef CONFIG_64BIT
+/*
+ * We override this value as its generic definition uses __pa too early in
+ * the boot process (before kernel_map.va_pa_offset is set).
+ */
+#define MIN_MEMBLOCK_ADDR 0
+#endif
+
#ifdef CONFIG_MMU
#define ARCH_PFN_OFFSET (PFN_DOWN(phys_ram_base))
#else
@@ -121,7 +129,11 @@ extern phys_addr_t phys_ram_base;
#define is_linear_mapping(x) \
((x) >= PAGE_OFFSET && (!IS_ENABLED(CONFIG_64BIT) || (x) < PAGE_OFFSET + KERN_VIRT_SIZE))

+#ifndef CONFIG_DEBUG_VIRTUAL
#define linear_mapping_pa_to_va(x) ((void *)((unsigned long)(x) + kernel_map.va_pa_offset))
+#else
+void *linear_mapping_pa_to_va(unsigned long x);
+#endif
#define kernel_mapping_pa_to_va(y) ({ \
unsigned long _y = (unsigned long)(y); \
(IS_ENABLED(CONFIG_XIP_KERNEL) && _y < phys_ram_base) ? \
@@ -130,7 +142,11 @@ extern phys_addr_t phys_ram_base;
})
#define __pa_to_va_nodebug(x) linear_mapping_pa_to_va(x)

+#ifndef CONFIG_DEBUG_VIRTUAL
#define linear_mapping_va_to_pa(x) ((unsigned long)(x) - kernel_map.va_pa_offset)
+#else
+phys_addr_t linear_mapping_va_to_pa(unsigned long x);
+#endif
#define kernel_mapping_va_to_pa(y) ({ \
unsigned long _y = (unsigned long)(y); \
(IS_ENABLED(CONFIG_XIP_KERNEL) && _y < kernel_map.virt_addr + XIP_OFFSET) ? \
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 225a7d2b65cc..9dfc0afdb114 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -213,6 +213,13 @@ static void __init setup_bootmem(void)
phys_ram_end = memblock_end_of_DRAM();
if (!IS_ENABLED(CONFIG_XIP_KERNEL))
phys_ram_base = memblock_start_of_DRAM();
+
+ /*
+ * Any use of __va/__pa before this point is wrong as we did not know the
+ * start of DRAM before.
+ */
+ kernel_map.va_pa_offset = PAGE_OFFSET - phys_ram_base;
+
/*
* memblock allocator is not aware of the fact that last 4K bytes of
* the addressable memory can not be mapped because of IS_ERR_VALUE
@@ -667,9 +674,16 @@ void __init create_pgd_mapping(pgd_t *pgdp,

static uintptr_t __init best_map_size(phys_addr_t base, phys_addr_t size)
{
- /* Upgrade to PMD_SIZE mappings whenever possible */
- base &= PMD_SIZE - 1;
- if (!base && size >= PMD_SIZE)
+ if (!(base & (PGDIR_SIZE - 1)) && size >= PGDIR_SIZE)
+ return PGDIR_SIZE;
+
+ if (!(base & (P4D_SIZE - 1)) && size >= P4D_SIZE)
+ return P4D_SIZE;
+
+ if (!(base & (PUD_SIZE - 1)) && size >= PUD_SIZE)
+ return PUD_SIZE;
+
+ if (!(base & (PMD_SIZE - 1)) && size >= PMD_SIZE)
return PMD_SIZE;

return PAGE_SIZE;
@@ -978,11 +992,9 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
set_satp_mode();
#endif

- kernel_map.va_pa_offset = PAGE_OFFSET - kernel_map.phys_addr;
+ kernel_map.va_pa_offset = 0UL;
kernel_map.va_kernel_pa_offset = kernel_map.virt_addr - kernel_map.phys_addr;

- phys_ram_base = kernel_map.phys_addr;
-
/*
* The default maximal physical memory size is KERN_VIRT_SIZE for 32-bit
* kernel, whereas for 64-bit kernel, the end of the virtual address
diff --git a/arch/riscv/mm/physaddr.c b/arch/riscv/mm/physaddr.c
index 9b18bda74154..18706f457da7 100644
--- a/arch/riscv/mm/physaddr.c
+++ b/arch/riscv/mm/physaddr.c
@@ -33,3 +33,19 @@ phys_addr_t __phys_addr_symbol(unsigned long x)
return __va_to_pa_nodebug(x);
}
EXPORT_SYMBOL(__phys_addr_symbol);
+
+phys_addr_t linear_mapping_va_to_pa(unsigned long x)
+{
+ BUG_ON(!kernel_map.va_pa_offset);
+
+ return ((unsigned long)(x) - kernel_map.va_pa_offset);
+}
+EXPORT_SYMBOL(linear_mapping_va_to_pa);
+
+void *linear_mapping_pa_to_va(unsigned long x)
+{
+ BUG_ON(!kernel_map.va_pa_offset);
+
+ return ((void *)((unsigned long)(x) + kernel_map.va_pa_offset));
+}
+EXPORT_SYMBOL(linear_mapping_pa_to_va);
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index f08b25195ae7..58107bd56f8f 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -891,12 +891,13 @@ const void * __init of_flat_dt_match_machine(const void *default_match,
static void __early_init_dt_declare_initrd(unsigned long start,
unsigned long end)
{
- /* ARM64 would cause a BUG to occur here when CONFIG_DEBUG_VM is
- * enabled since __va() is called too early. ARM64 does make use
- * of phys_initrd_start/phys_initrd_size so we can skip this
- * conversion.
+ /*
+ * __va() is not yet available this early on some platforms. In that
+ * case, the platform uses phys_initrd_start/phys_initrd_size instead
+ * and does the VA conversion itself.
*/
- if (!IS_ENABLED(CONFIG_ARM64)) {
+ if (!IS_ENABLED(CONFIG_ARM64) &&
+ !(IS_ENABLED(CONFIG_RISCV) && IS_ENABLED(CONFIG_64BIT))) {
initrd_start = (unsigned long)__va(start);
initrd_end = (unsigned long)__va(end);
initrd_below_start_ok = 1;
--
2.37.2

2023-01-25 11:40:53

by Andrew Jones

[permalink] [raw]

Subject: Re: [PATCH v5 1/2] riscv: Get rid of riscv_pfn_base variable

On Wed, Jan 25, 2023 at 09:12:13AM +0100, Alexandre Ghiti wrote:
> Use directly phys_ram_base instead, riscv_pfn_base is just the pfn of
> the address contained in phys_ram_base.
>
> Even if there is no functional change intended in this patch, actually
> setting phys_ram_base that early changes the behaviour of
> kernel_mapping_pa_to_va during the early boot: phys_ram_base used to be
> zero before this patch and now it is set to the physical start address of
> the kernel. But it does not break the conversion of a kernel physical
> address into a virtual address since kernel_mapping_pa_to_va should only
> be used on kernel physical addresses, i.e. addresses greater than the
> physical start address of the kernel.

afaict, only CONFIG_XIP_KERNEL kernels use phys_ram_base prior to
setup_bootmem() and, for them, this change only redundantly sets
phys_ram_base to the same thing, so I believe this is a no functional
change patch.

>
> Signed-off-by: Alexandre Ghiti <[email protected]>
> ---
> arch/riscv/include/asm/page.h | 3 +--
> arch/riscv/mm/init.c | 6 +-----
> 2 files changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
> index 9f432c1b5289..728eee53152a 100644
> --- a/arch/riscv/include/asm/page.h
> +++ b/arch/riscv/include/asm/page.h
> @@ -91,8 +91,7 @@ typedef struct page *pgtable_t;
> #endif
>
> #ifdef CONFIG_MMU
> -extern unsigned long riscv_pfn_base;
> -#define ARCH_PFN_OFFSET (riscv_pfn_base)
> +#define ARCH_PFN_OFFSET (PFN_DOWN(phys_ram_base))
> #else
> #define ARCH_PFN_OFFSET (PAGE_OFFSET >> PAGE_SHIFT)
> #endif /* CONFIG_MMU */
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 478d6763a01a..225a7d2b65cc 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -271,9 +271,6 @@ static void __init setup_bootmem(void)
> #ifdef CONFIG_MMU
> struct pt_alloc_ops pt_ops __initdata;
>
> -unsigned long riscv_pfn_base __ro_after_init;
> -EXPORT_SYMBOL(riscv_pfn_base);
> -
> pgd_t swapper_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
> pgd_t trampoline_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
> static pte_t fixmap_pte[PTRS_PER_PTE] __page_aligned_bss;
> @@ -285,7 +282,6 @@ static pmd_t __maybe_unused early_dtb_pmd[PTRS_PER_PMD] __initdata __aligned(PAG
>
> #ifdef CONFIG_XIP_KERNEL
> #define pt_ops (*(struct pt_alloc_ops *)XIP_FIXUP(&pt_ops))
> -#define riscv_pfn_base (*(unsigned long *)XIP_FIXUP(&riscv_pfn_base))
> #define trampoline_pg_dir ((pgd_t *)XIP_FIXUP(trampoline_pg_dir))
> #define fixmap_pte ((pte_t *)XIP_FIXUP(fixmap_pte))
> #define early_pg_dir ((pgd_t *)XIP_FIXUP(early_pg_dir))
> @@ -985,7 +981,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> kernel_map.va_pa_offset = PAGE_OFFSET - kernel_map.phys_addr;
> kernel_map.va_kernel_pa_offset = kernel_map.virt_addr - kernel_map.phys_addr;
>
> - riscv_pfn_base = PFN_DOWN(kernel_map.phys_addr);
> + phys_ram_base = kernel_map.phys_addr;

nit: I'd put this in the #else part of the #ifdef CONFIG_XIP_KERNEL above
to have some consistency with that #ifdef arm and also avoid the redundant
assignment of phys_ram_base for CONFIG_XIP_KERNEL.

>
> /*
> * The default maximal physical memory size is KERN_VIRT_SIZE for 32-bit
> --
> 2.37.2
>

Otherwise,

Reviewed-by: Andrew Jones <[email protected]>

Thanks,
drew

2023-01-25 11:42:39

by Andrew Jones

[permalink] [raw]

Subject: Re: [PATCH v5 2/2] riscv: Use PUD/P4D/PGD pages for the linear mapping

On Wed, Jan 25, 2023 at 09:12:14AM +0100, Alexandre Ghiti wrote:
> During the early page table creation, we used to set the mapping for
> PAGE_OFFSET to the kernel load address: but the kernel load address is
> always offseted by PMD_SIZE which makes it impossible to use PUD/P4D/PGD
> pages as this physical address is not aligned on PUD/P4D/PGD size (whereas
> PAGE_OFFSET is).
>
> But actually we don't have to establish this mapping (ie set va_pa_offset)
> that early in the boot process because:
>
> - first, setup_vm installs a temporary kernel mapping and among other
> things, discovers the system memory,
> - then, setup_vm_final creates the final kernel mapping and takes
> advantage of the discovered system memory to create the linear
> mapping.
>
> During the first phase, we don't know the start of the system memory and
> then until the second phase is finished, we can't use the linear mapping at
> all and phys_to_virt/virt_to_phys translations must not be used because it
> would result in a different translation from the 'real' one once the final
> mapping is installed.
>
> So here we simply delay the initialization of va_pa_offset to after the
> system memory discovery. But to make sure noone uses the linear mapping
> before, we add some guard in the DEBUG_VIRTUAL config.
>
> Finally we can use PUD/P4D/PGD hugepages when possible, which will result
> in a better TLB utilization.
>
> Note that we rely on the firmware to protect itself using PMP.
>
> Signed-off-by: Alexandre Ghiti <[email protected]>
> Acked-by: Rob Herring <[email protected]> # DT bits
> Reviewed-by: Andrew Jones <[email protected]>
> ---
> arch/riscv/include/asm/page.h | 16 ++++++++++++++++
> arch/riscv/mm/init.c | 24 ++++++++++++++++++------
> arch/riscv/mm/physaddr.c | 16 ++++++++++++++++
> drivers/of/fdt.c | 11 ++++++-----
> 4 files changed, 56 insertions(+), 11 deletions(-)
>
> diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
> index 728eee53152a..bd7b9dda1e4f 100644
> --- a/arch/riscv/include/asm/page.h
> +++ b/arch/riscv/include/asm/page.h
> @@ -90,6 +90,14 @@ typedef struct page *pgtable_t;
> #define PTE_FMT "%08lx"
> #endif
>
> +#ifdef CONFIG_64BIT
> +/*
> + * We override this value as its generic definition uses __pa too early in
> + * the boot process (before kernel_map.va_pa_offset is set).
> + */
> +#define MIN_MEMBLOCK_ADDR 0
> +#endif
> +
> #ifdef CONFIG_MMU
> #define ARCH_PFN_OFFSET (PFN_DOWN(phys_ram_base))
> #else
> @@ -121,7 +129,11 @@ extern phys_addr_t phys_ram_base;
> #define is_linear_mapping(x) \
> ((x) >= PAGE_OFFSET && (!IS_ENABLED(CONFIG_64BIT) || (x) < PAGE_OFFSET + KERN_VIRT_SIZE))
>
> +#ifndef CONFIG_DEBUG_VIRTUAL
> #define linear_mapping_pa_to_va(x) ((void *)((unsigned long)(x) + kernel_map.va_pa_offset))
> +#else
> +void *linear_mapping_pa_to_va(unsigned long x);
> +#endif
> #define kernel_mapping_pa_to_va(y) ({ \
> unsigned long _y = (unsigned long)(y); \
> (IS_ENABLED(CONFIG_XIP_KERNEL) && _y < phys_ram_base) ? \
> @@ -130,7 +142,11 @@ extern phys_addr_t phys_ram_base;
> })
> #define __pa_to_va_nodebug(x) linear_mapping_pa_to_va(x)
>
> +#ifndef CONFIG_DEBUG_VIRTUAL
> #define linear_mapping_va_to_pa(x) ((unsigned long)(x) - kernel_map.va_pa_offset)
> +#else
> +phys_addr_t linear_mapping_va_to_pa(unsigned long x);
> +#endif
> #define kernel_mapping_va_to_pa(y) ({ \
> unsigned long _y = (unsigned long)(y); \
> (IS_ENABLED(CONFIG_XIP_KERNEL) && _y < kernel_map.virt_addr + XIP_OFFSET) ? \
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 225a7d2b65cc..9dfc0afdb114 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -213,6 +213,13 @@ static void __init setup_bootmem(void)
> phys_ram_end = memblock_end_of_DRAM();
> if (!IS_ENABLED(CONFIG_XIP_KERNEL))
> phys_ram_base = memblock_start_of_DRAM();
> +
> + /*
> + * Any use of __va/__pa before this point is wrong as we did not know the
> + * start of DRAM before.
> + */
> + kernel_map.va_pa_offset = PAGE_OFFSET - phys_ram_base;
> +
> /*
> * memblock allocator is not aware of the fact that last 4K bytes of
> * the addressable memory can not be mapped because of IS_ERR_VALUE
> @@ -667,9 +674,16 @@ void __init create_pgd_mapping(pgd_t *pgdp,
>
> static uintptr_t __init best_map_size(phys_addr_t base, phys_addr_t size)
> {
> - /* Upgrade to PMD_SIZE mappings whenever possible */
> - base &= PMD_SIZE - 1;
> - if (!base && size >= PMD_SIZE)
> + if (!(base & (PGDIR_SIZE - 1)) && size >= PGDIR_SIZE)
> + return PGDIR_SIZE;
> +
> + if (!(base & (P4D_SIZE - 1)) && size >= P4D_SIZE)
> + return P4D_SIZE;
> +
> + if (!(base & (PUD_SIZE - 1)) && size >= PUD_SIZE)
> + return PUD_SIZE;
> +
> + if (!(base & (PMD_SIZE - 1)) && size >= PMD_SIZE)
> return PMD_SIZE;
>
> return PAGE_SIZE;
> @@ -978,11 +992,9 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> set_satp_mode();
> #endif
>
> - kernel_map.va_pa_offset = PAGE_OFFSET - kernel_map.phys_addr;
> + kernel_map.va_pa_offset = 0UL;
> kernel_map.va_kernel_pa_offset = kernel_map.virt_addr - kernel_map.phys_addr;
>
> - phys_ram_base = kernel_map.phys_addr;
> -
> /*
> * The default maximal physical memory size is KERN_VIRT_SIZE for 32-bit
> * kernel, whereas for 64-bit kernel, the end of the virtual address
> diff --git a/arch/riscv/mm/physaddr.c b/arch/riscv/mm/physaddr.c
> index 9b18bda74154..18706f457da7 100644
> --- a/arch/riscv/mm/physaddr.c
> +++ b/arch/riscv/mm/physaddr.c
> @@ -33,3 +33,19 @@ phys_addr_t __phys_addr_symbol(unsigned long x)
> return __va_to_pa_nodebug(x);
> }
> EXPORT_SYMBOL(__phys_addr_symbol);
> +
> +phys_addr_t linear_mapping_va_to_pa(unsigned long x)
> +{
> + BUG_ON(!kernel_map.va_pa_offset);
> +
> + return ((unsigned long)(x) - kernel_map.va_pa_offset);
> +}
> +EXPORT_SYMBOL(linear_mapping_va_to_pa);
> +
> +void *linear_mapping_pa_to_va(unsigned long x)
> +{
> + BUG_ON(!kernel_map.va_pa_offset);
> +
> + return ((void *)((unsigned long)(x) + kernel_map.va_pa_offset));

I missed pointing out this superfluous () in this function and the one
above it in my first review, but it's just a nit anyway.

Thanks,
drew

2023-01-27 08:48:21

by Alexandre Ghiti

[permalink] [raw]

Subject: Re: [PATCH v5 1/2] riscv: Get rid of riscv_pfn_base variable

On Wed, Jan 25, 2023 at 12:40 PM Andrew Jones <[email protected]> wrote:
>
> On Wed, Jan 25, 2023 at 09:12:13AM +0100, Alexandre Ghiti wrote:
> > Use directly phys_ram_base instead, riscv_pfn_base is just the pfn of
> > the address contained in phys_ram_base.
> >
> > Even if there is no functional change intended in this patch, actually
> > setting phys_ram_base that early changes the behaviour of
> > kernel_mapping_pa_to_va during the early boot: phys_ram_base used to be
> > zero before this patch and now it is set to the physical start address of
> > the kernel. But it does not break the conversion of a kernel physical
> > address into a virtual address since kernel_mapping_pa_to_va should only
> > be used on kernel physical addresses, i.e. addresses greater than the
> > physical start address of the kernel.
>
> afaict, only CONFIG_XIP_KERNEL kernels use phys_ram_base prior to
> setup_bootmem() and, for them, this change only redundantly sets
> phys_ram_base to the same thing, so I believe this is a no functional
> change patch.
>

Good, thanks for checking again

> >
> > Signed-off-by: Alexandre Ghiti <[email protected]>
> > ---
> > arch/riscv/include/asm/page.h | 3 +--
> > arch/riscv/mm/init.c | 6 +-----
> > 2 files changed, 2 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
> > index 9f432c1b5289..728eee53152a 100644
> > --- a/arch/riscv/include/asm/page.h
> > +++ b/arch/riscv/include/asm/page.h
> > @@ -91,8 +91,7 @@ typedef struct page *pgtable_t;
> > #endif
> >
> > #ifdef CONFIG_MMU
> > -extern unsigned long riscv_pfn_base;
> > -#define ARCH_PFN_OFFSET (riscv_pfn_base)
> > +#define ARCH_PFN_OFFSET (PFN_DOWN(phys_ram_base))
> > #else
> > #define ARCH_PFN_OFFSET (PAGE_OFFSET >> PAGE_SHIFT)
> > #endif /* CONFIG_MMU */
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index 478d6763a01a..225a7d2b65cc 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -271,9 +271,6 @@ static void __init setup_bootmem(void)
> > #ifdef CONFIG_MMU
> > struct pt_alloc_ops pt_ops __initdata;
> >
> > -unsigned long riscv_pfn_base __ro_after_init;
> > -EXPORT_SYMBOL(riscv_pfn_base);
> > -
> > pgd_t swapper_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
> > pgd_t trampoline_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
> > static pte_t fixmap_pte[PTRS_PER_PTE] __page_aligned_bss;
> > @@ -285,7 +282,6 @@ static pmd_t __maybe_unused early_dtb_pmd[PTRS_PER_PMD] __initdata __aligned(PAG
> >
> > #ifdef CONFIG_XIP_KERNEL
> > #define pt_ops (*(struct pt_alloc_ops *)XIP_FIXUP(&pt_ops))
> > -#define riscv_pfn_base (*(unsigned long *)XIP_FIXUP(&riscv_pfn_base))
> > #define trampoline_pg_dir ((pgd_t *)XIP_FIXUP(trampoline_pg_dir))
> > #define fixmap_pte ((pte_t *)XIP_FIXUP(fixmap_pte))
> > #define early_pg_dir ((pgd_t *)XIP_FIXUP(early_pg_dir))
> > @@ -985,7 +981,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> > kernel_map.va_pa_offset = PAGE_OFFSET - kernel_map.phys_addr;
> > kernel_map.va_kernel_pa_offset = kernel_map.virt_addr - kernel_map.phys_addr;
> >
> > - riscv_pfn_base = PFN_DOWN(kernel_map.phys_addr);
> > + phys_ram_base = kernel_map.phys_addr;
>
> nit: I'd put this in the #else part of the #ifdef CONFIG_XIP_KERNEL above
> to have some consistency with that #ifdef arm and also avoid the redundant
> assignment of phys_ram_base for CONFIG_XIP_KERNEL.

True, but as this is removed in the next patch, I guess we can live with that.

>
> >
> > /*
> > * The default maximal physical memory size is KERN_VIRT_SIZE for 32-bit
> > --
> > 2.37.2
> >
>
> Otherwise,
>
> Reviewed-by: Andrew Jones <[email protected]>

Thanks!

Alex

>
> Thanks,
> drew

2023-01-28 14:08:10

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH v5 1/2] riscv: Get rid of riscv_pfn_base variable

Hi Alexandre,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v6.2-rc5 next-20230127]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Alexandre-Ghiti/riscv-Get-rid-of-riscv_pfn_base-variable/20230125-161537
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link: https://lore.kernel.org/r/20230125081214.1576313-2-alexghiti%40rivosinc.com
patch subject: [PATCH v5 1/2] riscv: Get rid of riscv_pfn_base variable
config: riscv-randconfig-r014-20230123 (https://download.01.org/0day-ci/archive/20230128/[email protected]/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 4196ca3278f78c6e19246e54ab0ecb364e37d66a)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/90b21402dc8a7e6e36a62ad19c4969ff13fad168
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Alexandre-Ghiti/riscv-Get-rid-of-riscv_pfn_base-variable/20230125-161537
git checkout 90b21402dc8a7e6e36a62ad19c4969ff13fad168
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/mmc/host/ mm/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

>> mm/debug.c:95:25: warning: format specifies type 'unsigned long' but the argument has type 'phys_addr_t' (aka 'unsigned long long') [-Wformat]
page_to_pgoff(page), page_to_pfn(page));
^~~~~~~~~~~~~~~~~
include/linux/printk.h:510:37: note: expanded from macro 'pr_warn'
printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
include/linux/printk.h:457:60: note: expanded from macro 'printk'
#define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
include/linux/printk.h:429:19: note: expanded from macro 'printk_index_wrap'
_p_func(_fmt, ##__VA_ARGS__); \
~~~~ ^~~~~~~~~~~
include/asm-generic/memory_model.h:52:21: note: expanded from macro 'page_to_pfn'
#define page_to_pfn __page_to_pfn
^
include/asm-generic/memory_model.h:19:29: note: expanded from macro '__page_to_pfn'
#define __page_to_pfn(page) ((unsigned long)((page) - mem_map) + \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
--
>> mm/page_alloc.c:695:18: warning: format specifies type 'unsigned long' but the argument has type 'phys_addr_t' (aka 'unsigned long long') [-Wformat]
current->comm, page_to_pfn(page));
^~~~~~~~~~~~~~~~~
include/linux/printk.h:480:35: note: expanded from macro 'pr_alert'
printk(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
include/linux/printk.h:457:60: note: expanded from macro 'printk'
#define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
include/linux/printk.h:429:19: note: expanded from macro 'printk_index_wrap'
_p_func(_fmt, ##__VA_ARGS__); \
~~~~ ^~~~~~~~~~~
include/asm-generic/memory_model.h:52:21: note: expanded from macro 'page_to_pfn'
#define page_to_pfn __page_to_pfn
^
include/asm-generic/memory_model.h:19:29: note: expanded from macro '__page_to_pfn'
#define __page_to_pfn(page) ((unsigned long)((page) - mem_map) + \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
--
>> mm/page_isolation.c:174:26: warning: comparison of distinct pointer types ('typeof (((unsigned long)((page) - mem_map) + (((phys_ram_base) >> (12))))) *' (aka 'unsigned long long *') and 'typeof (start_pfn) *' (aka 'unsigned long *')) [-Wcompare-distinct-pointer-types]
check_unmovable_start = max(page_to_pfn(page), start_pfn);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/minmax.h:74:19: note: expanded from macro 'max'
#define max(x, y) __careful_cmp(x, y, >)
^~~~~~~~~~~~~~~~~~~~~~
include/linux/minmax.h:36:24: note: expanded from macro '__careful_cmp'
__builtin_choose_expr(__safe_cmp(x, y), \
^~~~~~~~~~~~~~~~
include/linux/minmax.h:26:4: note: expanded from macro '__safe_cmp'
(__typecheck(x, y) && __no_side_effects(x, y))
^~~~~~~~~~~~~~~~~
include/linux/minmax.h:20:28: note: expanded from macro '__typecheck'
(!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
>> mm/page_isolation.c:175:24: warning: comparison of distinct pointer types ('typeof ((((((((unsigned long)((page) - mem_map) + (((phys_ram_base) >> (12))))) + 1)) + ((typeof (((((unsigned long)((page) - mem_map) + (((phys_ram_base) >> (12))))) + 1)))(((1UL << __builtin_choose_expr(((!!(sizeof ((typeof ((unsigned int)((21 - (12)))) *)1 == (typeof ((unsigned int)(11 - 1)) *)1))) && ((sizeof(int) == sizeof (*(8 ? ((void *)((long)((unsigned int)((21 - (12)))) * 0L)) : (int *)8))) && (sizeof(int) == sizeof (*(8 ? ((void *)((long)((unsigned int)(11 - 1)) * 0L)) : (int *)8))))), (((unsigned int)((21 - (12)))) < ((unsigned int)(11 - 1)) ? ((unsigned int)((21 - (12)))) : ((unsigned int)(11 - 1))), ({
typeof ((unsigned int)((21 - (12)))) __UNIQUE_ID___x304 = ((unsigned int)((21 - (12))));
typeof ((unsigned int)(11 - 1)) __UNIQUE_ID___y305 = ((unsigned int)(11 - 1));
((__UNIQUE_ID___x304) < (__UNIQUE_ID___y305) ? (__UNIQUE_ID___x304) : (__UNIQUE_ID___y305));
}))))) - 1)) & ~((typeof (((((unsigned long)((page) - mem_map) + (((phys_ram_base) >> (12))))) + 1)))(((1UL << __builtin_choose_expr(((!!(sizeof ((typeof ((unsigned int)((21 - (12)))) *)1 == (typeof ((unsigned int)(11 - 1)) *)1))) && ((sizeof(int) == sizeof (*(8 ? ((void *)((long)((unsigned int)((21 - (12)))) * 0L)) : (int *)8))) && (sizeof(int) == sizeof (*(8 ? ((void *)((long)((unsigned int)(11 - 1)) * 0L)) : (int *)8))))), (((unsigned int)((21 - (12)))) < ((unsigned int)(11 - 1)) ? ((unsigned int)((21 - (12)))) : ((unsigned int)(11 - 1))), ({
typeof ((unsigned int)((21 - (12)))) __UNIQUE_ID___x304 = ((unsigned int)((21 - (12))));
typeof ((unsigned int)(11 - 1)) __UNIQUE_ID___y305 = ((unsigned int)(11 - 1));
((__UNIQUE_ID___x304) < (__UNIQUE_ID___y305) ? (__UNIQUE_ID___x304) : (__UNIQUE_ID___y305));
}))))) - 1))) *' (aka 'unsigned long long *') and 'typeof (end_pfn) *' (aka 'unsigned long *')) [-Wcompare-distinct-pointer-types]
check_unmovable_end = min(pageblock_end_pfn(page_to_pfn(page)),
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/minmax.h:67:19: note: expanded from macro 'min'
#define min(x, y) __careful_cmp(x, y, <)
^~~~~~~~~~~~~~~~~~~~~~
include/linux/minmax.h:36:24: note: expanded from macro '__careful_cmp'
__builtin_choose_expr(__safe_cmp(x, y), \
^~~~~~~~~~~~~~~~
include/linux/minmax.h:26:4: note: expanded from macro '__safe_cmp'
(__typecheck(x, y) && __no_side_effects(x, y))
^~~~~~~~~~~~~~~~~
include/linux/minmax.h:20:28: note: expanded from macro '__typecheck'
(!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
2 warnings generated.
--
>> drivers/mmc/host/usdhi6rol0.c:388:18: warning: format specifies type 'unsigned long' but the argument has type 'phys_addr_t' (aka 'unsigned long long') [-Wformat]
host->pg.page, page_to_pfn(host->pg.page), host->pg.mapped,
^~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:155:39: note: expanded from macro 'dev_dbg'
dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
include/linux/dynamic_debug.h:273:19: note: expanded from macro 'dynamic_dev_dbg'
dev, fmt, ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
include/linux/dynamic_debug.h:249:59: note: expanded from macro '_dynamic_func_call'
_dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
^~~~~~~~~~~
note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
include/linux/dynamic_debug.h:223:15: note: expanded from macro '__dynamic_func_call_cls'
func(&id, ##__VA_ARGS__); \
^~~~~~~~~~~
include/asm-generic/memory_model.h:52:21: note: expanded from macro 'page_to_pfn'
#define page_to_pfn __page_to_pfn
^
include/asm-generic/memory_model.h:19:29: note: expanded from macro '__page_to_pfn'
#define __page_to_pfn(page) ((unsigned long)((page) - mem_map) + \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/mmc/host/usdhi6rol0.c:511:18: warning: format specifies type 'unsigned long' but the argument has type 'phys_addr_t' (aka 'unsigned long long') [-Wformat]
host->pg.page, page_to_pfn(host->pg.page), host->pg.mapped,
^~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:155:39: note: expanded from macro 'dev_dbg'
dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
include/linux/dynamic_debug.h:273:19: note: expanded from macro 'dynamic_dev_dbg'
dev, fmt, ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
include/linux/dynamic_debug.h:249:59: note: expanded from macro '_dynamic_func_call'
_dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
^~~~~~~~~~~
note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
include/linux/dynamic_debug.h:223:15: note: expanded from macro '__dynamic_func_call_cls'
func(&id, ##__VA_ARGS__); \
^~~~~~~~~~~
include/asm-generic/memory_model.h:52:21: note: expanded from macro 'page_to_pfn'
#define page_to_pfn __page_to_pfn
^
include/asm-generic/memory_model.h:19:29: note: expanded from macro '__page_to_pfn'
#define __page_to_pfn(page) ((unsigned long)((page) - mem_map) + \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 warnings generated.

vim +174 mm/page_isolation.c

b48d8a8e5ce53e Zi Yan 2022-05-12 141
844fbae63e468e Zi Yan 2022-05-12 142 /*
844fbae63e468e Zi Yan 2022-05-12 143 * This function set pageblock migratetype to isolate if no unmovable page is
844fbae63e468e Zi Yan 2022-05-12 144 * present in [start_pfn, end_pfn). The pageblock must intersect with
844fbae63e468e Zi Yan 2022-05-12 145 * [start_pfn, end_pfn).
844fbae63e468e Zi Yan 2022-05-12 146 */
844fbae63e468e Zi Yan 2022-05-12 147 static int set_migratetype_isolate(struct page *page, int migratetype, int isol_flags,
844fbae63e468e Zi Yan 2022-05-12 148 unsigned long start_pfn, unsigned long end_pfn)
ee6f509c327401 Minchan Kim 2012-07-31 149 {
1c31cb493c3144 David Hildenbrand 2020-10-13 150 struct zone *zone = page_zone(page);
1c31cb493c3144 David Hildenbrand 2020-10-13 151 struct page *unmovable;
3f9903b9ca5e98 David Hildenbrand 2020-01-30 152 unsigned long flags;
844fbae63e468e Zi Yan 2022-05-12 153 unsigned long check_unmovable_start, check_unmovable_end;
ee6f509c327401 Minchan Kim 2012-07-31 154
ee6f509c327401 Minchan Kim 2012-07-31 155 spin_lock_irqsave(&zone->lock, flags);
ee6f509c327401 Minchan Kim 2012-07-31 156
2c7452a075d4db Mike Kravetz 2018-04-05 157 /*
2c7452a075d4db Mike Kravetz 2018-04-05 158 * We assume the caller intended to SET migrate type to isolate.
2c7452a075d4db Mike Kravetz 2018-04-05 159 * If it is already set, then someone else must have raced and
51030a53d81e30 David Hildenbrand 2020-10-13 160 * set it before us.
2c7452a075d4db Mike Kravetz 2018-04-05 161 */
51030a53d81e30 David Hildenbrand 2020-10-13 162 if (is_migrate_isolate_page(page)) {
51030a53d81e30 David Hildenbrand 2020-10-13 163 spin_unlock_irqrestore(&zone->lock, flags);
51030a53d81e30 David Hildenbrand 2020-10-13 164 return -EBUSY;
51030a53d81e30 David Hildenbrand 2020-10-13 165 }
2c7452a075d4db Mike Kravetz 2018-04-05 166
ee6f509c327401 Minchan Kim 2012-07-31 167 /*
ee6f509c327401 Minchan Kim 2012-07-31 168 * FIXME: Now, memory hotplug doesn't call shrink_slab() by itself.
ee6f509c327401 Minchan Kim 2012-07-31 169 * We just check MOVABLE pages.
844fbae63e468e Zi Yan 2022-05-12 170 *
844fbae63e468e Zi Yan 2022-05-12 171 * Pass the intersection of [start_pfn, end_pfn) and the page's pageblock
844fbae63e468e Zi Yan 2022-05-12 172 * to avoid redundant checks.
ee6f509c327401 Minchan Kim 2012-07-31 173 */
844fbae63e468e Zi Yan 2022-05-12 @174 check_unmovable_start = max(page_to_pfn(page), start_pfn);
4f9bc69ac5ce34 Kefeng Wang 2022-09-07 @175 check_unmovable_end = min(pageblock_end_pfn(page_to_pfn(page)),
844fbae63e468e Zi Yan 2022-05-12 176 end_pfn);
844fbae63e468e Zi Yan 2022-05-12 177
844fbae63e468e Zi Yan 2022-05-12 178 unmovable = has_unmovable_pages(check_unmovable_start, check_unmovable_end,
844fbae63e468e Zi Yan 2022-05-12 179 migratetype, isol_flags);
4a55c0474a92d5 Qian Cai 2020-01-30 180 if (!unmovable) {
2139cbe627b891 Bartlomiej Zolnierkiewicz 2012-10-08 181 unsigned long nr_pages;
4da2ce250f9860 Michal Hocko 2017-11-15 182 int mt = get_pageblock_migratetype(page);
2139cbe627b891 Bartlomiej Zolnierkiewicz 2012-10-08 183
a458431e176ddb Bartlomiej Zolnierkiewicz 2013-01-04 184 set_pageblock_migratetype(page, MIGRATE_ISOLATE);
ad53f92eb416d8 Joonsoo Kim 2014-11-13 185 zone->nr_isolate_pageblock++;
02aa0cdd72483c Vlastimil Babka 2017-05-08 186 nr_pages = move_freepages_block(zone, page, MIGRATE_ISOLATE,
02aa0cdd72483c Vlastimil Babka 2017-05-08 187 NULL);
2139cbe627b891 Bartlomiej Zolnierkiewicz 2012-10-08 188
4da2ce250f9860 Michal Hocko 2017-11-15 189 __mod_zone_freepage_state(zone, -nr_pages, mt);
1c31cb493c3144 David Hildenbrand 2020-10-13 190 spin_unlock_irqrestore(&zone->lock, flags);
1c31cb493c3144 David Hildenbrand 2020-10-13 191 return 0;
ee6f509c327401 Minchan Kim 2012-07-31 192 }
ee6f509c327401 Minchan Kim 2012-07-31 193
ee6f509c327401 Minchan Kim 2012-07-31 194 spin_unlock_irqrestore(&zone->lock, flags);
1c31cb493c3144 David Hildenbrand 2020-10-13 195 if (isol_flags & REPORT_FAILURE) {
4a55c0474a92d5 Qian Cai 2020-01-30 196 /*
3d680bdf60a5ba Qian Cai 2020-01-30 197 * printk() with zone->lock held will likely trigger a
4a55c0474a92d5 Qian Cai 2020-01-30 198 * lockdep splat, so defer it here.
4a55c0474a92d5 Qian Cai 2020-01-30 199 */
4a55c0474a92d5 Qian Cai 2020-01-30 200 dump_page(unmovable, "unmovable page");
3d680bdf60a5ba Qian Cai 2020-01-30 201 }
4a55c0474a92d5 Qian Cai 2020-01-30 202
1c31cb493c3144 David Hildenbrand 2020-10-13 203 return -EBUSY;
ee6f509c327401 Minchan Kim 2012-07-31 204 }
ee6f509c327401 Minchan Kim 2012-07-31 205

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

2023-01-28 14:59:52

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH v5 1/2] riscv: Get rid of riscv_pfn_base variable

2023-03-01 07:56:22

by Alexandre Ghiti

[permalink] [raw]

Subject: Re: [PATCH v5 1/2] riscv: Get rid of riscv_pfn_base variable

On 1/28/23 15:58, kernel test robot wrote:
> Hi Alexandre,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on robh/for-next]
> [also build test WARNING on linus/master v6.2-rc5 next-20230127]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Alexandre-Ghiti/riscv-Get-rid-of-riscv_pfn_base-variable/20230125-161537
> base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
> patch link: https://lore.kernel.org/r/20230125081214.1576313-2-alexghiti%40rivosinc.com
> patch subject: [PATCH v5 1/2] riscv: Get rid of riscv_pfn_base variable
> config: riscv-allnoconfig (https://download.01.org/0day-ci/archive/20230128/[email protected]/config)
> compiler: riscv64-linux-gcc (GCC) 12.1.0
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/intel-lab-lkp/linux/commit/90b21402dc8a7e6e36a62ad19c4969ff13fad168
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Alexandre-Ghiti/riscv-Get-rid-of-riscv_pfn_base-variable/20230125-161537
> git checkout 90b21402dc8a7e6e36a62ad19c4969ff13fad168
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=riscv olddefconfig
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <[email protected]>
>
> All warnings (new ones prefixed by >>):
>
> In file included from include/asm-generic/bug.h:22,
> from arch/riscv/include/asm/bug.h:83,
> from include/linux/bug.h:5,
> from arch/riscv/include/asm/cmpxchg.h:9,
> from arch/riscv/include/asm/atomic.h:19,
> from include/linux/atomic.h:7,
> from include/linux/jump_label.h:255,
> from arch/riscv/include/asm/vdso/processor.h:7,
> from include/vdso/processor.h:10,
> from arch/riscv/include/asm/processor.h:11,
> from arch/riscv/include/asm/irqflags.h:10,
> from include/linux/irqflags.h:16,
> from arch/riscv/include/asm/bitops.h:14,
> from include/linux/bitops.h:68,
> from include/linux/kernel.h:22,
> from mm/debug.c:9:
> mm/debug.c: In function '__dump_page':
>>> include/linux/kern_levels.h:5:25: warning: format '%lx' expects argument of type 'long unsigned int', but argument 7 has type 'long long unsigned int' [-Wformat=]
> 5 | #define KERN_SOH "\001" /* ASCII Start Of Header */
> | ^~~~~~
> include/linux/printk.h:429:25: note: in definition of macro 'printk_index_wrap'
> 429 | _p_func(_fmt, ##__VA_ARGS__); \
> | ^~~~
> include/linux/printk.h:510:9: note: in expansion of macro 'printk'
> 510 | printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
> | ^~~~~~
> include/linux/kern_levels.h:12:25: note: in expansion of macro 'KERN_SOH'
> 12 | #define KERN_WARNING KERN_SOH "4" /* warning conditions */
> | ^~~~~~~~
> include/linux/printk.h:510:16: note: in expansion of macro 'KERN_WARNING'
> 510 | printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
> | ^~~~~~~~~~~~
> mm/debug.c:93:9: note: in expansion of macro 'pr_warn'
> 93 | pr_warn("page:%p refcount:%d mapcount:%d mapping:%p index:%#lx pfn:%#lx\n",
> | ^~~~~~~
>
>
> vim +5 include/linux/kern_levels.h
>
> 314ba3520e513a Joe Perches 2012-07-30 4
> 04d2c8c83d0e3a Joe Perches 2012-07-30 @5 #define KERN_SOH "\001" /* ASCII Start Of Header */
> 04d2c8c83d0e3a Joe Perches 2012-07-30 6 #define KERN_SOH_ASCII '\001'
> 04d2c8c83d0e3a Joe Perches 2012-07-30 7
>

And this one was mine, sorry I i overlooked that!