Re: [PATCH] sh: remove unneeded constructor.

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:

quoted

Hi Sato-san,

CC Rob , Willy, linux-arch

Thanks!

quoted

quoted

-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