Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1303599 > unrolled thread

Re: [PATCH v2 03/13] arm64: use more granular reservations for static page table allocations

Started byMark Rutland <mark.rutland@arm.com>
First post2016-01-07 15:00 +0100
Last post2016-01-07 15:30 +0100
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v2 03/13] arm64: use more granular reservations for  static page table allocations Mark Rutland <mark.rutland@arm.com> - 2016-01-07 15:00 +0100
    Re: [PATCH v2 03/13] arm64: use more granular reservations for static  page table allocations Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2016-01-07 15:10 +0100
      Re: [PATCH v2 03/13] arm64: use more granular reservations for  static page table allocations Mark Rutland <mark.rutland@arm.com> - 2016-01-07 15:30 +0100

#1303599 — Re: [PATCH v2 03/13] arm64: use more granular reservations for static page table allocations

FromMark Rutland <mark.rutland@arm.com>
Date2016-01-07 15:00 +0100
SubjectRe: [PATCH v2 03/13] arm64: use more granular reservations for static page table allocations
Message-ID<qOjgD-om-27@gated-at.bofh.it>
On Wed, Dec 30, 2015 at 04:26:02PM +0100, Ard Biesheuvel wrote:
> Before introducing new statically allocated page tables and increasing
> their alignment in subsequent patches, update the reservation logic
> so that only pages that are in actual use end up as reserved with
> memblock.

Could you add something to the commit message about what this will gain
us (i.e. which pages we don't have to reserve)? It's not immediately
obvious why we'd have page tables we wouldn't want to reserve.

From the looks of the next patch we won't have redundant levels of
fixmap table for a given configuration, so I guess we're catering for
the case the fixmap shares a pgd/pud/pmd entry with the image mapping?

Does that happen? If so that would invalidate the assumption I make when
copying the fixmap over in [1] (see map_kernel).

To handle that either we need some special logic to copy over the
relevant bits for the fixmap (as with kasan_copy_shadow), or we need to
avoid sharing a pgd entry.

Thoughts?

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-January/397114.html

> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  arch/arm64/mm/init.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 6bacba847923..8e678d05ad84 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -36,6 +36,7 @@
>  #include <linux/swiotlb.h>
>  
>  #include <asm/fixmap.h>
> +#include <asm/kernel-pgtable.h>
>  #include <asm/memory.h>
>  #include <asm/sections.h>
>  #include <asm/setup.h>
> @@ -165,11 +166,13 @@ void __init arm64_memblock_init(void)
>  	 * Register the kernel text, kernel data, initrd, and initial
>  	 * pagetables with memblock.
>  	 */
> -	memblock_reserve(__pa(_text), _end - _text);
> +	memblock_reserve(__pa(_text), __bss_stop - _text);
>  #ifdef CONFIG_BLK_DEV_INITRD
>  	if (initrd_start)
>  		memblock_reserve(__virt_to_phys(initrd_start), initrd_end - initrd_start);
>  #endif
> +	memblock_reserve(__pa(idmap_pg_dir), IDMAP_DIR_SIZE);
> +	memblock_reserve(__pa(swapper_pg_dir), SWAPPER_DIR_SIZE);
>  
>  	early_init_fdt_scan_reserved_mem();
>  
> -- 
> 2.5.0
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1303602 — Re: [PATCH v2 03/13] arm64: use more granular reservations for static page table allocations

FromArd Biesheuvel <ard.biesheuvel@linaro.org>
Date2016-01-07 15:10 +0100
SubjectRe: [PATCH v2 03/13] arm64: use more granular reservations for static page table allocations
Message-ID<qOjqj-HF-11@gated-at.bofh.it>
In reply to#1303599
On 7 January 2016 at 14:55, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Dec 30, 2015 at 04:26:02PM +0100, Ard Biesheuvel wrote:
>> Before introducing new statically allocated page tables and increasing
>> their alignment in subsequent patches, update the reservation logic
>> so that only pages that are in actual use end up as reserved with
>> memblock.
>
> Could you add something to the commit message about what this will gain
> us (i.e. which pages we don't have to reserve)? It's not immediately
> obvious why we'd have page tables we wouldn't want to reserve.
>

OK. In the original series, I also aligned the pgdir section to a log2
upper bound of its size, but that is not necessary anymore with your
changes. So the original goal was to avoid reserving the alignment
padding as well as the pgdirs that end up unused

> From the looks of the next patch we won't have redundant levels of
> fixmap table for a given configuration, so I guess we're catering for
> the case the fixmap shares a pgd/pud/pmd entry with the image mapping?
>
> Does that happen? If so that would invalidate the assumption I make when
> copying the fixmap over in [1] (see map_kernel).
>

It is a lot less likely to happen now that I moved the kernel to the
start of the vmalloc area rather than right below PAGE_OFFSET. But in
general, it seems sensible to only populate entries after confirming
that they are in fact vacant.

> To handle that either we need some special logic to copy over the
> relevant bits for the fixmap (as with kasan_copy_shadow), or we need to
> avoid sharing a pgd entry.
>
> Thoughts?
>

Yes, I have added that to my v3 version of the vmalloc base move patch here

https://git.linaro.org/people/ard.biesheuvel/linux-arm.git/commitdiff/0beef2c1a6bfc90cc116a6ba1b24f2ba35e7e5f6

but I think 16k/4 levels is the only config affected when the kernel
is always in the lower half of the vmalloc area. That also implies
that the fixmap pgd is either always shared, or never, depending on
the build time config, so I could probably simplify that part
somewhat.

-- 
Ard.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1303610

FromMark Rutland <mark.rutland@arm.com>
Date2016-01-07 15:30 +0100
Message-ID<qOjJE-OF-3@gated-at.bofh.it>
In reply to#1303602
On Thu, Jan 07, 2016 at 03:02:00PM +0100, Ard Biesheuvel wrote:
> On 7 January 2016 at 14:55, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Wed, Dec 30, 2015 at 04:26:02PM +0100, Ard Biesheuvel wrote:
> >> Before introducing new statically allocated page tables and increasing
> >> their alignment in subsequent patches, update the reservation logic
> >> so that only pages that are in actual use end up as reserved with
> >> memblock.
> >
> > Could you add something to the commit message about what this will gain
> > us (i.e. which pages we don't have to reserve)? It's not immediately
> > obvious why we'd have page tables we wouldn't want to reserve.
> >
> 
> OK. In the original series, I also aligned the pgdir section to a log2
> upper bound of its size, but that is not necessary anymore with your
> changes. So the original goal was to avoid reserving the alignment
> padding as well as the pgdirs that end up unused

Ah, I see.

> > From the looks of the next patch we won't have redundant levels of
> > fixmap table for a given configuration, so I guess we're catering for
> > the case the fixmap shares a pgd/pud/pmd entry with the image mapping?
> >
> > Does that happen? If so that would invalidate the assumption I make when
> > copying the fixmap over in [1] (see map_kernel).
> >
> 
> It is a lot less likely to happen now that I moved the kernel to the
> start of the vmalloc area rather than right below PAGE_OFFSET. But in
> general, it seems sensible to only populate entries after confirming
> that they are in fact vacant.

Sure.

> > To handle that either we need some special logic to copy over the
> > relevant bits for the fixmap (as with kasan_copy_shadow), or we need to
> > avoid sharing a pgd entry.
> >
> > Thoughts?
> >
> 
> Yes, I have added that to my v3 version of the vmalloc base move patch here
> 
> https://git.linaro.org/people/ard.biesheuvel/linux-arm.git/commitdiff/0beef2c1a6bfc90cc116a6ba1b24f2ba35e7e5f6

Ah, great!

> but I think 16k/4 levels is the only config affected when the kernel
> is always in the lower half of the vmalloc area. That also implies
> that the fixmap pgd is either always shared, or never, depending on
> the build time config, so I could probably simplify that part
> somewhat.

Ok.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web