From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matthew Wilcox Date: Wed, 01 Aug 2018 11:20:19 +0000 Subject: Re: [PATCH] sh: remove unneeded constructor. Message-Id: <20180801112019.GA2666@bombadil.infradead.org> List-Id: References: <20180731051519.101249-1-ysato@users.sourceforge.jp> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Geert Uytterhoeven Cc: Yoshinori Sato , Rich Felker , Linux-sh list , Rob Landley , Linux-Arch 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< > - 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< > 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. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from bombadil.infradead.org ([198.137.202.133]:37236 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2387610AbeHANFl (ORCPT ); Wed, 1 Aug 2018 09:05:41 -0400 Date: Wed, 1 Aug 2018 04:20:19 -0700 From: Matthew Wilcox Subject: Re: [PATCH] sh: remove unneeded constructor. Message-ID: <20180801112019.GA2666@bombadil.infradead.org> References: <20180731051519.101249-1-ysato@users.sourceforge.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-arch-owner@vger.kernel.org List-ID: To: Geert Uytterhoeven Cc: Yoshinori Sato , Rich Felker , Linux-sh list , Rob Landley , Linux-Arch 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< > - 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< > 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.