| Message ID | 20240822183742.3550055-2-dario.binacchi@amarulasolutions.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | tinyinit and stm32f746_disco_sd_defconfig | expand |
On Thu, 22 Aug 2024 20:37:40 +0200 Dario Binacchi <dario.binacchi@amarulasolutions.com> wrote: > DEVELOPERS | 1 + > package/Config.in | 1 + > package/tinyinit/Config.in | 10 ++++++++++ > package/tinyinit/init | 32 ++++++++++++++++++++++++++++++++ > package/tinyinit/tinyinit.mk | 12 ++++++++++++ > 5 files changed, 56 insertions(+) We also need to change busybox.mk to ensure that tinyinit is built before Busybox: # Packages that provide commands that may also be busybox applets: BUSYBOX_DEPENDENCIES = \ ... $(if $(BR2_PACKAGE_TINYINIT),tinyinit) \ > diff --git a/package/tinyinit/Config.in b/package/tinyinit/Config.in > new file mode 100644 > index 000000000000..2ceb8e191e23 > --- /dev/null > +++ b/package/tinyinit/Config.in > @@ -0,0 +1,10 @@ > +config BR2_PACKAGE_TINYINIT > + bool "tinyinit" > + depends on BR2_INIT_NONE > + help > + A Linux tiny initialization script suitable for resource > + limited systems, which can be used as an alternative to the > + one provided by Busybox. > + > +comment "tinyinit needs BR2_INIT_NONE, i. e. no init system installed" > + depends on !BR2_INIT_NONE I find this a bit odd. In the end, shouldn't we simply promote tinyinit as an init implementation, and have its own BR2_INIT_TINYINIT entry? Also, should BR2_PACKAGE_TINYINIT select busybox? Or do we pretend that it is possible to use it with some other implementations of mount and other basic commands? Perhaps this is a good case to use "imply BR2_PACKAGE_BUSYBOX", so that BR2_PACKAGE_TINYINIT enables busybox by default, but still allows removing busybox if really the user knows what (s)he is doing? > +define TINYINIT_INSTALL_TARGET_CMDS > + $(INSTALL) -m 0755 -D $(TINYINIT_PKGDIR)/init $(TARGET_DIR)/sbin/init > + (cd $(TARGET_DIR); ln -sf /sbin/init init) Hmm. I initially thought this symlink shouldn't be needed, as it's created by fs/cpio/cpio.mk when you're building a cpio for initramfs: ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y) define ROOTFS_CPIO_ADD_INIT if [ ! -e $(TARGET_DIR)/init ]; then \ ln -sf sbin/init $(TARGET_DIR)/init; \ fi endef else # devtmpfs does not get automounted when initramfs is used. # Add a pre-init script to mount it before running init # We must have /dev/console very early, even before /init runs, # for stdin/stdout/stderr define ROOTFS_CPIO_ADD_INIT if [ ! -e $(TARGET_DIR)/init ]; then \ $(INSTALL) -m 0755 fs/cpio/init $(TARGET_DIR)/init; \ fi mkdir -p $(TARGET_DIR)/dev mknod -m 0622 $(TARGET_DIR)/dev/console c 5 1 endef But actually, in the case of a non-static /dev, it doesn't do a symlink, but adds a pre-init script, and your tinyinit already does the tasks of this pre-init provided by fs/cpio/init. So I guess what you did is probably OK, but maybe a comment would be useful. The only downside of your approach is that it leaves a /init symlink that isn't used/needed for non-initramfs use-cases, but perhaps that's OK. Thomas
Hello Thomas and Yann, On Fri, Aug 23, 2024 at 6:17 PM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > On Thu, 22 Aug 2024 20:37:40 +0200 > Dario Binacchi <dario.binacchi@amarulasolutions.com> wrote: > > > DEVELOPERS | 1 + > > package/Config.in | 1 + > > package/tinyinit/Config.in | 10 ++++++++++ > > package/tinyinit/init | 32 ++++++++++++++++++++++++++++++++ > > package/tinyinit/tinyinit.mk | 12 ++++++++++++ > > 5 files changed, 56 insertions(+) > > We also need to change busybox.mk to ensure that tinyinit is built > before Busybox: > > # Packages that provide commands that may also be busybox applets: > BUSYBOX_DEPENDENCIES = \ > ... > $(if $(BR2_PACKAGE_TINYINIT),tinyinit) \ > > > > diff --git a/package/tinyinit/Config.in b/package/tinyinit/Config.in > > new file mode 100644 > > index 000000000000..2ceb8e191e23 > > --- /dev/null > > +++ b/package/tinyinit/Config.in > > @@ -0,0 +1,10 @@ > > +config BR2_PACKAGE_TINYINIT > > + bool "tinyinit" > > + depends on BR2_INIT_NONE > > + help > > + A Linux tiny initialization script suitable for resource > > + limited systems, which can be used as an alternative to the > > + one provided by Busybox. > > + > > +comment "tinyinit needs BR2_INIT_NONE, i. e. no init system installed" > > + depends on !BR2_INIT_NONE > > I find this a bit odd. In the end, shouldn't we simply promote tinyinit > as an init implementation, and have its own BR2_INIT_TINYINIT entry? I removed TINYINIT from the init selections based on Yann's suggestion: https://patchwork.ozlabs.org/project/buildroot/patch/20240820191927.2126281-2-dario.binacchi@amarulasolutions.com/ IMHO, I believe there are positive aspects in both implementations, but my knowledge on the subject is more limited compared to yours. From my side, I can assure you that there's no problem to restore tinyinit as a new entry in the list of init options. Thanks and regards, Dario > > Also, should BR2_PACKAGE_TINYINIT select busybox? Or do we pretend that > it is possible to use it with some other implementations of mount and > other basic commands? Perhaps this is a good case to use "imply > BR2_PACKAGE_BUSYBOX", so that BR2_PACKAGE_TINYINIT enables busybox by > default, but still allows removing busybox if really the user knows > what (s)he is doing? > > > +define TINYINIT_INSTALL_TARGET_CMDS > > + $(INSTALL) -m 0755 -D $(TINYINIT_PKGDIR)/init $(TARGET_DIR)/sbin/init > > + (cd $(TARGET_DIR); ln -sf /sbin/init init) > > Hmm. I initially thought this symlink shouldn't be needed, as it's > created by fs/cpio/cpio.mk when you're building a cpio for initramfs: > > ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y) > > define ROOTFS_CPIO_ADD_INIT > if [ ! -e $(TARGET_DIR)/init ]; then \ > ln -sf sbin/init $(TARGET_DIR)/init; \ > fi > endef > > else > # devtmpfs does not get automounted when initramfs is used. > # Add a pre-init script to mount it before running init > # We must have /dev/console very early, even before /init runs, > # for stdin/stdout/stderr > define ROOTFS_CPIO_ADD_INIT > if [ ! -e $(TARGET_DIR)/init ]; then \ > $(INSTALL) -m 0755 fs/cpio/init $(TARGET_DIR)/init; \ > fi > mkdir -p $(TARGET_DIR)/dev > mknod -m 0622 $(TARGET_DIR)/dev/console c 5 1 > endef > > But actually, in the case of a non-static /dev, it doesn't do a > symlink, but adds a pre-init script, and your tinyinit already does the > tasks of this pre-init provided by fs/cpio/init. > > So I guess what you did is probably OK, but maybe a comment would be > useful. The only downside of your approach is that it leaves a /init > symlink that isn't used/needed for non-initramfs use-cases, but perhaps > that's OK. > > > Thomas > -- > Thomas Petazzoni, co-owner and CEO, Bootlin > Embedded Linux and Kernel engineering and training > https://bootlin.com
Thomas, All, On 2024-08-23 18:17 +0200, Thomas Petazzoni spake thusly: > On Thu, 22 Aug 2024 20:37:40 +0200 > Dario Binacchi <dario.binacchi@amarulasolutions.com> wrote: [--SNIP--] > We also need to change busybox.mk to ensure that tinyinit is built > before Busybox: > > # Packages that provide commands that may also be busybox applets: > BUSYBOX_DEPENDENCIES = \ > ... > $(if $(BR2_PACKAGE_TINYINIT),tinyinit) \ Good catch! We also need that for tini, then. > > diff --git a/package/tinyinit/Config.in b/package/tinyinit/Config.in > > new file mode 100644 > > index 000000000000..2ceb8e191e23 > > --- /dev/null > > +++ b/package/tinyinit/Config.in > > @@ -0,0 +1,10 @@ > > +config BR2_PACKAGE_TINYINIT > > + bool "tinyinit" > > + depends on BR2_INIT_NONE > > + help > > + A Linux tiny initialization script suitable for resource > > + limited systems, which can be used as an alternative to the > > + one provided by Busybox. > > + > > +comment "tinyinit needs BR2_INIT_NONE, i. e. no init system installed" > > + depends on !BR2_INIT_NONE > I find this a bit odd. In the end, shouldn't we simply promote tinyinit > as an init implementation, and have its own BR2_INIT_TINYINIT entry? As Dario already pointed out, that was my suggestion to drop it from the init selection. The reasoning is that we need to know the init system when we need some infra for it. For example, for busybox/sysvinit, we need to install init scripts, for systemd, the units, and for openrc, the openrc "units". And the "None" init choice really is "Custom", in that there is always a PID-1 process, whether in a real system, a VM, or a container: it is the first process that is started in that environment. Now, whether that process is a real init system, a stub, or directly the final applicaiton, it is still a PID-1, and thus treated as 'init' by the kernel. The existing tini package is to be used a a PID-1 in containers (I use it extensively in that situation) and it does not need any infra in Buildroot (it can only spawn a single child and is a reaper); to my eyes, tiny-init is also very simple and does not require any infra in Buildroot. Both tini and tiny-init are cases for a "Custom" init (currently labelled "None"). Regards, Yann E. MORIN.
On Tue, 27 Aug 2024 22:07:04 +0200 "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > > # Packages that provide commands that may also be busybox applets: > > BUSYBOX_DEPENDENCIES = \ > > ... > > $(if $(BR2_PACKAGE_TINYINIT),tinyinit) \ > > Good catch! We also need that for tini, then. Yes. > > > +comment "tinyinit needs BR2_INIT_NONE, i. e. no init system installed" > > > + depends on !BR2_INIT_NONE > > I find this a bit odd. In the end, shouldn't we simply promote tinyinit > > as an init implementation, and have its own BR2_INIT_TINYINIT entry? > > As Dario already pointed out, that was my suggestion to drop it from the > init selection. > > The reasoning is that we need to know the init system when we need some > infra for it. For example, for busybox/sysvinit, we need to install init > scripts, for systemd, the units, and for openrc, the openrc "units". > > And the "None" init choice really is "Custom", in that there is always a > PID-1 process, whether in a real system, a VM, or a container: it is the > first process that is started in that environment. Now, whether that > process is a real init system, a stub, or directly the final applicaiton, > it is still a PID-1, and thus treated as 'init' by the kernel. > > The existing tini package is to be used a a PID-1 in containers (I use > it extensively in that situation) and it does not need any infra in > Buildroot (it can only spawn a single child and is a reaper); to my > eyes, tiny-init is also very simple and does not require any infra in > Buildroot. Both tini and tiny-init are cases for a "Custom" init > (currently labelled "None"). I understand your point, but: (1) It is really unclear for the user that they have to chose "None" as init to be able to use tinyinit (tinyinit is proposed with depends on BR2_INIT_NONE) (2) Due to (1), Dario has added a "tinyinit needs BR2_INIT_NONE, i. e. no init system installed" Config.in comment, which I also find odd So while I understand your point that tinyinit or tini don't need as much infrastructure/logic as busybox init/sysvinit/systemd/openrc, I find the current way they are presented to be very weird (but I can live with it). Thomas
Thomas, All, On 2024-08-29 23:47 +0200, Thomas Petazzoni spake thusly: > On Tue, 27 Aug 2024 22:07:04 +0200 > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > > As Dario already pointed out, that was my suggestion to drop it from the > > init selection. [--SNIP--] > I understand your point, but: > > (1) It is really unclear for the user that they have to chose "None" as > init to be able to use tinyinit (tinyinit is proposed with depends > on BR2_INIT_NONE) > (2) Due to (1), Dario has added a "tinyinit needs BR2_INIT_NONE, i. e. > no init system installed" Config.in comment, which I also find odd To me, it really all looked very obvious, exactly thanks to the comment. tiniy-init is also not a reaper, which dos not really make it an init either... Note that tini does not install itself as /init or /sbin/init, but just as /usr/bin/tini (and a symlink in /usr/libexec/docker/docker-init); tini is also a reaper. > So while I understand your point that tinyinit or tini don't need as > much infrastructure/logic as busybox init/sysvinit/systemd/openrc, I > find the current way they are presented to be very weird (but I can > live with it). Then I think we should also organise the selection list, like: Init system *** General purpose *** ( ) BusyBox ( ) systemV ( ) OpenRC ( ) systemd *** Special purpose, read help *** ( ) tini ( ) tinyinit ( ) Custom (aka none) *** My br2-ext (in /path/to/my-br2-ext) *** ( ) my-init (br2-external entries are added by support/scripts/br2-external). Regards, Yann E. MORIN.
diff --git a/DEVELOPERS b/DEVELOPERS index d7d0af35431b..45cc1cc48996 100644 --- a/DEVELOPERS +++ b/DEVELOPERS @@ -791,6 +791,7 @@ F: configs/imx8mn_bsh_smm_s2_defconfig F: configs/imx8mn_bsh_smm_s2_pro_defconfig F: configs/stm32f769_disco_sd_defconfig F: package/sscep/ +F: package/tinyinit/ F: package/uuu/ N: Dario Binacchi <dariobin@libero.it> diff --git a/package/Config.in b/package/Config.in index 317f859684fe..ddf378b13e01 100644 --- a/package/Config.in +++ b/package/Config.in @@ -2869,6 +2869,7 @@ menu "System tools" source "package/tar/Config.in" source "package/tealdeer/Config.in" source "package/thermald/Config.in" + source "package/tinyinit/Config.in" source "package/tpm-tools/Config.in" source "package/tpm2-abrmd/Config.in" source "package/tpm2-tools/Config.in" diff --git a/package/tinyinit/Config.in b/package/tinyinit/Config.in new file mode 100644 index 000000000000..2ceb8e191e23 --- /dev/null +++ b/package/tinyinit/Config.in @@ -0,0 +1,10 @@ +config BR2_PACKAGE_TINYINIT + bool "tinyinit" + depends on BR2_INIT_NONE + help + A Linux tiny initialization script suitable for resource + limited systems, which can be used as an alternative to the + one provided by Busybox. + +comment "tinyinit needs BR2_INIT_NONE, i. e. no init system installed" + depends on !BR2_INIT_NONE diff --git a/package/tinyinit/init b/package/tinyinit/init new file mode 100644 index 000000000000..fbcb481010c2 --- /dev/null +++ b/package/tinyinit/init @@ -0,0 +1,32 @@ +#!/bin/sh + +# This script replaces the default busybox init process to avoid having that +# process staying alive and sleeping in the background, (uselessly) consuming +# precious memory. + +# Mount procfs and sysfs +/bin/mount -t proc proc /proc +/bin/mount -t sysfs sysfs /sys + +# When the kernel is directly booted, devtmpfs is not automatically mounted. +# Manually mount it if needed. +devmnt=$(mount | grep -c devtmpfs) +if [ "${devmnt}" -eq 0 ]; then + /bin/mount -t devtmpfs devtmpfs /dev +fi + +# Use the /dev/console device node from devtmpfs if possible to not +# confuse glibc's ttyname_r(). +# This may fail (E.G. booted with console=), and errors from exec will +# terminate the shell, so use a subshell for the test +if (exec 0</dev/console) 2>/dev/null; then + exec 0</dev/console + exec 1>/dev/console + exec 2>/dev/console +fi + +# Clear memory to reduce page fragmentation +echo 3 > /proc/sys/vm/drop_caches + +# Finally, let's start an interactive shell +exec /bin/sh diff --git a/package/tinyinit/tinyinit.mk b/package/tinyinit/tinyinit.mk new file mode 100644 index 000000000000..c7bb144dee68 --- /dev/null +++ b/package/tinyinit/tinyinit.mk @@ -0,0 +1,12 @@ +################################################################################ +# +# tinyinit +# +################################################################################ + +define TINYINIT_INSTALL_TARGET_CMDS + $(INSTALL) -m 0755 -D $(TINYINIT_PKGDIR)/init $(TARGET_DIR)/sbin/init + (cd $(TARGET_DIR); ln -sf /sbin/init init) +endef + +$(eval $(generic-package))
This package contains a Linux init script suitable for resource-limited systems which can be used as an alternative to the one provided by Busybox. Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com> --- Changes v2 -> v3: - Add the init script - Drop the entry in the init selection in system/Config.in. Changes v1 -> v2: - Rename tinit to tinyinit. - Put the script inside the tinyinit package without the need to use a github repo. - Update the commit message. DEVELOPERS | 1 + package/Config.in | 1 + package/tinyinit/Config.in | 10 ++++++++++ package/tinyinit/init | 32 ++++++++++++++++++++++++++++++++ package/tinyinit/tinyinit.mk | 12 ++++++++++++ 5 files changed, 56 insertions(+) create mode 100644 package/tinyinit/Config.in create mode 100644 package/tinyinit/init create mode 100644 package/tinyinit/tinyinit.mk