Re: [PATCH 4.4 20/43] mm/vmalloc: add interfaces to free unmapped page table

From: Nathan Chancellor <natechancellor@gmail.com>
To: "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>
Cc: "Kani, Toshi" <toshi.kani@hpe.com>,
	"dan.rue@linaro.org" <dan.rue@linaro.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"torvalds@linux-foundation.org" <torvalds@linux-foundation.org>,
	"bp@suse.de" <bp@suse.de>,
	"tglx@linutronix.de" <tglx@linutronix.de>,
	"lious.lilei@hisilicon.com" <lious.lilei@hisilicon.com>,
	"guohanjun@huawei.com" <guohanjun@huawei.com>,
	"wxf.wang@hisilicon.com" <wxf.wang@hisilicon.com>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"hpa@zytor.com" <hpa@zytor.com>,
	"will.deacon@arm.com" <will.deacon@arm.com>,
	"catalin.marinas@arm.com" <catalin.marinas@arm.com>,
	"mingo@redhat.com" <mingo@redhat.com>,
	"Hocko, Michal" <MHocko@suse.com>,
	"cpandya@codeaurora.org" <cpandya@codeaurora.org>,
	"willy@infradead.org" <willy@infradead.org>
Subject: Re: [PATCH 4.4 20/43] mm/vmalloc: add interfaces to free unmapped page table
Date: Tue, 27 Mar 2018 23:47:18 -0700	[thread overview]
Message-ID: <20180328064718.GA31963@flashbox> (raw)
In-Reply-To: <20180328063202.GB9547@kroah.com>

On Wed, Mar 28, 2018 at 08:32:02AM +0200, gregkh@linuxfoundation.org wrote:
> On Tue, Mar 27, 2018 at 01:47:55PM -0700, Nathan Chancellor wrote:
> > On Tue, Mar 27, 2018 at 08:40:56PM +0000, Kani, Toshi wrote:
> > > On Tue, 2018-03-27 at 13:31 -0700, Nathan Chancellor wrote:
> > > > On Tue, Mar 27, 2018 at 03:17:00PM -0500, Dan Rue wrote:
> > > > > On Tue, Mar 27, 2018 at 06:27:24PM +0200, Greg Kroah-Hartman wrote:
> > > > > > 4.4-stable review patch.  If anyone has any objections, please let me know.
> > > > > > 
> > >  :
> > > > > 
> > > > > This patch causes the following build error on 4.4 arm64:
> > > > > 
> > > > > $ make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- O=build-arm64 defconfig
> > > > > $ make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- O=build-arm64
> > > > > 
> > > > >   CC      arch/arm64/mm/mmu.o
> > > > > ../arch/arm64/mm/mmu.c:701:5: error: redefinition of ‘pud_free_pmd_page’
> > > > >  int pud_free_pmd_page(pud_t *pud)
> > > > >      ^~~~~~~~~~~~~~~~~
> > > > > In file included from ../arch/arm64/include/asm/pgtable.h:682:0,
> > > > >                  from ../include/linux/mm.h:55,
> > > > >                  from ../include/linux/mman.h:4,
> > > > >                  from ../arch/arm64/mm/mmu.c:25:
> > > > > ../include/asm-generic/pgtable.h:777:19: note: previous definition of ‘pud_free_pmd_page’ was here
> > > > >  static inline int pud_free_pmd_page(pud_t *pud)
> > > > >                    ^~~~~~~~~~~~~~~~~
> > > > > ../arch/arm64/mm/mmu.c:706:5: error: redefinition of ‘pmd_free_pte_page’
> > > > >  int pmd_free_pte_page(pmd_t *pmd)
> > > > >      ^~~~~~~~~~~~~~~~~
> > > > > In file included from ../arch/arm64/include/asm/pgtable.h:682:0,
> > > > >                  from ../include/linux/mm.h:55,
> > > > >                  from ../include/linux/mman.h:4,
> > > > >                  from ../arch/arm64/mm/mmu.c:25:
> > > > > ../include/asm-generic/pgtable.h:781:19: note: previous definition of ‘pmd_free_pte_page’ was here
> > > > >  static inline int pmd_free_pte_page(pmd_t *pmd)
> > > > >                    ^~~~~~~~~~~~~~~~~
> > > > > make[2]: *** [../scripts/Makefile.build:270: arch/arm64/mm/mmu.o] Error 1
> > > > > make[1]: *** [/home/drue/src/linux/4.4-rc/Makefile:969: arch/arm64/mm] Error 2
> > > > > make[1]: Leaving directory '/home/drue/src/linux/4.4-rc/build-arm64'
> > > > > make: *** [Makefile:152: sub-make] Error 2
> > > > > 
> > > > > 
> > > > 
> > > > Both of my arm64 devices built fine with this patch... It seems like
> > > > the only way to hit that error is if HAVE_ARCH_HUGE_VMAP isn't set,
> > > > which seems impossible since it is selected by ARM64...
> > > > 
> > > > Someone smarter than I might have more insight but this patch is
> > > > unchanged from upstream so I can only assume that this error would
> > > > manifest there as well.
> > > 
> > > It appears that HAVE_ARCH_HUGE_VMAP was introduced in 4.6 on arm64. 
> > > Hence the problem in 4.4.
> > > 
> > > Thanks,
> > > -Toshi
> > > 
> > 
> > Ah, thanks for the heads up, since I have 324420bf91f6 ("arm64: add
> > support for ioremap() block mappings") in my tree due to Linaro's
> > backport of it for their Linaro Stable Kernel, which serves as a base
> > for most Android kernels. My apologies for not digging deeper and sorry
> > for the noise!
> 
> So should I just drop this patch?

Unless I am reading the commit message wrong and ignoring the fact that
the mainling commit applied cleanly to 4.4.124, it seems like this is
still relevant for x86.

Toshi suggested dropping the changes to arch/arm64/mm/mmu.c, which won't
be a problem for arm64 devices running the stable kernels as they come
because they don't have HAVE_ARCH_HUGE_VMAP so they shouldn't be hitting
the bug mentioned in the commit message anyways.

However, for Android devices which have the mainline commit I mentioned
above introducing HAVE_ARCH_HUGE_VMAP, dropping those changes would mean
this commit isn't fixing the issue it mentions, which they would be able
to hit in theory.

I don't know how you want to handle that but a simple suggestion that
would not change the end result of the patch for both the stable tree
and frankenkernels would be to add a simple

#ifdef CONFIG_ARCH_HAVE_HUGE_VMAP

around the changes in arch/arm64/mm/mmu.c.

I'll let you be the final judge though! Cheers,
Nathan