lib/ukboot: Shutdown feature #1148

skuenzer · 2023-10-26T22:01:34Z

Base target

Architecture(s): [N/A]
Platform(s): [N/A]
Application(s): [N/A]

Description of changes

This PR introduces support for shutting down the system. The change consists of an extension of inittab. Each entry is extended to two function pointers: an initialization function pointer and a termination function pointer, which are handled in pairs and share the same priority. While initialization means iterating over the inittab and calling the initialization functions, termination is done by iterating over the inittab in reverse order.
Initialization and termination are handled by the "init" thread of lib/ukboot. With scheduling, PR introduces the option to start the main() of the application in a separate "main" thread. This allows shutdown requests to be handled independently of the state of main(). A shutdown request driver (e.g., ACPI button) or any other application thread can send a request to the "init" thread to initiate a shutdown.

include/uk/init.h

mogasergiu

Hi Simon 👋! Had a quick look, very clean PR! I like the semaphore idea! The only downside of this PR I can see is the shutdown event not being handled with highest priority (i.e. it has to wait to be scheduled in the round-robin fashion) which can be risky in the case of high emergency power management events, especially in a cooperative scheduler environment (maybe something like a deferred handler could solve this 🥺)

Anyway, I left some minor comments. I will probably have another look in the near future 🤠.

lib/ukboot/boot.c

lib/uklock/include/uk/isr/semaphore.h

lib/ukboot/shutdown_req.c

skuenzer · 2023-11-27T14:21:54Z

Hi Simon 👋! Had a quick look, very clean PR! I like the semaphore idea! The only downside of this PR I can see is the shutdown event not being handled with highest priority (i.e. it has to wait to be scheduled in the round-robin fashion) which can be risky in the case of high emergency power management events, especially in a cooperative scheduler environment (maybe something like a deferred handler could solve this 🥺)

Anyway, I left some minor comments. I will probably have another look in the near future 🤠.

Hi Sergiu, thanks a lot for your review. It is in fact true that a shutdown request can be ignored as long as a thread never releases the CPU which lets the init thread not being scheduled to forward a shutdown request to the application. However, I think this behavior is fine since we are speaking here about a shutdown request which the application could even ignore. So, my assumption is that this is a cooperative action. With preempt scheduling this might be even less of a problem and the hypervisor can still terminate non-cooperating virtual machines.

mogasergiu · 2023-11-27T14:30:56Z

can be ignored as long as a thread never releases the CPU

Yes you are right. I was however referring to the situation where this is not the next thread that will be executed on yield, because we do not have a priority scheduling schema, right?

hypervisor can still terminate non-cooperating virtual machines.

Yes, agreed, not something worth bothering with now 😃. I just thought it would be worth a mention 😉.

mogasergiu

Ok had my last look. After this I should be able to give my tag. Everything still looks very good 🙌, mostly just some (perhaps subjective?) styling comments.

lib/ukboot/boot.c

include/uk/init.h

include/uk/ctors.h

lib/ukboot/boot.c

lib/ukboot/include/uk/boot.h

lib/uksched/include/uk/isr/sched.h

lib/uksched/include/uk/isr/wait.h

lib/uksched/include/uk/sched_impl.h

lib/ukboot/include/uk/isr/boot.h

lib/ukboot/shutdown_req.c

The `struct uk_mutex` structure expects the members to be, in order, `lock_count`, `flags`, `owner`, `waitq`. Change the static inititializer to set the flags to `UK_MUTEX_CONFIG_RECURSE` instead of the owner. Signed-off-by: Stefan Jumarea <stefanjumarea02@gmail.com> Reviewed-by: Maria Sfiraiala <maria.sfiraiala@gmail.com> Approved-by: Marco Schlumpp <marco@unikraft.io> GitHub-Fixes: unikraft#1195

This commit introduces a context argument (`struct uk_init_ctx *`) to the initialization functions. The motivation is that 1) the list of arguments can be easily extended without changing the init function signature and 2) we introduce a way to handover boot information across initalization functions. This commit adds the argument count (`argc`) and a reference to the argument vector (`argv`) to `struct uk_init_ctx`. `lib/ukboot` is adopted accordingly. Signed-off-by: Simon Kuenzer <simon@unikraft.io>

This commit provides an ISR-safe variant of `uk_boot_shutdown_req()`. THis function is safe to be called from interrupt contexts, like a driver interrupt. The commit tries to avoid to duplicate code and compiles `shutdown_req.c` two times: once without ISR properties and another time with ISR property. Signed-off-by: Simon Kuenzer <simon@unikraft.io>

