Re: [PATCH 2/4] powerpc/64s: Remove unneeded #ifdef CONFIG_DEBUG_PAGEALLOC in hash_utils

[Daniel Axtens <dja@axtens.net>]

Hi Jordan and Christophe,

> --- a/arch/powerpc/mm/book3s64/hash_utils.c
> +++ b/arch/powerpc/mm/book3s64/hash_utils.c
> @@ -126,11 +126,8 @@ EXPORT_SYMBOL_GPL(mmu_slb_size);
>  #ifdef CONFIG_PPC_64K_PAGES
>  int mmu_ci_restrictions;
>  #endif
> -#ifdef CONFIG_DEBUG_PAGEALLOC
>  static u8 *linear_map_hash_slots;
>  static unsigned long linear_map_hash_count;
> -static DEFINE_SPINLOCK(linear_map_hash_lock);
> -#endif /* CONFIG_DEBUG_PAGEALLOC */
>  struct mmu_hash_ops mmu_hash_ops;
>  EXPORT_SYMBOL(mmu_hash_ops);
>  

> @@ -1944,6 +1937,8 @@ long hpte_insert_repeating(unsigned long hash, unsigned long vpn,
>  }
>  
>  #ifdef CONFIG_DEBUG_PAGEALLOC
> +static DEFINE_SPINLOCK(linear_map_hash_lock);
> +

I had some trouble figuring out why the spinlock has to be in the
ifdef. A bit of investigation suggests that it's only used in functions
that are only defined under CONFIG_DEBUG_PAGEALLOC - unlike the other
variables. So that makes sense.

While I was poking around, I noticed that linear_map_hash_slots is
manipulated under linear_map_hash_lock in kernel_(un)map_linear_page but
is manipulated outside the lock in htab_bolt_mapping(). Is that OK? (I
don't know when htab_bolt_mapping is called, it's possible it's only
called at times where nothing else could be happing to that array.)

Kind regards,
Daniel

>  static void kernel_map_linear_page(unsigned long vaddr, unsigned long lmi)
>  {
>  	unsigned long hash;
> -- 
> 2.25.1