Skip to content

gaps between app shared memory partitions can waste a lot of space #14121

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
andrewboie opened this issue Mar 6, 2019 · 3 comments · Fixed by #14243
Closed

gaps between app shared memory partitions can waste a lot of space #14121

andrewboie opened this issue Mar 6, 2019 · 3 comments · Fixed by #14243
Assignees
Labels
area: Memory Protection bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug

Comments

@andrewboie
Copy link
Contributor

andrewboie commented Mar 6, 2019

Describe the bug

On systems which require power-of-two size/alignment, we are not ordering the app shared memory partitions in an optimal way, which results in a lot of memory being wasted.

A good example is running the app memory size script from #14120 on the kernel produced by tests/subsys/jwt on mps2_an385 after applying #14115. Each partition is either very small or has good efficiency, but notice the huge gap in between z_libc_partition and k_mbedtls_partition:

$ python3 ~/projects/zephyr4/scripts/appmem_report.py zephyr/zephyr.elf 
Userspace memory partition report:
Partition                     Total       Used        Wasted      Efficiency
----------------------------------------------------------------------------
ztest_mem_partition           0x40        0x34        0xc         81.25%
  gap 0x0         
z_libc_partition              0x20        0x4         0x1c        12.50%
  gap 0x7fa0      
k_mbedtls_partition           0x8000      0x7fe0      0x20        99.90%

total                         0x10000     0x8018      0x7fe8      50.04%

Ordering the app shared memory partitions in reverse order of total size should greatly reduce these gaps.

We don't know the full sizes of these partitions when gen_app_partitions.py runs since the zephyr_prebuilt.elf has not been linked yet, but since we are examining the individual object files we might be able to assemble a total anyway. Or we could somehow figure out a way to get the linker to reorganize these partitions for us, but that might require them being placed in individual sections.

@andrewboie andrewboie added bug The issue is a bug, or the PR is fixing a bug area: Memory Protection priority: medium Medium impact/importance bug labels Mar 6, 2019
@andrewboie
Copy link
Contributor Author

If we try to solve this in gen_app_partitions.py, any solution for this will need to consider the memory used not only in .o files scanned, but libraries added via the --library option to gen_app_partitions.py

@galak
Copy link
Collaborator

galak commented Mar 7, 2019

The linker has SORT_BY_ALIGNMENT which might allow us to sort the partitions at link time. Just requires that each partition ends up in its own section. Not sure what the implications or requirements in the linker script are related to that.

I know we need to keep the partitions together, that should be easy enough. @ioannisg had some additional constraints we needed to ensure.

Maybe first step is just to get each partition in its own linker section and ensuring the proper constraints.

@dcpleung
Copy link
Member

dcpleung commented Mar 9, 2019

SORT_BY_ALIGNMENT can only be applied to input sections. With app shared memory, the sorting needs to be done to across different output sections. The issue is due to ARM's MPU alignment requirements, where a 64KB region must be aligned on 64KB. So k_mbedtls_partition in the description has to be aligned at 32KB since its partition is 32KB. Hence there is a big gap between z_libc_partition and k_mbedtls_partition because of all the padding.

dcpleung added a commit to dcpleung/zephyr that referenced this issue Mar 11, 2019
If CONFIG_MPU_REQUIRES_POWER_OF_TWO_ALIGNMENT is enabled,
the app shared memory partition may cause waste of memory
due to the need for padding.

For example, tests/subsys/jwt and board mps2_an385:

  z_test_mem_partition: addr 0x20000000, size 52
  z_libc_partition    : addr 0x20000040, size 4
  k_mbedtls_partition : addr 0x20008000, size 32736

    ending at 0x2000ffff, taking up 65536 bytes

With power-of-two size and alignment requirement,
k_mbedtls_partition takes up 32KB memory and needs to be
aligned on 32KB boundary. If the above partitions are
ordered as shown, there needs to be a lot of padding
after z_libc_partition before k_mbedtls_partition can
start. In order to minimize padding, these partitions
need to be sort by size in descending order.

