Re: [PATCH] riscv: mm: fix 2 instances of -Wmissing-variable-declarations

From: Nathan Chancellor <nathan@kernel.org>
To: Nick Desaulniers <ndesaulniers@google.com>
Cc: Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>, Tom Rix <trix@redhat.com>,
	linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
	llvm@lists.linux.dev, kernel test robot <lkp@intel.com>
Subject: Re: [PATCH] riscv: mm: fix 2 instances of -Wmissing-variable-declarations
Date: Tue, 8 Aug 2023 09:24:12 -0700	[thread overview]
Message-ID: <20230808162412.GA2172017@dev-arch.thelio-3990X> (raw)
In-Reply-To: <20230808-riscv_static-v1-1-9f3dc99dafe8@google.com>

On Tue, Aug 08, 2023 at 09:15:41AM -0700, Nick Desaulniers wrote:
> I'm looking to enable -Wmissing-variable-declarations behind W=1. 0day
> bot spotted the following instance in ARCH=riscv builds:
> 
>   arch/riscv/mm/init.c:276:7: warning: no previous extern declaration
>   for non-static variable 'trampoline_pg_dir'
>   [-Wmissing-variable-declarations]
>   276 | pgd_t trampoline_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
>       |       ^
>   arch/riscv/mm/init.c:276:1: note: declare 'static' if the variable is
>   not intended to be used outside of this translation unit
>   276 | pgd_t trampoline_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
>       | ^
>   arch/riscv/mm/init.c:279:7: warning: no previous extern declaration
>   for non-static variable 'early_pg_dir'
>   [-Wmissing-variable-declarations]
>   279 | pgd_t early_pg_dir[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE);
>       |       ^
>   arch/riscv/mm/init.c:279:1: note: declare 'static' if the variable is
>   not intended to be used outside of this translation unit
>   279 | pgd_t early_pg_dir[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE);
>       | ^
> 
> These symbols are referenced by more than one translation unit, so make
> sure they're both declared and include the correct header for their
> declarations. Finally, sort the list of includes to help keep them tidy.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/llvm/202308081000.tTL1ElTr-lkp@intel.com/
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>  arch/riscv/include/asm/pgtable.h | 1 +
>  arch/riscv/mm/init.c             | 9 +++++----
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> index 75970ee2bda2..4c8c0f83a974 100644
> --- a/arch/riscv/include/asm/pgtable.h
> +++ b/arch/riscv/include/asm/pgtable.h
> @@ -188,6 +188,7 @@ extern struct pt_alloc_ops pt_ops __initdata;
>  #define PAGE_KERNEL_IO		__pgprot(_PAGE_IOREMAP)
>  
>  extern pgd_t swapper_pg_dir[];
> +extern pgd_t trampoline_pg_dir[];

Missing early_pg_dir too? It looks like there is already a declaration
in arch/riscv/mm/kasan_init.c that could be hoisted here?

>  
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>  static inline int pmd_present(pmd_t pmd)
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 9ce504737d18..cc0e06b4f223 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -26,12 +26,13 @@
>  #include <linux/kfence.h>
>  
>  #include <asm/fixmap.h>
> -#include <asm/tlbflush.h>
> -#include <asm/sections.h>
> -#include <asm/soc.h>
>  #include <asm/io.h>
> -#include <asm/ptdump.h>
>  #include <asm/numa.h>
> +#include <asm/pgtable.h>
> +#include <asm/ptdump.h>
> +#include <asm/sections.h>
> +#include <asm/soc.h>
> +#include <asm/tlbflush.h>
>  
>  #include "../kernel/head.h"
>  
> 
> ---
> base-commit: 14f9643dc90adea074a0ffb7a17d337eafc6a5cc
> change-id: 20230808-riscv_static-348036edcae7
> 
> Best regards,
> -- 
> Nick Desaulniers <ndesaulniers@google.com>
> 