linker: do not force keep common kernel objects #28486

dcpleung · 2020-09-17T20:02:01Z

This uses the new macros so the common kernel objects can be
garbage collected if they are not referred directly anywhere
in the code. This is useful for reducing library data size
as not all library functions and their corresponding kobjects
are being used.

Fixes #20663

Signed-off-by: Daniel Leung [email protected]

andrewboie

looks good

dcpleung · 2020-09-18T06:42:49Z

The compliance complain is about linker script not acting like C code...

carlescufi

Thanks for this!

However, this still seems to leave stuff in that should not be there:

west build -b reel_board -t rom_report samples/basic/minimal/ -- -DCONF_FILE="common.conf no-mt.conf no-timers.conf arm.conf"

So it doesn't fix #20663 entirely.

You will likely need something like:
carlescufi@a0d30e2#diff-4560d3c90a6d35d90781757e85d2056dR12
and
carlescufi@a0d30e2#diff-9112b5bcdb85f9fc628e8175f32264b5R28

to stop pulling heap and mempool code when not needed.

andrewboie · 2020-09-18T19:00:12Z

Thanks for this!

However, this still seems to leave stuff in that should not be there:
west build -b reel_board -t rom_report samples/basic/minimal/ -- -DCONF_FILE="common.conf no-mt.conf no-timers.conf arm.conf"
So it doesn't fix #20663 entirely.

You will likely need something like:
carlescufi@a0d30e2#diff-4560d3c90a6d35d90781757e85d2056dR12
and
carlescufi@a0d30e2#diff-9112b5bcdb85f9fc628e8175f32264b5R28

to stop pulling heap and mempool code when not needed.

