Skip to content
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

NUM_KERNEL_OBJECT_FILES is too small #7703

Closed
andrewboie opened this issue May 21, 2018 · 5 comments · Fixed by #9280
Closed

NUM_KERNEL_OBJECT_FILES is too small #7703

andrewboie opened this issue May 21, 2018 · 5 comments · Fixed by #9280
Labels
area: ARC ARC Architecture area: ARM ARM (32-bit) Architecture area: Memory Protection bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug

Comments

@andrewboie
Copy link
Contributor

I found this when trying to build frdm_k64f/tests/drivers/build_all/test_build_sensors_a_m

In file included from /home/buildslave/src/github.com/zephyrproject-rtos/zephyr/include/arch/arm/cortex_m/scripts/linker.ld:21:0,
                 from /home/buildslave/src/github.com/zephyrproject-rtos/zephyr/arch/arm/soc/nxp_kinetis/k6x/linker.ld:31:
/home/buildslave/src/github.com/zephyrproject-rtos/zephyr/include/linker/linker-defs.h:115:2: error: #error "Max supported kernel objects is 32."
 #error "Max supported kernel objects is 32."
  ^~~~~

32 is unreasonably small limit of the number of .o files for a real-world application.

@andrewboie
Copy link
Contributor Author

Apparently this is a limitation of UTIL_LISTIFY and we really need to just get rid of this.

#ifndef NUM_KERNEL_OBJECT_FILES
#error "Expected NUM_KERNEL_OBJECT_FILES to be defined"
#elif NUM_KERNEL_OBJECT_FILES > 32
#error "Max supported kernel objects is 32."
/* TODO: Using the preprocessor to do this was a mistake. Rewrite to
   scale better. e.g. by aggregating the kernel objects into two
   archives like KBuild did.*/
#endif

andrewboie pushed a commit that referenced this issue May 21, 2018
We run into the limit of 32 object files on ARM when
CONFIG_APPLICATION_MEMORY is enabled.

Bug #7703 filed, meanwhile just disable it, it's not
needed for this test case.

Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
@lpereira
Copy link
Collaborator

I would even rename UTIL_LISTIFY to _UTIL_LISTIFY to discourage it being used by people and make it easier to get rid of it in the next version.

andrewboie pushed a commit that referenced this issue May 21, 2018
We run into the limit of 32 object files on ARM when
CONFIG_APPLICATION_MEMORY is enabled.

Bug #7703 filed, meanwhile just disable it, it's not
needed for this test case.

Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
@SebastianBoe
Copy link
Collaborator

I would even rename UTIL_LISTIFY to _UTIL_LISTIFY to discourage it being used by people and make it easier to get rid of it in the next version.

I wouldn't go that far ...

It is useful to generate sequences of code that are relatively short.

e.g. it is the cleanest possible solution (that I know of) to solve problems like the below.

#define TIMER_IRQHANDLER_GENERATOR(j, _)                                                            \
    void TIMER##j##_IRQHandler(void)                                                                \
    {                                                                                              \
        timer_i_irqhandler(j);                                                                      \
    }

#ifdef TIMER_COUNT
STD_SHARED_LIB_LISTIFY(TIMER_COUNT, TIMER_IRQHANDLER_GENERATOR)
#endif

It was just inappropriately used here.

andrewboie pushed a commit that referenced this issue May 22, 2018
We run into the limit of 32 object files on ARM when
CONFIG_APPLICATION_MEMORY is enabled.

Bug #7703 filed, meanwhile just disable it, it's not
needed for this test case.

Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
@andrewboie
Copy link
Contributor Author

IMO UTIL_LISTIFY needs to die and we should just generate the code with a post-build step. It just doesn't scale. I am not sure if #7632 (the successor to CONFIG_APPLICATION_MEMORY) needs it.

@andrewboie andrewboie added bug The issue is a bug, or the PR is fixing a bug area: ARM ARM (32-bit) Architecture area: ARC ARC Architecture labels May 22, 2018
andrewboie pushed a commit to andrewboie/zephyr that referenced this issue May 22, 2018
We run into the limit of 32 object files on ARM when
CONFIG_APPLICATION_MEMORY is enabled.

Bug zephyrproject-rtos#7703 filed, meanwhile just disable it, it's not
needed for this test case.

Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
andrewboie pushed a commit that referenced this issue May 22, 2018
We run into the limit of 32 object files on ARM when
CONFIG_APPLICATION_MEMORY is enabled.

Bug #7703 filed, meanwhile just disable it, it's not
needed for this test case.

Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
@MaureenHelm MaureenHelm added the priority: low Low impact/importance bug label May 25, 2018
@tejlmand
Copy link
Collaborator

tejlmand commented Jul 5, 2018

RFC #8541 and issue #8441 indirectly address this issue.

By significantly reducing the numbers of CMake libraries, as example bluetooth goes from up to 6 sub-libraries to just a single. Net can go from up to 12 sub-libraries to also a single.

A samples app, such as bluetooth/ipsp, will go from 15 libraries that would be in Kernel spaces and down to just 6.
Of course the actual reduction depends on how much is enabled in the app, but for subsystems such as net, the potential gain is high.

In the ipsp case the following reduction is seen:
libzephyr.a (stays)
libkernel.a (stays)

libdrivers__gpio.a
libdrivers__serial.a
libdrivers__entropy.a
=> libdrivers.a

libarch__arm__core.a
libarch__arm__core__cortex_m.a
libarch__arm__core__cortex_m__mpu.a
=> libarch_arm.a

libsubsys__bluetooth__common.a
libsubsys__bluetooth__host.a
libsubsys__bluetooth__controller.a
=> libsubsys_bluetooth.a

libsubsys__net.a
libsubsys__net__l2.a
libsubsys__net__ip.a
libsubsys__net__lib__config.a
=> libsubsys_net.a

SebastianBoe added a commit to SebastianBoe/zephyr that referenced this issue Aug 3, 2018
This rewrites the implementation of the APP_INPUT_SECTION and
KERNEL_INPUT_SECTION macros such that an unbounded amount of
kernelspace libraries can be used.

This resolves zephyrproject-rtos#7703

The new implementation has a caveat/limitation; the linker script
developer must invoke APP_INPUT_SECTION before KERNEL_INPUT_SECTION.

All in-tree linker scripts happened to already be doing this so no
in-tree porting was necessary.

Signed-off-by: Sebastian Bøe <sebastian.boe@nordicsemi.no>
nashif pushed a commit that referenced this issue Aug 17, 2018
This rewrites the implementation of the APP_INPUT_SECTION and
KERNEL_INPUT_SECTION macros such that an unbounded amount of
kernelspace libraries can be used.

This resolves #7703

The new implementation has a caveat/limitation; the linker script
developer must invoke APP_INPUT_SECTION before KERNEL_INPUT_SECTION.

All in-tree linker scripts happened to already be doing this so no
in-tree porting was necessary.

Signed-off-by: Sebastian Bøe <sebastian.boe@nordicsemi.no>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ARC ARC Architecture area: ARM ARM (32-bit) Architecture area: Memory Protection bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants