Re: [PATCH] sh: remove unneeded constructor.

[Rich Felker <dalias@libc.org>]

On Wed, Aug 01, 2018 at 04:20:19AM -0700, Matthew Wilcox wrote:
> On Wed, Aug 01, 2018 at 09:42:02AM +0200, Geert Uytterhoeven wrote:
> > Hi Sato-san,
> > 
> > CC Rob , Willy, linux-arch
> 
> Thanks!
> 
> > > -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.
> > 
> > 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?
> 
> __GFP_ZERO overrode the constructor.  That is, before 128227e7fe40,
> if you specified both a constructor and __GFP_ZERO, first the slab
> code would invoke the constructor, then it would zero the allocation.
> So this patch is preserving the existing behaviour.  Whether the existing
> behaviour is correct or not, I cannot say.

Then I think we should really try to figure out whether this is a
buried bug before deleting the evidence of it...

Rich