This commit documents the shutdown request event queue within `include/uk/plat/bootstrap.h`. The idea is that a platform declare the queue and raises events with drivers. Any library can then register shutdown handlers to this queue and can take according actions. Signed-off-by: Simon Kuenzer <simon@unikraft.io>

This commit registers the shutdown request function `uk_boot_shutdown_req_isr()` to the shutdown request event queue. This allows drivers to trigger a shutdown which is then performed by the "init" thread. Signed-off-by: Simon Kuenzer <simon@unikraft.io>

This commit adopts the glue code to work with the Unikraft updates introduced with [PR #1148](unikraft/unikraft#1148). Signed-off-by: Simon Kuenzer <simon@unikraft.io>

mogasergiu

Reviewed-by: Sergiu Moga sergiu@unikraft.io

razvand

Approved-by: Razvan Deaconescu razvand@unikraft.io

This commit adopts the glue code to work with the Unikraft updates introduced with [PR #1148](unikraft/unikraft#1148). Signed-off-by: Simon Kuenzer <simon@unikraft.io>

This commit introduces a context argument (`struct uk_init_ctx *`) to the initialization functions. The motivation is that 1) the list of arguments can be easily extended without changing the init function signature and 2) we introduce a way to handover boot information across initalization functions. This commit adds the argument count (`argc`) and a reference to the argument vector (`argv`) to `struct uk_init_ctx`. `lib/ukboot` is adopted accordingly. Signed-off-by: Simon Kuenzer <simon@unikraft.io> Reviewed-by: Sergiu Moga <sergiu@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GitHub-Closes: #1148

This commit extends the inittab entries with another function pointer: a corresponding termination function. The functions are handled as pair because the idea is that a shutdown means iterating over the table in reverse order and calling the termination functions. `lib/ukboot` is adopted accordingly. Similar to the init functions, a context argument for the termination functions is introduced, too: `struct uk_term_ctx *`. In its initial version, it only contains the shutdown request (e.g., `UKPLAT_HALT`, `UKPLAT_RESTART`). Signed-off-by: Simon Kuenzer <simon@unikraft.io> Reviewed-by: Sergiu Moga <sergiu@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GitHub-Closes: #1148

This commit adopts the inittab registration macros so that termination function can be handed over, too. This happens always in pair: Inititalization function together with termination function. Signed-off-by: Simon Kuenzer <simon@unikraft.io> Reviewed-by: Sergiu Moga <sergiu@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GitHub-Closes: #1148

Most UNIX-like operating systems call the constructors listed in __init_array with the argument vector. This commit changes the constructor function type to be non-prototyped so that we can use the same type for all classes of constructors. The commit then changes the boot library to supply the argument vector to the __init_array constructors. Signed-off-by: Marc Rittinghaus <marc@unikraft.io> Reviewed-by: Sergiu Moga <sergiu@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GitHub-Closes: #1148

This commit introduces a "main" thread for calling the application's `main()` function. This is done so that "init" is purely responsible for system initialization and shutdown. After creating the "main" thread, "init" will block on a barrier which can only be unblocked by a call to `uk_boot_shutdown_req()`. "init" will then perform a system shutdown. With this model, any thread, including "main", can request a shutdown. Signed-off-by: Simon Kuenzer <simon@unikraft.io> Reviewed-by: Sergiu Moga <sergiu@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GitHub-Closes: #1148

This commit provides ISR variants of the following thread wake up functions: - `uk_waitq_wake_up()` - `uk_sched_thread_woken()` - `uk_thread_wake()` This enables to call these functions from interrupt contextes. Due to dependencies it is expected that an actual scheduler implementation is providing an ISR variant of the `thread_woken_func` function. Signed-off-by: Simon Kuenzer <simon@unikraft.io> Reviewed-by: Sergiu Moga <sergiu@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GitHub-Closes: #1148

This commit compiles the implementation of `schedcoop_thread_woken()` as ISR-safe. The same function is used for both: non-ISR and ISR contexts. This is safe to do because by definition an ISR-safe function can be called from non-ISR context but not vice versa. Signed-off-by: Simon Kuenzer <simon@unikraft.io> Reviewed-by: Sergiu Moga <sergiu@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GitHub-Closes: #1148

This commit provides an ISR-safe variant of `uk_semaphore_up()`. This enables unblocking of threads that wait on a semaphore from an interrupt context. Signed-off-by: Simon Kuenzer <simon@unikraft.io> Reviewed-by: Sergiu Moga <sergiu@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GitHub-Closes: #1148

This commit uses the ISR-safe semaphore variant (`uk_semaphore_up_isr()`) so that `uk_netdev_drv_rx_event()` can be called from a drivers' interrupt context. Signed-off-by: Simon Kuenzer <simon@unikraft.io> Reviewed-by: Sergiu Moga <sergiu@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GitHub-Closes: #1148

This commit provides an ISR-safe variant of `uk_boot_shutdown_req()`. THis function is safe to be called from interrupt contexts, like a driver interrupt. The commit tries to avoid to duplicate code and compiles `shutdown_req.c` two times: once without ISR properties and another time with ISR property. Signed-off-by: Simon Kuenzer <simon@unikraft.io> Reviewed-by: Sergiu Moga <sergiu@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GitHub-Closes: #1148

This commit documents the shutdown request event queue within `include/uk/plat/bootstrap.h`. The idea is that a platform declare the queue and raises events with drivers. Any library can then register shutdown handlers to this queue and can take according actions. Signed-off-by: Simon Kuenzer <simon@unikraft.io> Reviewed-by: Sergiu Moga <sergiu@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GitHub-Closes: #1148

This commit registers the shutdown request function `uk_boot_shutdown_req_isr()` to the shutdown request event queue. This allows drivers to trigger a shutdown which is then performed by the "init" thread. Signed-off-by: Simon Kuenzer <simon@unikraft.io> Reviewed-by: Sergiu Moga <sergiu@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GitHub-Closes: #1148

This commit adopts the glue code to work with the Unikraft updates introduced with [PR #1148](unikraft/unikraft#1148). Signed-off-by: Simon Kuenzer <simon@unikraft.io> Reviewed-by: Sergiu Moga <sergiu@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GitHub-Closes: #40

This commit adopts the glue code to work with the Unikraft updates introduced with [PR #1148](unikraft/unikraft#1148). Signed-off-by: Simon Kuenzer <simon@unikraft.io>

skuenzer requested review from a team as code owners October 26, 2023 22:01

skuenzer assigned razvand Oct 31, 2023

skuenzer requested review from mogasergiu and removed request for a team October 31, 2023 13:57

mschlumpp reviewed Nov 15, 2023

View reviewed changes

include/uk/init.h Show resolved Hide resolved

skuenzer force-pushed the skuenzer/feat/shutdown branch from f87cfc0 to 9892f94 Compare November 15, 2023 10:51

mogasergiu reviewed Nov 27, 2023

View reviewed changes

lib/ukboot/boot.c Outdated Show resolved Hide resolved

lib/uklock/include/uk/isr/semaphore.h Outdated Show resolved Hide resolved

lib/ukboot/shutdown_req.c Show resolved Hide resolved

skuenzer force-pushed the skuenzer/feat/shutdown branch from 9892f94 to 4621a3c Compare November 27, 2023 13:57

mogasergiu requested changes Nov 27, 2023

View reviewed changes

nderjung added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Nov 28, 2023

skuenzer force-pushed the skuenzer/feat/shutdown branch 2 times, most recently from e692e59 to 6521376 Compare December 8, 2023 20:36

skuenzer added 3 commits December 8, 2023 21:37

skuenzer force-pushed the skuenzer/feat/shutdown branch from 6521376 to f5a3c80 Compare December 8, 2023 20:37

skuenzer mentioned this pull request Dec 8, 2023

Update to latest inittab changes unikraft/lib-lwip#40

Merged

skuenzer force-pushed the staging branch from 654b66c to 4244c95 Compare December 9, 2023 00:11

mogasergiu approved these changes Dec 9, 2023

View reviewed changes

razvand requested a review from mschlumpp December 11, 2023 10:17

razvand approved these changes Dec 11, 2023

View reviewed changes

skuenzer changed the base branch from staging to staging-1148 December 11, 2023 13:52

skuenzer merged commit 265949b into unikraft:staging-1148 Dec 11, 2023
9 checks passed

This was referenced Dec 30, 2023

Unikraft System Shutdown #571

Closed

plat/kvm/arm: Zero out the .bss on runtime setup #443

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lib/ukboot: Shutdown feature #1148

lib/ukboot: Shutdown feature #1148

skuenzer commented Oct 26, 2023

mogasergiu left a comment

skuenzer commented Nov 27, 2023

mogasergiu commented Nov 27, 2023

mogasergiu left a comment

mogasergiu left a comment

razvand left a comment

lib/ukboot: Shutdown feature #1148

lib/ukboot: Shutdown feature #1148

Conversation

skuenzer commented Oct 26, 2023

Base target

Description of changes

mogasergiu left a comment

Choose a reason for hiding this comment

skuenzer commented Nov 27, 2023

mogasergiu commented Nov 27, 2023

mogasergiu left a comment

Choose a reason for hiding this comment

mogasergiu left a comment

Choose a reason for hiding this comment

razvand left a comment

Choose a reason for hiding this comment