* [PATCH] arm64: define __alloc_zeroed_user_highpage @ 2020-03-12 15:59 glider 2020-03-12 16:49 ` Mark Rutland 2020-03-17 18:37 ` Catalin Marinas 0 siblings, 2 replies; 5+ messages in thread From: glider @ 2020-03-12 15:59 UTC (permalink / raw) To: catalin.marinas, will.deacon, mark.rutland Cc: akpm, Alexander Potapenko, linux-kernel, linux-arm-kernel, keescook When running the kernel with init_on_alloc=1, calling the default implementation of __alloc_zeroed_user_highpage() from include/linux/highmem.h leads to double-initialization of the allocated page (first by the page allocator, then by clear_user_page(). Calling alloc_page_vma() with __GFP_ZERO, similarly to e.g. x86, seems to be enough to ensure the user page is zeroed only once. Signed-off-by: Alexander Potapenko <glider@google.com> --- arch/arm64/include/asm/page.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/arch/arm64/include/asm/page.h b/arch/arm64/include/asm/page.h index d39ddb258a049..75d6cd23a6790 100644 --- a/arch/arm64/include/asm/page.h +++ b/arch/arm64/include/asm/page.h @@ -21,6 +21,10 @@ extern void __cpu_copy_user_page(void *to, const void *from, extern void copy_page(void *to, const void *from); extern void clear_page(void *to); +#define __alloc_zeroed_user_highpage(movableflags, vma, vaddr) \ + alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO | movableflags, vma, vaddr) +#define __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE + #define clear_user_page(addr,vaddr,pg) __cpu_clear_user_page(addr, vaddr) #define copy_user_page(to,from,vaddr,pg) __cpu_copy_user_page(to, from, vaddr) -- 2.25.1.481.gfbce0eb801-goog _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64: define __alloc_zeroed_user_highpage 2020-03-12 15:59 [PATCH] arm64: define __alloc_zeroed_user_highpage glider @ 2020-03-12 16:49 ` Mark Rutland 2020-03-12 19:59 ` Alexander Potapenko 2020-03-17 18:37 ` Catalin Marinas 1 sibling, 1 reply; 5+ messages in thread From: Mark Rutland @ 2020-03-12 16:49 UTC (permalink / raw) To: glider Cc: keescook, catalin.marinas, will.deacon, linux-kernel, akpm, linux-arm-kernel On Thu, Mar 12, 2020 at 04:59:20PM +0100, glider@google.com wrote: > When running the kernel with init_on_alloc=1, calling the default > implementation of __alloc_zeroed_user_highpage() from include/linux/highmem.h > leads to double-initialization of the allocated page (first by the page > allocator, then by clear_user_page(). > Calling alloc_page_vma() with __GFP_ZERO, similarly to e.g. x86, seems > to be enough to ensure the user page is zeroed only once. Just to check, is there a functional ussue beyond the redundant zeroing, or is this jsut a performance issue? On architectures with real highmem, does GFP_HIGHUSER prevent the allocator from zeroing the page in this case, or is the architecture prevented from allocating from highmem? This feels like something we should be able to fix in the generic implementation of __alloc_zeroed_user_highpage(), with an additional check to see if init_on_alloc is in use. Thanks, Mark. > > Signed-off-by: Alexander Potapenko <glider@google.com> > --- > arch/arm64/include/asm/page.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/arm64/include/asm/page.h b/arch/arm64/include/asm/page.h > index d39ddb258a049..75d6cd23a6790 100644 > --- a/arch/arm64/include/asm/page.h > +++ b/arch/arm64/include/asm/page.h > @@ -21,6 +21,10 @@ extern void __cpu_copy_user_page(void *to, const void *from, > extern void copy_page(void *to, const void *from); > extern void clear_page(void *to); > > +#define __alloc_zeroed_user_highpage(movableflags, vma, vaddr) \ > + alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO | movableflags, vma, vaddr) > +#define __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE > + > #define clear_user_page(addr,vaddr,pg) __cpu_clear_user_page(addr, vaddr) > #define copy_user_page(to,from,vaddr,pg) __cpu_copy_user_page(to, from, vaddr) > > -- > 2.25.1.481.gfbce0eb801-goog > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64: define __alloc_zeroed_user_highpage 2020-03-12 16:49 ` Mark Rutland @ 2020-03-12 19:59 ` Alexander Potapenko 2020-03-13 15:03 ` Catalin Marinas 0 siblings, 1 reply; 5+ messages in thread From: Alexander Potapenko @ 2020-03-12 19:59 UTC (permalink / raw) To: Mark Rutland Cc: Kees Cook, Catalin Marinas, Will Deacon, LKML, Andrew Morton, linux-arm-kernel On Thu, Mar 12, 2020 at 5:49 PM Mark Rutland <mark.rutland@arm.com> wrote: > > On Thu, Mar 12, 2020 at 04:59:20PM +0100, glider@google.com wrote: > > When running the kernel with init_on_alloc=1, calling the default > > implementation of __alloc_zeroed_user_highpage() from include/linux/highmem.h > > leads to double-initialization of the allocated page (first by the page > > allocator, then by clear_user_page(). > > Calling alloc_page_vma() with __GFP_ZERO, similarly to e.g. x86, seems > > to be enough to ensure the user page is zeroed only once. > > Just to check, is there a functional ussue beyond the redundant zeroing, > or is this jsut a performance issue? This is just a performance issue that only manifests when running the kernel with init_on_alloc=1. > On architectures with real highmem, does GFP_HIGHUSER prevent the > allocator from zeroing the page in this case, or is the architecture > prevented from allocating from highmem? I was hoping one of ARM maintainers can answer this question. My understanding was that __GFP_ZERO should be sufficient, but there's probably something I'm missing. > Thanks, > Mark. > > > > > Signed-off-by: Alexander Potapenko <glider@google.com> > > --- > > arch/arm64/include/asm/page.h | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/arch/arm64/include/asm/page.h b/arch/arm64/include/asm/page.h > > index d39ddb258a049..75d6cd23a6790 100644 > > --- a/arch/arm64/include/asm/page.h > > +++ b/arch/arm64/include/asm/page.h > > @@ -21,6 +21,10 @@ extern void __cpu_copy_user_page(void *to, const void *from, > > extern void copy_page(void *to, const void *from); > > extern void clear_page(void *to); > > > > +#define __alloc_zeroed_user_highpage(movableflags, vma, vaddr) \ > > + alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO | movableflags, vma, vaddr) > > +#define __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE > > + > > #define clear_user_page(addr,vaddr,pg) __cpu_clear_user_page(addr, vaddr) > > #define copy_user_page(to,from,vaddr,pg) __cpu_copy_user_page(to, from, vaddr) > > > > -- > > 2.25.1.481.gfbce0eb801-goog > > -- Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Straße, 33 80636 München Geschäftsführer: Paul Manicle, Halimah DeLaine Prado Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64: define __alloc_zeroed_user_highpage 2020-03-12 19:59 ` Alexander Potapenko @ 2020-03-13 15:03 ` Catalin Marinas 0 siblings, 0 replies; 5+ messages in thread From: Catalin Marinas @ 2020-03-13 15:03 UTC (permalink / raw) To: Alexander Potapenko Cc: Mark Rutland, Kees Cook, Will Deacon, LKML, Andrew Morton, linux-arm-kernel On Thu, Mar 12, 2020 at 08:59:28PM +0100, Alexander Potapenko wrote: > On Thu, Mar 12, 2020 at 5:49 PM Mark Rutland <mark.rutland@arm.com> wrote: > > > > On Thu, Mar 12, 2020 at 04:59:20PM +0100, glider@google.com wrote: > > > When running the kernel with init_on_alloc=1, calling the default > > > implementation of __alloc_zeroed_user_highpage() from include/linux/highmem.h > > > leads to double-initialization of the allocated page (first by the page > > > allocator, then by clear_user_page(). > > > Calling alloc_page_vma() with __GFP_ZERO, similarly to e.g. x86, seems > > > to be enough to ensure the user page is zeroed only once. > > > > Just to check, is there a functional ussue beyond the redundant zeroing, > > or is this jsut a performance issue? > > This is just a performance issue that only manifests when running the > kernel with init_on_alloc=1. > > > On architectures with real highmem, does GFP_HIGHUSER prevent the > > allocator from zeroing the page in this case, or is the architecture > > prevented from allocating from highmem? > > I was hoping one of ARM maintainers can answer this question. My > understanding was that __GFP_ZERO should be sufficient, but there's > probably something I'm missing. On architectures with aliasing D-cache (whether it's VIVT or aliasing VIPT), clear_user_highpage() ensures that the correct alias, as seen by the user, is cleared (see the arm32 v6_clear_user_highpage_aliasing() as an example). The clear_highpage() call as done by page_alloc.c does not have the user address information, so it can only clear the kernel alias. On arm64 we don't have such issue, so we can optimise this case as per your patch. We may change this function later with MTE if we allow tags other than 0 on the first allocation of anonymous pages. -- Catalin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64: define __alloc_zeroed_user_highpage 2020-03-12 15:59 [PATCH] arm64: define __alloc_zeroed_user_highpage glider 2020-03-12 16:49 ` Mark Rutland @ 2020-03-17 18:37 ` Catalin Marinas 1 sibling, 0 replies; 5+ messages in thread From: Catalin Marinas @ 2020-03-17 18:37 UTC (permalink / raw) To: glider Cc: mark.rutland, keescook, will.deacon, linux-kernel, akpm, linux-arm-kernel On Thu, Mar 12, 2020 at 04:59:20PM +0100, glider@google.com wrote: > When running the kernel with init_on_alloc=1, calling the default > implementation of __alloc_zeroed_user_highpage() from include/linux/highmem.h > leads to double-initialization of the allocated page (first by the page > allocator, then by clear_user_page(). > Calling alloc_page_vma() with __GFP_ZERO, similarly to e.g. x86, seems > to be enough to ensure the user page is zeroed only once. > > Signed-off-by: Alexander Potapenko <glider@google.com> I queued this for 5.7. Thanks. -- Catalin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-03-17 18:37 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-03-12 15:59 [PATCH] arm64: define __alloc_zeroed_user_highpage glider 2020-03-12 16:49 ` Mark Rutland 2020-03-12 19:59 ` Alexander Potapenko 2020-03-13 15:03 ` Catalin Marinas 2020-03-17 18:37 ` Catalin Marinas
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).