Re: [PATCH] sh: remove unneeded constructor.

[Yoshinori Sato <ysato@users.sourceforge.jp>]

On Wed, 01 Aug 2018 16:42:02 +0900,
Geert Uytterhoeven wrote:
> 
> Hi Sato-san,
> 
> CC Rob , Willy, linux-arch
> 
> On Tue, Jul 31, 2018 at 7:15 AM Yoshinori Sato
> <ysato@users.sourceforge.jp> wrote:
> > pgd_cache specifies __GFP_ZERO when allocating.
> > This constructor is meaningless.
> >
> > Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> 
> Thanks for your patch!
> 
> > --- a/arch/sh/mm/pgtable.c
> > +++ b/arch/sh/mm/pgtable.c
> > @@ -9,20 +9,11 @@ static struct kmem_cache *pgd_cachep;
> >  static struct kmem_cache *pmd_cachep;
> >  #endif
> >
> > -void pgd_ctor(void *x)
> > -{
> > -       pgd_t *pgd = x;
> > -
> > -       memcpy(pgd + USER_PTRS_PER_PGD,
> > -              swapper_pg_dir + USER_PTRS_PER_PGD,
> > -              (PTRS_PER_PGD - USER_PTRS_PER_PGD) * sizeof(pgd_t));
> > -}
> > -
> >  void pgtable_cache_init(void)
> >  {
> >         pgd_cachep = kmem_cache_create("pgd_cache",
> >                                        PTRS_PER_PGD * (1<<PTE_MAGNITUDE),
> > -                                      PAGE_SIZE, SLAB_PANIC, pgd_ctor);
> > +                                      PAGE_SIZE, SLAB_PANIC, NULL);
> >  #if PAGETABLE_LEVELS > 2
> >         pmd_cachep = kmem_cache_create("pmd_cache",
> >                                        PTRS_PER_PMD * (1<<PTE_MAGNITUDE),
> 
> While I can confirm your patch fixes the
> 
>     WARNING: CPU: 0 PID: 1 at mm/slub.c:2412
> ___slab_alloc.constprop.34+0x196/0x288
> 
> seen since commit 128227e7fe4087b6 ("slab: __GFP_ZERO is incompatible
> with a constructor"), I'm not 100% sure this is the correct fix.

I tried it in myself, but with this fix I will not get a warning.
I do not think that it is related.

> swapper_pg_dir[] does have two non-zero entries (768 and 895), which were
> copied by the constructor.  Perhaps SH does have a need for these two
> entries, and thus for the constructor?
> 
> Unfortunately I'm too SH-illiterate to answer this myself.

I have not tested enough to impose mm on my part, so it may be
by chance this too.
Restore the constructor and modify it so that __ GFP_ZERO is not specified.
Because then I think that it is safer because it is exactly the same as
before the fix.

> 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
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Yosinori Sato