Is it the cross-dependencies between kernel/ and lib/ that are defeating the --no-whole-archive logic (as described in #20663 (comment))? We can ifdef stuff but I'm (wildly) speculating if there is an even better solution we are overlooking...

The iterable section macros Z_ITERABLE_SECTION_ROM()/_RAM() uses Z_LINK_ITERABLE() which does KEEP() on all symbols. This results in all symbols under those sections being kept in final image even though these symbols are not referred directly from the code. This adds the new set of macros which does not force KEEP() on the symbols, and allows symbols to be garbage collected. The existing macros are kept as-is so as not to change their behaviors. Signed-off-by: Daniel Leung <[email protected]>

This uses the new macros so the common kernel objects can be garbage collected if they are not referred directly anywhere in the code. This is useful for reducing library data size as not all library functions and their corresponding kobjects are being used. Fixes zephyrproject-rtos#20663 Signed-off-by: Daniel Leung <[email protected]>

Adds a kconfig CONFIG_KERNEL_MEM_POOL to decide whether kernel memory pool related code is compiled. This option can be disabled to shrink code size. If k_heap is not being used at all, kheap.c will also not be compiled, resulting in further smaller code size. Signed-off-by: Daniel Leung <[email protected]>

This adds the option to disable kernel memory pool when multithreading is also not enabled. This saves some code space. Signed-off-by: Daniel Leung <[email protected]>

dcpleung · 2020-09-18T23:47:12Z

It looks like mempool.c is causing kheap.c to be included regardless if heap is being used, even though the mempool.c is not included in the final image. I added a kconfig for memory pool and have it disabled in the minimal sample itself. The ROM size of the image from the build command above goes from 3446 -> 2926.

@carlescufi if you want, I can have CONFIG_KERNEL_MEM_POOL defaults to n if CONFIG_MULTITHREADING=n.

andrewboie · 2020-09-19T02:50:22Z

@carlescufi if you want, I can have CONFIG_KERNEL_MEM_POOL defaults to n if CONFIG_MULTITHREADING=n.

I'm not sure if the multithreading=n matters much. This configuration is deprecated and is going away soon.

It looks like mempool.c is causing kheap.c to be included regardless if heap is being used, even though the mempool.c is not included in the final image.

I think it's a general issue we need to solve, I don't think we want to go down a path where we have to have Kconfigs for core kernel APIs because of some interaction in dependencies between kernel/ and lib/os/. I don't think we had this problem before the heap code landed and we had the kernel mempool code and the os lib mempool code.

What specifically is pulling in what, with respect to lib/os/mempool.c and kernel/kheap.c?

I have a feeling that the real proper fix for this is to finish what we started in #24358 I don't understand why that issue keeps bouncing back and forth between "enhancement" and "bug" status

dcpleung · 2020-09-19T06:27:13Z

It's caused by calling of k_mem_pool_alloc() in mempool.c, where this function resides in kheap.c if CONFIG_MEM_POOL_HEAP_BACKEND=y. Even if you extract this function into another file, it still calls k_heap_alloc() and k_heap_free() which results in kheap.c being included.

Setting CONFIG_MEM_POOL_HEAP_BACKEND to n simply changes from including kheap.c to including mempool_sys.c which also has SYS_LIST().

To completely prevent either SYS_LIST() from being included, mempool.c needs to be excluded from build, where kconfig is a way to achieve this.

carlescufi

I am happy with the outcome. Thank you @dcpleung.

dcpleung requested review from andrewboie and andyross as code owners September 17, 2020 20:02

dcpleung requested a review from nashif September 17, 2020 20:02

github-actions bot added the area: API Changes to public APIs label Sep 17, 2020

andrewboie approved these changes Sep 17, 2020

View reviewed changes

nashif approved these changes Sep 18, 2020

View reviewed changes

nashif requested a review from carlescufi September 18, 2020 11:38

carlescufi requested changes Sep 18, 2020

View reviewed changes

MaureenHelm added the bug The issue is a bug, or the PR is fixing a bug label Sep 18, 2020

MaureenHelm added this to the v2.4.0 milestone Sep 18, 2020

dcpleung added 4 commits September 18, 2020 16:39

samples: basic/minimal: disable memory pool when MT=n

acac115

This adds the option to disable kernel memory pool when multithreading is also not enabled. This saves some code space. Signed-off-by: Daniel Leung <[email protected]>

dcpleung force-pushed the kobj_linker_keep branch from 28a0b00 to acac115 Compare September 18, 2020 23:39

github-actions bot added area: Kernel area: Samples Samples labels Sep 18, 2020

nashif requested a review from carlescufi September 19, 2020 02:15

carlescufi approved these changes Sep 19, 2020

View reviewed changes

nashif merged commit 4a9ee68 into zephyrproject-rtos:master Sep 19, 2020

dcpleung deleted the kobj_linker_keep branch September 19, 2020 18:09

martinjaeger mentioned this pull request Nov 16, 2020

Memory corruption for newlib-nano with float printf and disabled heap #30055

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

linker: do not force keep common kernel objects #28486

linker: do not force keep common kernel objects #28486

dcpleung commented Sep 17, 2020

andrewboie left a comment

dcpleung commented Sep 18, 2020

carlescufi left a comment •

edited

Loading

andrewboie commented Sep 18, 2020

dcpleung commented Sep 18, 2020 •

edited

Loading

andrewboie commented Sep 19, 2020

dcpleung commented Sep 19, 2020 •

edited

Loading

carlescufi left a comment

linker: do not force keep common kernel objects #28486

linker: do not force keep common kernel objects #28486

Conversation

dcpleung commented Sep 17, 2020

andrewboie left a comment

Choose a reason for hiding this comment

dcpleung commented Sep 18, 2020

carlescufi left a comment • edited Loading

Choose a reason for hiding this comment

andrewboie commented Sep 18, 2020

dcpleung commented Sep 18, 2020 • edited Loading

andrewboie commented Sep 19, 2020

dcpleung commented Sep 19, 2020 • edited Loading

carlescufi left a comment

Choose a reason for hiding this comment

carlescufi left a comment •

edited

Loading

dcpleung commented Sep 18, 2020 •

edited

Loading

dcpleung commented Sep 19, 2020 •

edited

Loading