[PATCH 1/1] mm: vmalloc: Fix a warning in the crash_save_vmcoreinfo_init()

[PATCH 1/1] mm: vmalloc: Fix a warning in the crash_save_vmcoreinfo_init()

[Uladzislau Rezki (Sony)]

The vmcoreinfo_append_str() function expects "long unsigned int"
type as a second argument(0x%lx) to print a beginning of vmalloc
start address which is defined as a VMALLOC_START macro.

For some architectures it can be considered as "int" type, for
example m68 generates a compile warning message. To fix it cast
a second argument to "unsigned long".

Fixes: 9bdb180b2d ("mm/vmalloc: remove vmap_area_list")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202401120218.y469Puyf-lkp@intel.com/
Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
---
 kernel/crash_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index b60de490c1fc..49b31e59d3cc 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -748,7 +748,7 @@ static int __init crash_save_vmcoreinfo_init(void)
 	VMCOREINFO_SYMBOL_ARRAY(swapper_pg_dir);
 #endif
 	VMCOREINFO_SYMBOL(_stext);
-	vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", VMALLOC_START);
+	vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", (unsigned long) VMALLOC_START);
 
 #ifndef CONFIG_NUMA
 	VMCOREINFO_SYMBOL(mem_map);
-- 
2.39.2

Re: [PATCH 1/1] mm: vmalloc: Fix a warning in the crash_save_vmcoreinfo_init()

[Baoquan He]

On 01/11/24 at 08:23pm, Uladzislau Rezki (Sony) wrote:
> The vmcoreinfo_append_str() function expects "long unsigned int"
> type as a second argument(0x%lx) to print a beginning of vmalloc
> start address which is defined as a VMALLOC_START macro.
> 
> For some architectures it can be considered as "int" type, for
> example m68 generates a compile warning message. To fix it cast
> a second argument to "unsigned long".
> 
> Fixes: 9bdb180b2d ("mm/vmalloc: remove vmap_area_list")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202401120218.y469Puyf-lkp@intel.com/
> Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> ---
>  kernel/crash_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index b60de490c1fc..49b31e59d3cc 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -748,7 +748,7 @@ static int __init crash_save_vmcoreinfo_init(void)
>  	VMCOREINFO_SYMBOL_ARRAY(swapper_pg_dir);
>  #endif
>  	VMCOREINFO_SYMBOL(_stext);
> -	vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", VMALLOC_START);
> +	vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", (unsigned long) VMALLOC_START);

Thanks for the fix.

Acked-by: Baoquan He <bhe@redhat.com>

>  
>  #ifndef CONFIG_NUMA
>  	VMCOREINFO_SYMBOL(mem_map);
> -- 
> 2.39.2
> 

Re: [PATCH 1/1] mm: vmalloc: Fix a warning in the crash_save_vmcoreinfo_init()

[Christoph Hellwig]

On Thu, Jan 11, 2024 at 08:23:29PM +0100, Uladzislau Rezki (Sony) wrote:
>  #endif
>  	VMCOREINFO_SYMBOL(_stext);
> -	vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", VMALLOC_START);
> +	vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", (unsigned long) VMALLOC_START);

Well, the right fix is of course to make sure VMALLOC_START has a
consistent type, else we need to plaster this crud all over.
unsigned long seems like the right type for it, so at least m68k should
be fixed to confirm to that by adding a UL postfix to the definition.

Re: [PATCH 1/1] mm: vmalloc: Fix a warning in the crash_save_vmcoreinfo_init()

[Uladzislau Rezki]

