-
-
Notifications
You must be signed in to change notification settings - Fork 11.1k
image: respect TARGET_PER_DEVICE_ROOTFS for initramfs #12959
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
Conversation
09e6f97 to
9194e26
Compare
|
Pushed a fix for images without TARGET_PER_DEVICE_ROOTFS There is still an issue with parallel build. I noticed that .config for initramfs might be based on an empty .config.old. Maybe two initramfs images are being built in parallel or .config.old was never generated. |
|
Does this PR fix this or has nothing to do with it? |
Nothing to do. However, if that issue is related to a too big kernel/rootfs, this PR might allow to move some code/packages from the default selection into the devices. |
|
The parallel build fails because multiple images are built in parallel. How can we avoid parallelization only during kernel rebuild? |
9194e26 to
4073f6f
Compare
|
I cleaned the changes to keep as much as possible the original calls. The issue is how to mutex Kernel/CompileImage/Initramfs. If I let them as is, Makefile will try to rebuild kernel images in parallel for each set of packages and Kernel/CompileImage/Initramfs is not thread-safe. You'll get a broken kernel config. |
4073f6f to
f1b5bfa
Compare
|
As a workaround, I'm simply disabling parallel jobs in image.mk when initramfs and multi rootfs are both enabled. |
|
@luizluca Hi, this is a very good feature and I think we should try to make it work... but we need to be careful and it may take some time... A good approach to this given how crucial this is follow some practice:
Hope you understand why I'm asking to follow these 2 thing. Aside from them I read the last commit and I have a question... I wonder if it's possible to create something like a 2 stage creation.
I wonder if it's possible to create the generic rootfs and then add the required packages for each device. This way in theory the generic rootfs implementation can be used and just modified for the task. One thing we should do is check if appending stuff results in saving some build time or not. |
What really matters is to get the feature merged. ;-)
Revisiting that, I believe we do need to touch that file. As an alternative approach is to copy the file into a temporary directory and append that directory. The goal of that commit is to keep the rootfs untouched as it will be shared between initramfs and squashfs images. Also, /generic/other-files/ might not be the best name for a single file used only by initramfs. Maybe /generic/initramfs-files/ and copy them as a directory?
Sure. I'll isolate the first commit and submit it. The second one does nothing if you don't give an argument to Kernel/CompileImage/Initramfs.
I don't create a new rootfs for initramfs. I just reuse the one already created by squashfs images. And that one is already copied from a shared
It might not save now as copying/deleting a file is not relevant for the build time. However, it will save the time allowing the reuse of an rootfs already generated for squashfs. |
|
I'd like to start taking a look at this problem, but I don't know much about this part of the makefile yet. the $eval that you added to me is a red flag and shouldn't be necessary. do you remember why you added it? |
You mean this one? In order to expand $$(ROOTFS_ID/$(1)), you need that eval. It is not different from other defines that use ROOTFS_ID |
|
The difficult here is that when you have a "define" with a sequence of independent commands (new line not escaped), you cannot run that as a single mutex context (i.e.: $(call locked ...). So, we either need to convert Kernel/CompileImage/Initramfs into a single shell command (with lots of "", including defines called from it), some other Makefile black magic to avoid two initramfs to be built at the same time or simply disable parallel image building when initramfs with multi rootfs. Also, there is the single extra file (/init) copied for initramfs. It would be nice if we could inject that respecting the $SOURCE_DATE_EPOCH. I really don't like the cp....build...rm strategy in use today nor a new temporary directory only for setting the mtime. However, it seems that the rm approach is the easier one. Kernel does use $KBUILD_BUILD_TIMESTAMP for links, dirs and devs. Maybe we could add a new option to set it for all file. |
f1b5bfa to
1b16160
Compare
|
I pushed a simplified PR. It has just two simple commits. The first is just a preparation, allowing the user to define a different target-dir. I removed the part of including the /init as an appended directory for now. I still believe it would be cleaner than cp/rm but I would need to use a tmp directory to set mtime. There is a list of FIXMEs in the last commit message:
For this last one, I even created a simple PoC: define xxx
target$(1):
@echo starting xxx $(1)
$(call yyy,$(1))
endef
define yyy
@echo starting yyy $(1)
@sleep 1
$(call zzz,v$(1))
@echo finishing yyy $(1)
endef
define zzz
@echo doing zzz $(1)
@sleep 1
endef
all: $(foreach i,1 2 3,target$i)
$(foreach i,1 2 3,$(eval $(call xxx,$(i))))If I And with And I need xxx to run in parallel but yyy in mutex: |
|
@dangowrt btw i analyzed the problem a bit and this is not trivial at all... One solution might be get out the big guns and manually create alle the files... And this cause all kind of problems as notice here with parallel build... But I may have found a funny solution... Considering this is the last step and everything has been already built.... We can do a meme thing and create a separate linux directory for each device, symbolic link for everyone and copy only the directory that will be affected... I just tested this and it seems to work correctly... I still need to test this with the PER_DEVICE_ROOTFS tho... but it seems to be the only solution currently... |
1b16160 to
9cf51b3
Compare
5a279ba to
296c6d6
Compare
|
adding a new which can be done with a pattern substitution the tricky part is debuging enough to figure out which variable represents that list of items that we must build strictly in order. I'm not sure what it is just by looking at the diff here... I wish I had time to dive deeper into this but I'm too busy in real life, let alone all of the smaller problems here that are in my todo list. However, I have a suspicion that some of the changes currently presented here may not be necessary if we set up structured dependency chains like this between unique images.
I think we ought to have unique file names for each instance of each component of each image using the device name as the beginning of the filename, just like it is for the final image in the bin directory, but only when this configure option is enabled, otherwise the prepended part of the filename could just be "common". For the rootfs there ought to be a "common" subdirectory for all files of the rootfs which will always be common, which will then become the template for each individual image by copying everything from there into the actual rootfs directory that will be squashed. That way, uniqueness of the name of the rootfs directory before squashing is not necessary. The rootfs directory should also be deleted after each image has finished building since unlike the kernel, one can select many packages and make the rootfs extremely large, so having a rootfs directory for each image could be a massive hog on disk space. |
|
Further test scrapped my idea of link and stuff... Locking is mandatory for initramfs generation... Alternative is to copy the entire linux directory but that is really not something doable... The only remaining thing is the handling of the separate cpio... It seems they might comflict. Aside from that this is pretty much ready. |
|
yeah we should stay away from symlinking here... there ought to be a way to get the linux build system to use existing binaries or objects to wrap up a new compresssed image without a full rebuild and install it under a new name... but maybe that would require patching the linux makefiles? |
Allow Kernel/CompileImage/Initramfs to use a different rootfs location. If the additional arg is not defined, TARGET_DIR is used by default. This allows the caller to customize the kernel initramfs for different rootfs. Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> [ simplify commit and rework commit description ] Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Rework SetInitramfs functions to take a second arg to define the location of the .config. This is needed in preparation for PER_ROOTFS Initramfs support as we will prepare .config in dedicated directory and use them only later when the image is actually built. Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Simplify SetInitramfs compression ALGO config setup by using Makefile foreach. While at it also make it more readable. Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Initramfs images were using a common rootfs (TARGET_DIR) for all devices, ignoring TARGET_PER_DEVICE_ROOTFS. If a single device required a package to build a functional initramfs image, it should be included by default for all devices or that device should be isolated into a new subtarget. Now the initramfs will be built using the target-specific Implementing Per Device Rootfs for Initramfs is not trivial as the rootfs needs to be embedded in the kernel image. The kernel supports an option to define the initramfs location and the image generation for the kernel can't be run in parallel as other checks are done to config and other arch dependent files. To handle this, we prepare a config for each rootfs and we generate the images under lock to prevent problem with parallel execution. Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> [ rework implementation for locking support ] Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Fit command makes use of CONFIG_TARGET_ROOTFS_INITRAMFS_SEPARATE as the cpio is provided externally and is not embedded in the kernel image. As done with embedded cpio, also handle PER_DEVICE_ROOTFS by generating a cpio for each rootfs and reference them by the ROOTFS_ID generated previously. The generated cpio are placed in the linux directory + the package ID. Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
With PER_DEVICE_ROOTFS, rebuilding kernel is needed to embed the cpio image in the kernel image. With ROOTFS_INITRAMFS_SEPARATE, the cpio image is external hence we can reuse the same kernel image without rebuilding it. Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Allow Kernel/CompileImage/Initramfs to use a different rootfs location. If the additional arg is not defined, TARGET_DIR is used by default. This allows the caller to customize the kernel initramfs for different rootfs. Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> [ simplify commit and rework commit description ] Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Rework SetInitramfs functions to take a second arg to define the location of the .config. This is needed in preparation for PER_ROOTFS Initramfs support as we will prepare .config in dedicated directory and use them only later when the image is actually built. Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Simplify SetInitramfs compression ALGO config setup by using Makefile foreach. While at it also make it more readable. Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Initramfs images were using a common rootfs (TARGET_DIR) for all devices, ignoring TARGET_PER_DEVICE_ROOTFS. If a single device required a package to build a functional initramfs image, it should be included by default for all devices or that device should be isolated into a new subtarget. Now the initramfs will be built using the target-specific Implementing Per Device Rootfs for Initramfs is not trivial as the rootfs needs to be embedded in the kernel image. The kernel supports an option to define the initramfs location and the image generation for the kernel can't be run in parallel as other checks are done to config and other arch dependent files. To handle this, we prepare a config for each rootfs and we generate the images under lock to prevent problem with parallel execution. Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> [ rework implementation for locking support ] Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Fit command makes use of CONFIG_TARGET_ROOTFS_INITRAMFS_SEPARATE as the cpio is provided externally and is not embedded in the kernel image. As done with embedded cpio, also handle PER_DEVICE_ROOTFS by generating a cpio for each rootfs and reference them by the ROOTFS_ID generated previously. The generated cpio are placed in the linux directory + the package ID. Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
With PER_DEVICE_ROOTFS, rebuilding kernel is needed to embed the cpio image in the kernel image. With ROOTFS_INITRAMFS_SEPARATE, the cpio image is external hence we can reuse the same kernel image without rebuilding it. Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Allow Kernel/CompileImage/Initramfs to use a different rootfs location. If the additional arg is not defined, TARGET_DIR is used by default. This allows the caller to customize the kernel initramfs for different rootfs. Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> [ simplify commit and rework commit description ] Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Rework SetInitramfs functions to take a second arg to define the location of the .config. This is needed in preparation for PER_ROOTFS Initramfs support as we will prepare .config in dedicated directory and use them only later when the image is actually built. Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Simplify SetInitramfs compression ALGO config setup by using Makefile foreach. While at it also make it more readable. Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Initramfs images were using a common rootfs (TARGET_DIR) for all devices, ignoring TARGET_PER_DEVICE_ROOTFS. If a single device required a package to build a functional initramfs image, it should be included by default for all devices or that device should be isolated into a new subtarget. Now the initramfs will be built using the target-specific Implementing Per Device Rootfs for Initramfs is not trivial as the rootfs needs to be embedded in the kernel image. The kernel supports an option to define the initramfs location and the image generation for the kernel can't be run in parallel as other checks are done to config and other arch dependent files. To handle this, we prepare a config for each rootfs and we generate the images under lock to prevent problem with parallel execution. Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> [ rework implementation for locking support ] Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Fit command makes use of CONFIG_TARGET_ROOTFS_INITRAMFS_SEPARATE as the cpio is provided externally and is not embedded in the kernel image. As done with embedded cpio, also handle PER_DEVICE_ROOTFS by generating a cpio for each rootfs and reference them by the ROOTFS_ID generated previously. The generated cpio are placed in the linux directory + the package ID. Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
With PER_DEVICE_ROOTFS, rebuilding kernel is needed to embed the cpio image in the kernel image. With ROOTFS_INITRAMFS_SEPARATE, the cpio image is external hence we can reuse the same kernel image without rebuilding it. Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Allow Kernel/CompileImage/Initramfs to use a different rootfs location. If the additional arg is not defined, TARGET_DIR is used by default. This allows the caller to customize the kernel initramfs for different rootfs. Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> [ simplify commit and rework commit description ] Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Rework SetInitramfs functions to take a second arg to define the location of the .config. This is needed in preparation for PER_ROOTFS Initramfs support as we will prepare .config in dedicated directory and use them only later when the image is actually built. Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Simplify SetInitramfs compression ALGO config setup by using Makefile foreach. While at it also make it more readable. Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Initramfs images were using a common rootfs (TARGET_DIR) for all devices, ignoring TARGET_PER_DEVICE_ROOTFS. If a single device required a package to build a functional initramfs image, it should be included by default for all devices or that device should be isolated into a new subtarget. Now the initramfs will be built using the target-specific Implementing Per Device Rootfs for Initramfs is not trivial as the rootfs needs to be embedded in the kernel image. The kernel supports an option to define the initramfs location and the image generation for the kernel can't be run in parallel as other checks are done to config and other arch dependent files. To handle this, we prepare a config for each rootfs and we generate the images under lock to prevent problem with parallel execution. Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> [ rework implementation for locking support ] Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Fit command makes use of CONFIG_TARGET_ROOTFS_INITRAMFS_SEPARATE as the cpio is provided externally and is not embedded in the kernel image. As done with embedded cpio, also handle PER_DEVICE_ROOTFS by generating a cpio for each rootfs and reference them by the ROOTFS_ID generated previously. The generated cpio are placed in the linux directory + the package ID. Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
With PER_DEVICE_ROOTFS, rebuilding kernel is needed to embed the cpio image in the kernel image. With ROOTFS_INITRAMFS_SEPARATE, the cpio image is external hence we can reuse the same kernel image without rebuilding it. Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Allow Kernel/CompileImage/Initramfs to use a different rootfs location. If the additional arg is not defined, TARGET_DIR is used by default. This allows the caller to customize the kernel initramfs for different rootfs. Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> [ simplify commit and rework commit description ] Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Rework SetInitramfs functions to take a second arg to define the location of the .config. This is needed in preparation for PER_ROOTFS Initramfs support as we will prepare .config in dedicated directory and use them only later when the image is actually built. Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Simplify SetInitramfs compression ALGO config setup by using Makefile foreach. While at it also make it more readable. Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Initramfs images were using a common rootfs (TARGET_DIR) for all devices, ignoring TARGET_PER_DEVICE_ROOTFS. If a single device required a package to build a functional initramfs image, it should be included by default for all devices or that device should be isolated into a new subtarget. Now the initramfs will be built using the target-specific Implementing Per Device Rootfs for Initramfs is not trivial as the rootfs needs to be embedded in the kernel image. The kernel supports an option to define the initramfs location and the image generation for the kernel can't be run in parallel as other checks are done to config and other arch dependent files. To handle this, we prepare a config for each rootfs and we generate the images under lock to prevent problem with parallel execution. Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> [ rework implementation for locking support ] Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Fit command makes use of CONFIG_TARGET_ROOTFS_INITRAMFS_SEPARATE as the cpio is provided externally and is not embedded in the kernel image. As done with embedded cpio, also handle PER_DEVICE_ROOTFS by generating a cpio for each rootfs and reference them by the ROOTFS_ID generated previously. The generated cpio are placed in the linux directory + the package ID. Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
With PER_DEVICE_ROOTFS, rebuilding kernel is needed to embed the cpio image in the kernel image. With ROOTFS_INITRAMFS_SEPARATE, the cpio image is external hence we can reuse the same kernel image without rebuilding it. Link: openwrt#12959 Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
This PR tries to solve the problem of devices that requires a specific package to work (i.e.: switch drivers) that are not required or even conflict with other devices.
I'm not an expert in Makefile and most of this PR was done using try-n-error. I'm not sure, for example, if it is OK to include kernel-defaults.mk in image.mk. It might be better ways to get the same result.
The PR was still not tested with other situation, like
without TARGET_PER_DEVICE_ROOTFS, with CONFIG_TARGET_ROOTFS_INITRAMFS_SEPARATE or CONFIG_EXTERNAL_CPIO.It also failed sometimes with parallel jobs (-j21) because multiple initramfs are built in parallel:
How can I avoid it?
Fixes initramfs after 575ec7a b168a07 6e0f0ea when building for multiple devices.