After the changes here,	the partitions are:

  k_mbedtls_partition : addr 0x20000000, size 32736
  z_test_mem_partition: addr 0x20008000, size 52
  z_libc_partition    : addr 0x20008040, size 4

    ending at 0x2000805f, taking up 32864 bytes

With the above example, sorting results in a saving
of 32672 bytes of saving.

Fixes zephyrproject-rtos#14121

Signed-off-by: Daniel Leung <[email protected]>
nashif pushed a commit that referenced this issue Mar 13, 2019
If CONFIG_MPU_REQUIRES_POWER_OF_TWO_ALIGNMENT is enabled,
the app shared memory partition may cause waste of memory
due to the need for padding.

For example, tests/subsys/jwt and board mps2_an385:

  z_test_mem_partition: addr 0x20000000, size 52
  z_libc_partition    : addr 0x20000040, size 4
  k_mbedtls_partition : addr 0x20008000, size 32736

    ending at 0x2000ffff, taking up 65536 bytes

With power-of-two size and alignment requirement,
k_mbedtls_partition takes up 32KB memory and needs to be
aligned on 32KB boundary. If the above partitions are
ordered as shown, there needs to be a lot of padding
after z_libc_partition before k_mbedtls_partition can
start. In order to minimize padding, these partitions
need to be sort by size in descending order.

After the changes here,	the partitions are:

  k_mbedtls_partition : addr 0x20000000, size 32736
  z_test_mem_partition: addr 0x20008000, size 52
  z_libc_partition    : addr 0x20008040, size 4

    ending at 0x2000805f, taking up 32864 bytes

With the above example, sorting results in a saving
of 32672 bytes of saving.

Fixes #14121

Signed-off-by: Daniel Leung <[email protected]>
marc-hb added a commit to marc-hb/zephyr that referenced this issue Jun 27, 2019
…erministic"

This reverts commit 725abdf which did get rid of randomness in the
order of the partition _names_ as claimed but regressed commit
212ec9a / feature zephyrproject-rtos#14121 and broke the previous size order which I
missed. Huge thanks to Sigvart Hovland for spotting this in a post-merge
but prompt code review. Proper fix in the next commit.

Signed-off-by: Marc Herbert <[email protected]>
marc-hb added a commit to marc-hb/zephyr that referenced this issue Jun 27, 2019
Commit 212ec9a / feature zephyrproject-rtos#14121 already ordered partitions by
decreasing size, however it was common in samples/userspace/shared_mem/
/sample.kernel.memory_protection.shared_mem for two partitions to have
the same size and be randomly ordered between them. This adds the
partition name as a second sort key.

Unlike previous attempt in commit 725abdf this doesn't use the
partition name as the first (and only) key and doesn't break the
decreasing size order.  Huge thanks to Sigvart Hovland for spotting this
in a post-merge but prompt code review.

Signed-off-by: Marc Herbert <[email protected]>
andrewboie pushed a commit that referenced this issue Jun 28, 2019
…erministic"

This reverts commit 725abdf which did get rid of randomness in the
order of the partition _names_ as claimed but regressed commit
212ec9a / feature #14121 and broke the previous size order which I
missed. Huge thanks to Sigvart Hovland for spotting this in a post-merge
but prompt code review. Proper fix in the next commit.

Signed-off-by: Marc Herbert <[email protected]>
andrewboie pushed a commit that referenced this issue Jun 28, 2019
Commit 212ec9a / feature #14121 already ordered partitions by
decreasing size, however it was common in samples/userspace/shared_mem/
/sample.kernel.memory_protection.shared_mem for two partitions to have
the same size and be randomly ordered between them. This adds the
partition name as a second sort key.

Unlike previous attempt in commit 725abdf this doesn't use the
partition name as the first (and only) key and doesn't break the
decreasing size order.  Huge thanks to Sigvart Hovland for spotting this
in a post-merge but prompt code review.

Signed-off-by: Marc Herbert <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Memory Protection bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants