Skip to content

init: Make entry init-function less and introduce service objects #90014

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+154 −42
Open

init: Make entry init-function less and introduce service objects #90014

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b1ab527
init: Make entry init-function less and introduce service objects
tbursztyka May 14, 2025
0c2aa72
scripts: Removing init function pointer from check_init_priorities.py
tbursztyka May 20, 2025
b06e83b
tests: misc: Fixing expected output on check_init_priorities test
tbursztyka May 20, 2025
f0e862f
cmake: Add service section to commo-rom linker generator
tbursztyka May 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmake/linker_script/common/common-rom.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ zephyr_linker_section_obj_level(SECTION init LEVEL APPLICATION)
zephyr_linker_section_obj_level(SECTION init LEVEL SMP)

zephyr_iterable_section(NAME device NUMERIC KVMA RAM_REGION GROUP RODATA_REGION SUBALIGN ${CONFIG_LINKER_ITERABLE_SUBALIGN})
zephyr_iterable_section(NAME service NUMERIC KVMA RAM_REGION GROUP RODATA_REGION SUBALIGN ${CONFIG_LINKER_ITERABLE_SUBALIGN})

if(CONFIG_GEN_SW_ISR_TABLE AND NOT CONFIG_DYNAMIC_INTERRUPTS)
# ld align has been changed to subalign to provide identical behavior scatter vs. ld.
Expand Down
1 change: 0 additions & 1 deletion include/zephyr/device.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1241,7 +1241,6 @@ device_get_dt_nodelabels(const struct device *dev)
static const Z_DECL_ALIGN(struct init_entry) __used __noasan Z_INIT_ENTRY_SECTION( \
level, prio, Z_DEVICE_INIT_SUB_PRIO(node_id)) \
Z_INIT_ENTRY_NAME(DEVICE_NAME_GET(dev_id)) = { \
.init_fn = NULL, \
.dev = (const struct device *)&DEVICE_NAME_GET(dev_id), \
}

Expand Down
22 changes: 13 additions & 9 deletions include/zephyr/init.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
#include <zephyr/sys/util.h>
#include <zephyr/toolchain.h>

#include <zephyr/service.h>

#ifdef __cplusplus
extern "C" {
#endif
Expand Down Expand Up @@ -65,15 +67,14 @@ struct device;
*/
struct init_entry {
/**
* If the init function belongs to a SYS_INIT, this field stored the
* initialization function, otherwise it is set to NULL.
*/
int (*init_fn)(void);
/**
* If the init entry belongs to a device, this fields stores a
* reference to it, otherwise it is set to NULL.
* An init entry can be about a device or a service, _init_object
* will be used to differentiate depending on relative sections.
*/
const struct device *dev;
union {
const void *_init_object;
const struct device *dev;
const struct service *srv;
};
};

/** @cond INTERNAL_HIDDEN */
Expand Down Expand Up @@ -164,9 +165,12 @@ struct init_entry {
* @see SYS_INIT()
*/
#define SYS_INIT_NAMED(name, init_fn_, level, prio) \
Z_SERVICE_DEFINE(name, init_fn_, level, prio); \
static const Z_DECL_ALIGN(struct init_entry) \
Z_INIT_ENTRY_SECTION(level, prio, 0) __used __noasan \
Z_INIT_ENTRY_NAME(name) = {.init_fn = (init_fn_), .dev = NULL} \
Z_INIT_ENTRY_NAME(name) = { \
.srv = (const struct service *)&Z_SERVICE_NAME_GET(name) \
}

/** @} */

Expand Down
2 changes: 2 additions & 0 deletions include/zephyr/linker/common-rom/common-rom-kernel-devices.ld
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@

ITERABLE_SECTION_ROM_NUMERIC(device, Z_LINK_ITERABLE_SUBALIGN)

ITERABLE_SECTION_ROM_NUMERIC(service, Z_LINK_ITERABLE_SUBALIGN)

#if defined(CONFIG_GEN_SW_ISR_TABLE) && defined(CONFIG_SHARED_INTERRUPTS)
/* since z_shared_isr() is not referenced anywhere when
* zephyr_pre0.elf is built, the linker will end up dropping it.
Expand Down
92 changes: 92 additions & 0 deletions include/zephyr/service.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/*
* Copyright (c) 2024 Tomasz Bursztyka.
*
* SPDX-License-Identifier: Apache-2.0
*/

#ifndef ZEPHYR_INCLUDE_SERVICE_H_
#define ZEPHYR_INCLUDE_SERVICE_H_

#include <zephyr/sys/iterable_sections.h>

#ifdef __cplusplus
extern "C" {
#endif

/**
* @defgroup service System service
* @ingroup os_services
*
* A system service is a one-shot initialized software component, instantiated
* via SYS_INIT() or SYS_INIT_NAMED(). It does not provide any public API.
*
* @{
*/

/**
* @brief Structure to store service instance
*
* This will be defined through SYS_INIT/SYS_INIT_NAMED
*/
struct service {
/**
* Initialization function
*/
int (*init)(void);
};

/** @cond INTERNAL_HIDDEN */

/**
* @brief Expands to the name of a global service object
*
* @param name Service name.
*
* @return The full name of the service object defined by the service
* definition macro.
*/
#define Z_SERVICE_NAME_GET(name) _CONCAT(__service_, name)

/**
* @brief Initializer for @ref service
*
* @param init_fn Initialization function (mandatory).
*/
#define Z_SERVICE_INIT(init_fn) \
{ \
.init = (init_fn) \
}

/**
* @brief Service section name (used for sorting purposes).
*
* @param level Initialization level.
* @param prio Initialization priority.
*/
#define Z_SERVICE_SECTION_NAME(level, prio) \
_CONCAT(INIT_LEVEL_ORD(level), _##prio)

/**
* @brief Define a @ref service
*
* @param name Service name.
* @param init_fn Initialization function.
* @param level Initialization level.
* @param prio Initialization priority.
*/
#define Z_SERVICE_DEFINE(name, init_fn, level, prio) \
static const \
STRUCT_SECTION_ITERABLE_NAMED_ALTERNATE( \
service, service, \
Z_SERVICE_SECTION_NAME(level, prio), \
Z_SERVICE_NAME_GET(name)) = Z_SERVICE_INIT(init_fn)

/** @endcond */

/** @} */

#ifdef __cplusplus
}
#endif

#endif /* ZEPHYR_INCLUDE_SERVICE_H_ */
26 changes: 22 additions & 4 deletions kernel/init.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@ enum init_level {
extern const struct init_entry __init_SMP_start[];
#endif /* CONFIG_SMP */

TYPE_SECTION_START_EXTERN(struct service, service);
TYPE_SECTION_END_EXTERN(struct service, service);

/*
* storage space for the interrupt stack
*
Expand Down Expand Up @@ -337,6 +340,12 @@ static int do_device_init(const struct device *dev)
return rc;
}

static inline bool is_entry_about_service(const void *obj)
{
return (obj >= (void *)_service_list_start &&
obj < (void *)_service_list_end);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This per-item test seems needless. You're sorting them in the linker already as a "sub-section" of an existing contiguous level. No need to check every item, just walk the prefix that is "devices" and the suffix which is "services", no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am afraid I don't see what you are seeing. Service and device presence in init entries is shuffled, so one after another an init entry might point to a service or a device. What am I missing?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be possible to do what Andy is suggesting, but I think it would require initializing devices either before or after services. Given that some services likely do depend on devices being initialized and some do not depend on devices being initialized, it might also require multiple passes through the service area to look only for certain priorities before, and then only certain priorities after.

The way it's done in this PR is probably the lesser evil of the two but it does add a few extra instructions to be executed before every init routine. So there is maybe a small price to pay here in terms of speed and the trade-off is to gain some amount of space back.

So @tbursztyka - this PR is kind of a space-optimization, unless I'm misunderstanding.

If you quantize the space savings, it is is the size of 1 void * per device, is that right?

So on 32-bit system with 16 devices, we would be 64 bytes in .rodata and gaining maybe 32 bytes in .text?

Of course the growth in .text is O(1) whereas the savings in .rodata is O(n), in terms of number of devices, so in the end, the space savings should be worth it.

Is the space savings worth it at the expense of additional time needed for init?

Maybe I'm looking into it too much 😅

What do you think @peter-mitsis ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the space savings worth it at the expense of additional time needed for init?

.text size depends on the architecture, so on x86 it's a mere 12bytes added, on arm cortex_m0 it's 20bytes.

I don't think timing is going to be badly impacted by such check. (it's on if statement added, and a new indirection for services). We are probably talking about µs of difference in overall.

It's a small gain, one could argue it's negligible but there are numerous place where rom/ram size is loosely taken into account (it always surprises me that ppl developing for embedded systems are sometimes not aligning structures for instance, nor using bits instead of bytes whenever it makes sense to pack things up etc...). So such places here + there + also there etc... we end up consuming quite much data that could be useful for something else. Take #90009 for instance and add it to this current one. Continuing your example it's already 128 bytes gained on devices only. Little by little, it starts to make a difference.

Now from a design point of view, I believe my proposal makes more sense as well. init_entry is only about sorting out objects, which objects then provide the means to be initialized etc...
On further changes, in the future, we could even generalize the init operation, thus removing altogether the if statement I added to discriminate object type. (that would mean however that service init fn would take a const void * parameter as well etc...)

}

/**
* @brief Execute all the init entry initialization functions at a given level
*
Expand Down Expand Up @@ -365,17 +374,26 @@ static void z_sys_init_run_level(enum init_level level)
const struct init_entry *entry;

for (entry = levels[level]; entry < levels[level+1]; entry++) {
const struct device *dev = entry->dev;
int result = 0;

if (unlikely(entry->_init_object == NULL)) {
continue;
}

sys_trace_sys_init_enter(entry, level);
if (dev != NULL) {

if (is_entry_about_service(entry->_init_object)) {
const struct service *srv = entry->srv;

result = srv->init();
} else {
const struct device *dev = entry->dev;

if ((dev->flags & DEVICE_FLAG_INIT_DEFERRED) == 0U) {
result = do_device_init(dev);
}
} else {
result = entry->init_fn();
}

sys_trace_sys_init_exit(entry, level, result);
}
}
Expand Down
21 changes: 10 additions & 11 deletions scripts/build/check_init_priorities.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,15 +202,14 @@ def _process_initlevels(self):
raise ValueError(f"no symbol at addr {addr:08x}")
obj, size, shidx = self._objects[addr]

arg0_name = self._object_name(self._initlevel_pointer(addr, 0, shidx))
arg1_name = self._object_name(self._initlevel_pointer(addr, 1, shidx))
arg_name = self._object_name(self._initlevel_pointer(addr, 0, shidx))

self.initlevels[level].append(f"{obj}: {arg0_name}({arg1_name})")
self.initlevels[level].append(f"{obj}: {arg_name}")

ordinal = self._device_ord_from_name(arg1_name)
ordinal = self._device_ord_from_name(arg_name)
if ordinal:
prio = Priority(level, priority)
self.devices[ordinal] = (prio, arg0_name)
self.devices[ordinal] = prio

addr += size
priority += 1
Expand Down Expand Up @@ -256,8 +255,8 @@ def _check_dep(self, dev_ord, dep_ord):
self.log.info(f"Ignoring priority: {dev_node._binding.compatible}")
return

dev_prio, dev_init = self._obj.devices.get(dev_ord, (None, None))
dep_prio, dep_init = self._obj.devices.get(dep_ord, (None, None))
dev_prio = self._obj.devices.get(dev_ord, None)
dep_prio = self._obj.devices.get(dep_ord, None)

if not dev_prio or not dep_prio:
return
Expand All @@ -272,12 +271,12 @@ def _check_dep(self, dev_ord, dep_ord):
"the devicetree dependencies.")
self.errors += 1
self.log.error(
f"{dev_node.path} <{dev_init}> is initialized before its dependency "
f"{dep_node.path} <{dep_init}> ({dev_prio} < {dep_prio})")
f"{dev_node.path} is initialized before its dependency "
f"{dep_node.path} ({dev_prio} < {dep_prio})")
else:
self.log.info(
f"{dev_node.path} <{dev_init}> {dev_prio} > "
f"{dep_node.path} <{dep_init}> {dep_prio}")
f"{dev_node.path} {dev_prio} > "
f"{dep_node.path} {dep_prio}")

def check_edt(self):
"""Scan through all known devices and validate the init priorities."""
Expand Down
21 changes: 9 additions & 12 deletions scripts/build/check_init_priorities_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,12 +215,8 @@ def test_process_initlevels(self, mock_zilinit, mock_ip, mock_on):

def mock_obj_name(*args):
if args[0] == (0, 0, 0):
return "i0"
elif args[0] == (0, 1, 0):
return "__device_dts_ord_11"
elif args[0] == (4, 0, 0):
return "i1"
elif args[0] == (4, 1, 0):
return "__device_dts_ord_22"
return f"name_{args[0][0]}_{args[0][1]}"
mock_on.side_effect = mock_obj_name
Expand All @@ -230,14 +226,15 @@ def mock_obj_name(*args):
self.assertDictEqual(obj.initlevels, {
"EARLY": [],
"PRE_KERNEL_1": [],
"PRE_KERNEL_2": ["a: i0(__device_dts_ord_11)", "b: i1(__device_dts_ord_22)"],
"POST_KERNEL": ["c: name_8_0(name_8_1)"],
"PRE_KERNEL_2": ["a: __device_dts_ord_11", "b: __device_dts_ord_22"],
"POST_KERNEL": ["c: name_8_0"],
"APPLICATION": [],
"SMP": [],
})

self.assertDictEqual(obj.devices, {
11: (check_init_priorities.Priority("PRE_KERNEL_2", 0), "i0"),
22: (check_init_priorities.Priority("PRE_KERNEL_2", 1), "i1"),
11: check_init_priorities.Priority("PRE_KERNEL_2", 0),
22: check_init_priorities.Priority("PRE_KERNEL_2", 1),
})

class testValidator(unittest.TestCase):
Expand Down Expand Up @@ -303,14 +300,14 @@ def test_check(self, mock_vinit):
validator._ord2node[2]._binding = None
validator._ord2node[2].path = "/2"

validator._obj.devices = {1: (10, "i1"), 2: (20, "i2")}
validator._obj.devices = {1: 10, 2: 20}

validator._check_dep(2, 1)
validator._check_dep(1, 2)

validator.log.info.assert_called_once_with("/2 <i2> 20 > /1 <i1> 10")
validator.log.info.assert_called_once_with("/2 20 > /1 10")
validator.log.error.assert_has_calls([
mock.call("/1 <i1> is initialized before its dependency /2 <i2> (10 < 20)")
mock.call("/1 is initialized before its dependency /2 (10 < 20)")
])
self.assertEqual(validator.errors, 1)

Expand All @@ -327,7 +324,7 @@ def test_check_same_prio_assert(self, mock_vinit):
validator._ord2node[2]._binding = None
validator._ord2node[2].path = "/2"

validator._obj.devices = {1: (10, "i1"), 2: (10, "i2")}
validator._obj.devices = {1: 10, 2: 10,}

with self.assertRaises(ValueError):
validator._check_dep(1, 2)
Expand Down
2 changes: 1 addition & 1 deletion subsys/net/lib/sockets/sockets_service.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ static K_CONDVAR_DEFINE(wait_start);
STRUCT_SECTION_START_EXTERN(net_socket_service_desc);
STRUCT_SECTION_END_EXTERN(net_socket_service_desc);

static struct service {
static struct service_context {
struct zsock_pollfd events[CONFIG_ZVFS_POLL_MAX];
int count;
} ctx;
Expand Down
8 changes: 4 additions & 4 deletions tests/misc/check_init_priorities/validate_check_init_priorities_output.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@

REFERENCE_OUTPUT = [
"ERROR: Device initialization priority validation failed, the sequence of initialization calls does not match the devicetree dependencies.",
"ERROR: /i2c@11112222/test-i2c-dev@10 <NULL> is initialized before its dependency /gpio@ffff <NULL> (PRE_KERNEL_1+0 < PRE_KERNEL_1+1)",
"ERROR: /i2c@11112222/test-i2c-dev@10 <NULL> is initialized before its dependency /i2c@11112222 <NULL> (PRE_KERNEL_1+0 < PRE_KERNEL_1+2)",
"INFO: /i2c@11112222/test-i2c-dev@11 <NULL> PRE_KERNEL_1+3 > /gpio@ffff <NULL> PRE_KERNEL_1+1",
"INFO: /i2c@11112222/test-i2c-dev@11 <NULL> PRE_KERNEL_1+3 > /i2c@11112222 <NULL> PRE_KERNEL_1+2",
"ERROR: /i2c@11112222/test-i2c-dev@10 is initialized before its dependency /gpio@ffff (PRE_KERNEL_1+0 < PRE_KERNEL_1+1)",
"ERROR: /i2c@11112222/test-i2c-dev@10 is initialized before its dependency /i2c@11112222 (PRE_KERNEL_1+0 < PRE_KERNEL_1+2)",
"INFO: /i2c@11112222/test-i2c-dev@11 PRE_KERNEL_1+3 > /gpio@ffff PRE_KERNEL_1+1",
"INFO: /i2c@11112222/test-i2c-dev@11 PRE_KERNEL_1+3 > /i2c@11112222 PRE_KERNEL_1+2",
]

if len(sys.argv) != 2:
Expand Down
Loading