Re: [PATCH v3 4/4] slub: Force on no_hash_pointers when slub_debug is enabled

[Stephen Boyd]

Quoting Andrew Morton (2021-06-01 17:26:59)
> On Wed, 2 Jun 2021 06:45:55 +0800 kernel test robot <lkp@xxxxxxxxx> wrote:
>
> > >> mm/slub.c:4464:30: error: use of undeclared identifier 'slub_debug_enabled'
> >            if (static_branch_unlikely(&slub_debug_enabled))
> >                                        ^
> > >> mm/slub.c:4464:30: error: use of undeclared identifier 'slub_debug_enabled'
> > >> mm/slub.c:4464:30: error: use of undeclared identifier 'slub_debug_enabled'
> > >> mm/slub.c:4464:30: error: use of undeclared identifier 'slub_debug_enabled'
> > >> mm/slub.c:4464:6: error: invalid argument type 'void' to unary expression
> >            if (static_branch_unlikely(&slub_debug_enabled))
> >                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Thanks.  Stephen, how about this?

Looks good to me. Thanks for the quick fix!

>
> --- a/mm/slub.c~slub-force-on-no_hash_pointers-when-slub_debug-is-enabled-fix
> +++ a/mm/slub.c
> @@ -117,12 +117,26 @@
>   */
>
>  #ifdef CONFIG_SLUB_DEBUG
> +
>  #ifdef CONFIG_SLUB_DEBUG_ON
>  DEFINE_STATIC_KEY_TRUE(slub_debug_enabled);
>  #else
>  DEFINE_STATIC_KEY_FALSE(slub_debug_enabled);
>  #endif
> -#endif
> +
> +static inline bool __slub_debug_enabled(void)
> +{
> +       return static_branch_unlikely(&slub_debug_enabled);

To make this even better it could be

	return static_branch_maybe(CONFIG_SLUB_DEBUG_ON, &slub_debug_enabled);

> +}
> +
> +#else          /* CONFIG_SLUB_DEBUG */
> +
> +static inline bool __slub_debug_enabled(void)
> +{
> +       return false;
> +}
> +
> +#endif         /* CONFIG_SLUB_DEBUG */
>
>  static inline bool kmem_cache_debug(struct kmem_cache *s)
>  {
> @@ -4493,7 +4507,7 @@ void __init kmem_cache_init(void)
>                 slub_max_order = 0;
>
>         /* Print slub debugging pointers without hashing */
> -       if (static_branch_unlikely(&slub_debug_enabled))
> +       if (__slub_debug_enabled())

It would be super cool if static branches could be optimized out when
they're never changed by any code, nor exported to code, just tested in
conditions. I've no idea if that is the case though.

>                 no_hash_pointers_enable(NULL);
>
>         kmem_cache_node = &boot_kmem_cache_node;