> On Thu, Jan 11, 2024 at 08:23:29PM +0100, Uladzislau Rezki (Sony) wrote:
> >  #endif
> >  	VMCOREINFO_SYMBOL(_stext);
> > -	vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", VMALLOC_START);
> > +	vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", (unsigned long) VMALLOC_START);
> 
> Well, the right fix is of course to make sure VMALLOC_START has a
> consistent type, else we need to plaster this crud all over.
> unsigned long seems like the right type for it, so at least m68k should
> be fixed to confirm to that by adding a UL postfix to the definition.
> 
I agree with you. I wanted to focus on fixing that particular place
because i wanted to avoid other(on this step), possible side effects
or drawbacks if i went with patching the arch/m68k/* files.

But, in general arch/m68k/* has to be fixed.

--
Uladzislau Rezki

Re: [PATCH 1/1] mm: vmalloc: Fix a warning in the crash_save_vmcoreinfo_init()

[Lorenzo Stoakes]

On Thu, Jan 11, 2024 at 08:23:29PM +0100, Uladzislau Rezki (Sony) wrote:
> The vmcoreinfo_append_str() function expects "long unsigned int"
> type as a second argument(0x%lx) to print a beginning of vmalloc
> start address which is defined as a VMALLOC_START macro.
>
> For some architectures it can be considered as "int" type, for
> example m68 generates a compile warning message. To fix it cast
> a second argument to "unsigned long".

Ugh dear Lord. Insert grumble about maintenance costs surrounding hobbyist
architectures. I agree with Christoph the correct fix is within m68 (and
any others with inconsistent typing).

But for now we do need to work around this.

>
> Fixes: 9bdb180b2d ("mm/vmalloc: remove vmap_area_list")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202401120218.y469Puyf-lkp@intel.com/
> Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> ---
>  kernel/crash_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index b60de490c1fc..49b31e59d3cc 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -748,7 +748,7 @@ static int __init crash_save_vmcoreinfo_init(void)
>  	VMCOREINFO_SYMBOL_ARRAY(swapper_pg_dir);
>  #endif
>  	VMCOREINFO_SYMBOL(_stext);
> -	vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", VMALLOC_START);
> +	vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", (unsigned long) VMALLOC_START);
>
>  #ifndef CONFIG_NUMA
>  	VMCOREINFO_SYMBOL(mem_map);
> --
> 2.39.2
>

We definitely need to get this sorted for the bots, though moving forward
we should patch this problem correctly in the right arch. However for the
purposes of the fix feel free to add:

Reviewed-by: Lorenzo Stoakes <lstoakes@gmail.com>

Re: [PATCH 1/1] mm: vmalloc: Fix a warning in the crash_save_vmcoreinfo_init()

[Anshuman Khandual]

On 1/12/24 00:53, Uladzislau Rezki (Sony) wrote:
> The vmcoreinfo_append_str() function expects "long unsigned int"
> type as a second argument(0x%lx) to print a beginning of vmalloc
> start address which is defined as a VMALLOC_START macro.
> 
> For some architectures it can be considered as "int" type, for
> example m68 generates a compile warning message. To fix it cast
> a second argument to "unsigned long".
> 
> Fixes: 9bdb180b2d ("mm/vmalloc: remove vmap_area_list")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202401120218.y469Puyf-lkp@intel.com/
> Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> ---
>  kernel/crash_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index b60de490c1fc..49b31e59d3cc 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -748,7 +748,7 @@ static int __init crash_save_vmcoreinfo_init(void)
>  	VMCOREINFO_SYMBOL_ARRAY(swapper_pg_dir);
>  #endif
>  	VMCOREINFO_SYMBOL(_stext);
> -	vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", VMALLOC_START);
> +	vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", (unsigned long) VMALLOC_START);
>  
>  #ifndef CONFIG_NUMA
>  	VMCOREINFO_SYMBOL(mem_map);

Agree with Christoph - the right place to have this fixed properly is in m68k
platform while defining VMALLOC_START. But otherwise this fix in itself LGTM.

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

Re: [PATCH 1/1] mm: vmalloc: Fix a warning in the crash_save_vmcoreinfo_init()

[Geert Uytterhoeven]

CC linux-m68k

On Fri, Jan 12, 2024 at 9:11 AM Uladzislau Rezki <urezki@gmail.com> wrote:
> > On Thu, Jan 11, 2024 at 08:23:29PM +0100, Uladzislau Rezki (Sony) wrote:
> > >  #endif
> > >     VMCOREINFO_SYMBOL(_stext);
> > > -   vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", VMALLOC_START);
> > > +   vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", (unsigned long) VMALLOC_START);
> >
> > Well, the right fix is of course to make sure VMALLOC_START has a
> > consistent type, else we need to plaster this crud all over.
> > unsigned long seems like the right type for it, so at least m68k should
> > be fixed to confirm to that by adding a UL postfix to the definition.
> >
> I agree with you. I wanted to focus on fixing that particular place
> because i wanted to avoid other(on this step), possible side effects
> or drawbacks if i went with patching the arch/m68k/* files.
>
> But, in general arch/m68k/* has to be fixed.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds