diff mbox series

[v2] arch/arc: Accommodate 16 KiB MMU pages

Message ID 20191218155555.45617-1-abrodkin@synopsys.com
State Accepted
Headers show
Series [v2] arch/arc: Accommodate 16 KiB MMU pages | expand

Commit Message

Alexey Brodkin Dec. 18, 2019, 3:55 p.m. UTC
ARC processors are known for its configurability and one of those
configurable things is MMU page size which might be set to any
power of two from 4 KiB to 16 MiB, though in the Linux kernel we
only support 4, 8 and 16 KiB due to practical considerations.

And the most used setting is 8 KiB thus GNU LD assumes maximum
page size is 8 KiB by default and while this works for smaller
pages (it's OK to align segments by larger value it will be still
peoperly aligned) this breaks execution of user-space apps on HW
with larger pages because Elf sections might very well span across
allocated pages and thus make executable broken.

Simplest example:
------------------------------------>8-----------------------------------
$ arc-linux-gcc test.c
$ arc-linux-readelf --segments a.out

Program Headers:
  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
...
  LOAD           0x000000 0x00010000 0x00010000 0x003e8 0x003e8 R E 0x2000 <-- See
  LOAD           0x001f24 0x00013f24 0x00013f24 0x000f0 0x0010c RW  0x2000
------------------------------------>8-----------------------------------

Fortunately we may override default page size settings with "max-page-size"
linker option this way:
------------------------------------>8-----------------------------------
$ arc-linux-gcc test.c -Wl,-z,max-page-size=16384
$ arc-linux-readelf --segments a.out
Elf file type is EXEC (Executable file)
Entry point 0x102c4
There are 8 program headers, starting at offset 52

Program Headers:
  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
...
  LOAD           0x000000 0x00010000 0x00010000 0x003e8 0x003e8 R E 0x4000 <-- See
  LOAD           0x001f24 0x00015f24 0x00015f24 0x000f0 0x0010c RW  0x4000
------------------------------------>8-----------------------------------

Which we implement with that change.

Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
---

Changes v1 -> v2:

 * Obviously we want to append lenker settings to the wrapper
   but not overwrite atomic option if is set as these are orthogonal
   things.

 arch/arch.mk.arc | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Yann E. MORIN Dec. 22, 2019, 9:06 p.m. UTC | #1
Alexey, All,

On 2019-12-18 18:55 +0300, Alexey Brodkin spake thusly:
> ARC processors are known for its configurability and one of those
> configurable things is MMU page size which might be set to any
> power of two from 4 KiB to 16 MiB, though in the Linux kernel we
> only support 4, 8 and 16 KiB due to practical considerations.
> 
> And the most used setting is 8 KiB thus GNU LD assumes maximum
> page size is 8 KiB by default and while this works for smaller
> pages (it's OK to align segments by larger value it will be still
> peoperly aligned) this breaks execution of user-space apps on HW
> with larger pages because Elf sections might very well span across
> allocated pages and thus make executable broken.

Applied to master, thanks.

As you explained nicely in the commit log, when the CPU uses 4KiB pages,
the default 8KiB alignment is OK. However, is there a performance penalty
to it? Does it impose a limitation in how much memory one may use, per
process or system-wide?

In short, my question is: even though it works for 4KiB, would it not be
better to also override the default, like is done for 16KiB pages?

And to push thing even further, should we not also set it for 8KiB, in
case the default changes in a later binutils version?

If you believe those are valid, please send a follow up patch. Otherwise
just ignore my ramblings. ;-)

Regards,
Yann E. MORIN.

> Simplest example:
> ------------------------------------>8-----------------------------------
> $ arc-linux-gcc test.c
> $ arc-linux-readelf --segments a.out
> 
> Program Headers:
>   Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
> ...
>   LOAD           0x000000 0x00010000 0x00010000 0x003e8 0x003e8 R E 0x2000 <-- See
>   LOAD           0x001f24 0x00013f24 0x00013f24 0x000f0 0x0010c RW  0x2000
> ------------------------------------>8-----------------------------------
> 
> Fortunately we may override default page size settings with "max-page-size"
> linker option this way:
> ------------------------------------>8-----------------------------------
> $ arc-linux-gcc test.c -Wl,-z,max-page-size=16384
> $ arc-linux-readelf --segments a.out
> Elf file type is EXEC (Executable file)
> Entry point 0x102c4
> There are 8 program headers, starting at offset 52
> 
> Program Headers:
>   Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
> ...
>   LOAD           0x000000 0x00010000 0x00010000 0x003e8 0x003e8 R E 0x4000 <-- See
>   LOAD           0x001f24 0x00015f24 0x00015f24 0x000f0 0x0010c RW  0x4000
> ------------------------------------>8-----------------------------------
> 
> Which we implement with that change.
> 
> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> ---
> 
> Changes v1 -> v2:
> 
>  * Obviously we want to append lenker settings to the wrapper
>    but not overwrite atomic option if is set as these are orthogonal
>    things.
> 
>  arch/arch.mk.arc | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arch.mk.arc b/arch/arch.mk.arc
> index 5fcffb7f4b..d45299144c 100644
> --- a/arch/arch.mk.arc
> +++ b/arch/arch.mk.arc
> @@ -1,4 +1,14 @@
> +ifeq ($(BR2_arc),y)
> +
>  # -matomic is always required when the ARC core has the atomic extensions
> -ifeq ($(BR2_arc)$(BR2_ARC_ATOMIC_EXT),yy)
> +ifeq ($(BR2_ARC_ATOMIC_EXT),y)
>  ARCH_TOOLCHAIN_WRAPPER_OPTS = -matomic
>  endif
> +
> +# By default MAXPAGESIZE for ARC is 8196 so for larger MMU pages
> +# it needs to be overridden.
> +ifeq ($(BR2_ARC_PAGE_SIZE_16K),y)
> +ARCH_TOOLCHAIN_WRAPPER_OPTS += -Wl,-z,max-page-size=16384
> +endif
> +
> +endif
> -- 
> 2.16.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Alexey Brodkin Dec. 23, 2019, 7:18 a.m. UTC | #2
Hi Yann,

> -----Original Message-----
> From: Yann E. MORIN <yann.morin.1998@free.fr>
> Sent: Monday, December 23, 2019 12:06 AM
> To: Alexey Brodkin <abrodkin@synopsys.com>
> Cc: buildroot@buildroot.org; arc-buildroot@synopsys.com
> Subject: Re: [Buildroot] [PATCH v2] arch/arc: Accommodate 16 KiB MMU pages
> 
> Alexey, All,
> 
> On 2019-12-18 18:55 +0300, Alexey Brodkin spake thusly:
> > ARC processors are known for its configurability and one of those
> > configurable things is MMU page size which might be set to any
> > power of two from 4 KiB to 16 MiB, though in the Linux kernel we
> > only support 4, 8 and 16 KiB due to practical considerations.
> >
> > And the most used setting is 8 KiB thus GNU LD assumes maximum
> > page size is 8 KiB by default and while this works for smaller
> > pages (it's OK to align segments by larger value it will be still
> > peoperly aligned) this breaks execution of user-space apps on HW
> > with larger pages because Elf sections might very well span across
> > allocated pages and thus make executable broken.
> 
> Applied to master, thanks.
> 
> As you explained nicely in the commit log, when the CPU uses 4KiB pages,
> the default 8KiB alignment is OK. However, is there a performance penalty
> to it?

There's a performance penalty related to page size itself. Even though I don't
have full set of data at hand to explain all minor details but from our
experience 8KiB page size seem to be a kind of optimal value for use-cases we
worry about. With smaller pages page fault exceptions happen more often,
with larger 16KiB pages memory segmentation is higher, page zeroing takes more time.
So typically we use 8KiB and advise our users to do the same (and if I were
an external ARC user designing a new SoC I'd rather use the same default value
as this is supposed to be much more tested compared to less often used configurations).

> Does it impose a limitation in how much memory one may use, per
> process or system-wide?

Given this value (the one we change - LD's "max-page-size") only affects how
linker aligns segments in resulting Elf I don't think there might be any kind of
real limitation that gets introduced by this. Indeed we may start using process'
address space a bit less efficient (think about small segments in Elf each of which
will "occupy" 2 4KiB pages instead of just 1). But I would think it is pretty rare
situation when almost 2GiB of process' address space is used for miriads of Elfs
with tiny segments. So I don't think something should become terribly bad.
 
> In short, my question is: even though it works for 4KiB, would it not be
> better to also override the default, like is done for 16KiB pages?

Well as it quite often happens I was first solving a breakage that bothered
me for real. Obviously w/o discussed change user-space apps on 16KiB MMU pages
just didn’t work and so I fixed that, thus no mention of special handling of
4KiB or 8KiB pages - there stuff just worked nicely. But read-on...

> And to push thing even further, should we not also set it for 8KiB, in
> case the default changes in a later binutils version?

Now looking at your comment indeed I realize there're at least a couple of reasons
to explicitly set this thing for al supported page sizes, namely:

1. Clarity. Implicitly set something is not good. Some people know what is
   implicitly set, some don't, some forget, some people change this :)

2. Future-proof. Indeed what if at some point we decide 4KiB is the new default.
   Well for some reason most of the 32-bit CPUs use that: x86, ARM etc.

3. Uniformity. It always helps even if it sometimes adds a few LoC :)

> If you believe those are valid, please send a follow up patch. Otherwise
> just ignore my ramblings. ;-)

So thanks a lot for these inspiring questions, will send a follow-up soon.

Thanks again for taking care and being curious!
-Alexey
diff mbox series

Patch

diff --git a/arch/arch.mk.arc b/arch/arch.mk.arc
index 5fcffb7f4b..d45299144c 100644
--- a/arch/arch.mk.arc
+++ b/arch/arch.mk.arc
@@ -1,4 +1,14 @@ 
+ifeq ($(BR2_arc),y)
+
 # -matomic is always required when the ARC core has the atomic extensions
-ifeq ($(BR2_arc)$(BR2_ARC_ATOMIC_EXT),yy)
+ifeq ($(BR2_ARC_ATOMIC_EXT),y)
 ARCH_TOOLCHAIN_WRAPPER_OPTS = -matomic
 endif
+
+# By default MAXPAGESIZE for ARC is 8196 so for larger MMU pages
+# it needs to be overridden.
+ifeq ($(BR2_ARC_PAGE_SIZE_16K),y)
+ARCH_TOOLCHAIN_WRAPPER_OPTS += -Wl,-z,max-page-size=16384
+endif
+
+endif