| Message ID | 1553868440-26476-3-git-send-email-ynezz@true.cz |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | x86: Fix small disk space in squashfs overlay | expand |
On 2019-03-29 15:07, Petr Štetiar wrote: > In order to share common functionality across platforms. > > Signed-off-by: Petr Štetiar <ynezz@true.cz> > --- > include/image-commands.mk | 3 +-- > include/image.mk | 9 +++++++++ > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/include/image-commands.mk b/include/image-commands.mk > index 8251a81..06c084c 100644 > --- a/include/image-commands.mk > +++ b/include/image-commands.mk > @@ -230,8 +230,7 @@ define Build/append-uboot > endef > > define Build/pad-to > - dd if=$@ of=$@.new bs=$(1) conv=sync > - mv $@.new $@ > + $(call Image/pad-to,$@,$(1)) > endef > > define Build/pad-extra > diff --git a/include/image.mk b/include/image.mk > index 6d9e347..8b84c8c 100644 > --- a/include/image.mk > +++ b/include/image.mk > @@ -180,6 +180,15 @@ ifeq ($(strip $(call kernel_patchver_ge,4.17.0)),1) > -Wno-unique_unit_address > endif > > +define Image/pad-to > + dd if=$(1) of=$(1).new bs=$(2) conv=sync > + mv $(1).new $(1) > +endef > + > +define Image/pad-root-squashfs > + $(call Image/pad-to,$(KDIR)/root.squashfs,$(if $(1),$(1),$(CONFIG_TARGET_ROOTFS_PARTSIZE)M)) > +endef The image should only be padded if CONFIG_TARGET_IMAGES_PAD is set. Keeping images not padded by default makes them faster to build and faster to write to storage on upgrade. I think it would be better to write a separate script to pad images to partition size for qemu purposes. - Felix
Felix Fietkau <nbd@nbd.name> [2019-03-29 18:14:36]: > On 2019-03-29 15:07, Petr Štetiar wrote: > > + > > +define Image/pad-root-squashfs > > + $(call Image/pad-to,$(KDIR)/root.squashfs,$(if $(1),$(1),$(CONFIG_TARGET_ROOTFS_PARTSIZE)M)) > > +endef > > The image should only be padded if CONFIG_TARGET_IMAGES_PAD is set. > Keeping images not padded by default makes them faster to build and > faster to write to storage on upgrade. I've just flashed the combined image to my apu2 and it works as expected, now I get it why it should be handled differently only for QEMU. Thanks. > I think it would be better to write a separate script to pad images to > partition size for qemu purposes. Since I find it quite useful to be able to download the image from snapshots and use it on QEMU for quick testing, I'm wondering how to handle this use case properly. Should we simply add CONFIG_QEMU_SQUASHFS_IMAGES (enabled by default) and CONFIG_QEMU_SQUASHFS_PARTSIZE=32 and use this information for generating of images for QEMU? On armvirt/malta it will simply produce working images based on this settings, on x86 it will produce two additional images usable in QEMU as well: openwrt-x86-64-qemu-combined-squashfs.img openwrt-x86-64-qemu-rootfs-squashfs.img What do you think? -- ynezz
On 2019-03-30 07:48, Petr Štetiar wrote: > Felix Fietkau <nbd@nbd.name> [2019-03-29 18:14:36]: > >> On 2019-03-29 15:07, Petr Štetiar wrote: >> > + >> > +define Image/pad-root-squashfs >> > + $(call Image/pad-to,$(KDIR)/root.squashfs,$(if $(1),$(1),$(CONFIG_TARGET_ROOTFS_PARTSIZE)M)) >> > +endef >> >> The image should only be padded if CONFIG_TARGET_IMAGES_PAD is set. >> Keeping images not padded by default makes them faster to build and >> faster to write to storage on upgrade. > > I've just flashed the combined image to my apu2 and it works as expected, now > I get it why it should be handled differently only for QEMU. Thanks. > >> I think it would be better to write a separate script to pad images to >> partition size for qemu purposes. > > Since I find it quite useful to be able to download the image from snapshots > and use it on QEMU for quick testing, I'm wondering how to handle this use > case properly. > > Should we simply add CONFIG_QEMU_SQUASHFS_IMAGES (enabled by default) and > CONFIG_QEMU_SQUASHFS_PARTSIZE=32 and use this information for generating of > images for QEMU? On armvirt/malta it will simply produce working images based > on this settings, on x86 it will produce two additional images usable in QEMU > as well: I would like to avoid adding generating padded images by default. I just came up with this simple script, which takes an existing image and pads it to full size: http://nbd.name/pad-image.pl With this, shipping padded images should be unnecessary. > openwrt-x86-64-qemu-combined-squashfs.img > openwrt-x86-64-qemu-rootfs-squashfs.img Another semi-related thing: do we really need those separate rootfs images? We could save some download server storage space by dropping them. If necessary, we could make another script like the one above to extract it from the combined image. - Felix
Felix Fietkau <nbd@nbd.name> [2019-03-30 09:18:39]: > >> On 2019-03-29 15:07, Petr Štetiar wrote: > > > > Should we simply add CONFIG_QEMU_SQUASHFS_IMAGES (enabled by default) and > > CONFIG_QEMU_SQUASHFS_PARTSIZE=32 and use this information for generating of > > images for QEMU? On armvirt/malta it will simply produce working images based > > on this settings, on x86 it will produce two additional images usable in QEMU > > as well: > > I would like to avoid adding generating padded images by default. To just save some space? We're gzipping the images by default, which makes image padded to 256M just 2.7M big. I find 256M overkill for testing, so that's why I've suggested 32M by default for QEMU padded images. > I just came up with this simple script, which takes an existing image and > pads it to full size: http://nbd.name/pad-image.pl With this, shipping > padded images should be unnecessary. Ok, if that is preferred, it's fine with me. But we should probably add some note somewhere, that in order to test this images in QEMU (x86, armvirt, malta), the images should be padded with this script in order to provide usable images. BTW that script needs some tweaking, it's producing here 2TB images from this file http://downloads.openwrt.org/snapshots/targets/x86/64/openwrt-x86-64-rootfs-squashfs.img.gz > > openwrt-x86-64-qemu-combined-squashfs.img > > openwrt-x86-64-qemu-rootfs-squashfs.img > > Another semi-related thing: do we really need those separate rootfs > images? We could save some download server storage space by dropping > them. If necessary, we could make another script like the one above to > extract it from the combined image. On x86, the only use case which comes to my mind is kernel testing, otherwise combined image is good enough (if I ommit the missing padding for QEMU). On armivrt/malta those images are probably necessary, as there is no combined image on this targets. Again, without padding the images are unusable as well. -- ynezz
On 2019-03-31 20:41, Petr Štetiar wrote: > Felix Fietkau <nbd@nbd.name> [2019-03-30 09:18:39]: > >> >> On 2019-03-29 15:07, Petr Štetiar wrote: >> > >> > Should we simply add CONFIG_QEMU_SQUASHFS_IMAGES (enabled by default) and >> > CONFIG_QEMU_SQUASHFS_PARTSIZE=32 and use this information for generating of >> > images for QEMU? On armvirt/malta it will simply produce working images based >> > on this settings, on x86 it will produce two additional images usable in QEMU >> > as well: >> >> I would like to avoid adding generating padded images by default. > > To just save some space? We're gzipping the images by default, which makes > image padded to 256M just 2.7M big. I find 256M overkill for testing, so > that's why I've suggested 32M by default for QEMU padded images. Not building padded images saves build time, because the system doesn't have to waste so many cycles compressing empty data. >> I just came up with this simple script, which takes an existing image and >> pads it to full size: http://nbd.name/pad-image.pl With this, shipping >> padded images should be unnecessary. > > Ok, if that is preferred, it's fine with me. But we should probably add some > note somewhere, that in order to test this images in QEMU (x86, armvirt, > malta), the images should be padded with this script in order to provide > usable images. I agree. > BTW that script needs some tweaking, it's producing here 2TB images from this file > > http://downloads.openwrt.org/snapshots/targets/x86/64/openwrt-x86-64-rootfs-squashfs.img.gz That's just the rootfs image, you're supposed to run the script on a combined image. What do you need that rootfs image for? - Felix
Hi, >>> I would like to avoid adding generating padded images by default. >> >> To just save some space? We're gzipping the images by default, which makes >> image padded to 256M just 2.7M big. I find 256M overkill for testing, so >> that's why I've suggested 32M by default for QEMU padded images. > Not building padded images saves build time, because the system doesn't > have to waste so many cycles compressing empty data. There's so many inefficiencies in the overall build process that gzip compressing a 256MB file is neglectible. >>> I just came up with this simple script, which takes an existing image and >>> pads it to full size: http://nbd.name/pad-image.pl With this, shipping >>> padded images should be unnecessary. >> >> Ok, if that is preferred, it's fine with me. But we should probably add some >> note somewhere, that in order to test this images in QEMU (x86, armvirt, >> malta), the images should be padded with this script in order to provide >> usable images. Most other targets ship image artifacts which are usable ootb, requiring one extra step to pad the combined images is a waste of user resources every single time. It also causes recurring confusion among users wanting to use x86 builds since the need for padding is neither documented, nor obvious while a combined.img.gz makes it obvious that it is an image file which requires decompression. I really don't see the huge problem with conservatively padding the combined image artifacts to something like 32 or 48MB, it must not even be 256M or more. ~ Jo
Felix Fietkau <nbd@nbd.name> [2019-03-31 22:21:59]: > >> I just came up with this simple script, which takes an existing image and > >> pads it to full size: http://nbd.name/pad-image.pl With this, shipping > >> padded images should be unnecessary. > > > BTW that script needs some tweaking, it's producing here 2TB images from this file > > > > http://downloads.openwrt.org/snapshots/targets/x86/64/openwrt-x86-64-rootfs-squashfs.img.gz > > That's just the rootfs image, you're supposed to run the script on a > combined image. It wasn't obvious to me, that it should be used on combined image. > What do you need that rootfs image for? That rootfs for x86 was just improperly chosen example, so to be more specific, how are we going to pad rootfs squashfs images for armvirt and malta, where we don't have combined image, just separate kernel and rootfs. -- ynezz
Jo-Philipp Wich <jo@mein.io> [2019-04-01 07:33:40]: > Most other targets ship image artifacts which are usable ootb, requiring > one extra step to pad the combined images is a waste of user resources > every single time. It also causes recurring confusion among users > wanting to use x86 builds x86, armvirt and malta builds to be more specific > I really don't see the huge problem with conservatively padding the > combined image artifacts to something like 32 or 48MB, it must not even > be 256M or more. Felix had a good point about sysupgrade, where we would needlesly write few dozen MBs of 0s. In order to avoid that, I would simply add following qemu variants to x86: openwrt-x86-{64,generic,geode,legacy}-qemu-{rootfs,combined}-squashfs.img and produce padded squashfs rootfs images for QEMU only targets by default: openwrt-armvirt-{32,64}-rootfs-squashfs.img openwrt-malta-{be,be64,le,le64}-rootfs-squashfs.img I'm wondering if we should introduce CONFIG_QEMU_SQUASHFS_PARTSIZE=32 and use this information for generating of images for QEMU. Without this config option I don't see, how could we handle this on x86. On armvirt/malta we could probably just set already available option CONFIG_TARGET_ROOTFS_PARTSIZE=32 and use that to pad the images. Or should we just forget about this QEMU specific CONFIG_QEMU_SQUASHFS_PARTSIZE and use CONFIG_TARGET_ROOTFS_PARTSIZE for QEMU images as well? We can decrease the default 256M to 32M on armvirt/malta. -- ynezz
Hi, > Felix had a good point about sysupgrade, where we would needlesly write few > dozen MBs of 0s. In order to avoid that, I would simply add following qemu > variants to x86: I wonder what kind of storage media on x86 is so brittle that writing a few dozen MB of 0s will cause any noticeable effect. > openwrt-x86-{64,generic,geode,legacy}-qemu-{rootfs,combined}-squashfs.img Not opposed to that, but that nullifies the entire "save space on servers" argument. Instead of having one set of images a few MB larger, we have a completely new set of larger images *and* the existing ones. > I'm wondering if we should introduce CONFIG_QEMU_SQUASHFS_PARTSIZE=32 and use > this information for generating of images for QEMU. > > Without this config option I don't see, how could we handle this on x86. On > armvirt/malta we could probably just set already available option > CONFIG_TARGET_ROOTFS_PARTSIZE=32 and use that to pad the images. > > Or should we just forget about this QEMU specific CONFIG_QEMU_SQUASHFS_PARTSIZE > and use CONFIG_TARGET_ROOTFS_PARTSIZE for QEMU images as well? We can decrease > the default 256M to 32M on armvirt/malta. I'd prefer the later option. ~ Jo
Jo-Philipp Wich <jo@mein.io> [2019-04-01 11:18:55]: > > Felix had a good point about sysupgrade, where we would needlesly write few > > dozen MBs of 0s. In order to avoid that, I would simply add following qemu > > variants to x86: > > I wonder what kind of storage media on x86 is so brittle that writing a > few dozen MB of 0s will cause any noticeable effect. SD cards could be slow. So it's going to make a difference if you're going to write 20M or 273M combined image. On my apu2 with some noname SDHC SD card (~6MB/s write max) I get ~15s difference in writting squashfs image (unpadded=0.29s Vs padded=15s). I'm not sure if this is negligible or acceptable tradeoff, considering how minor is the QEMU use case. > Not opposed to that, but that nullifies the entire "save space on servers" > argument. It wasn't my argument. > Instead of having one set of images a few MB larger, we have a completely > new set of larger images *and* the existing ones. I'm all in for having usable images out of the box, without any additional post-processing steps, but on the other hand I understand, that this might slow down noticeably some existing use cases. BTW, we're talking on x86 probably about additional 40M (~10M per each of 4 subtargets) and 8 additional QEMU ready images. On armvirt/malta we're going to produce(if agreed upon) padded images by default, so no difference there. -- ynezz
On Mon, Apr 1, 2019 at 7:25 PM Petr Štetiar <ynezz@true.cz> wrote: > Jo-Philipp Wich <jo@mein.io> [2019-04-01 11:18:55]: > > > Instead of having one set of images a few MB larger, we have a completely > > new set of larger images *and* the existing ones. > > I'm all in for having usable images out of the box, without any additional > post-processing steps, but on the other hand I understand, that this might > slow down noticeably some existing use cases. > > BTW, we're talking on x86 probably about additional 40M (~10M per each of 4 > subtargets) and 8 additional QEMU ready images. On armvirt/malta we're > going > to produce(if agreed upon) padded images by default, so no difference > there. > May I ask, is there consensus on how to approach this? (Apologies, I haven't been following this thread very closely.) Jeff
Hi, Bumping this thread after years. On 01/04/19 11:03, Jo-Philipp Wich wrote: >>>> I would like to avoid adding generating padded images by default. >>> >>>> I just came up with this simple script, which takes an existing image and >>>> pads it to full size: http://nbd.name/pad-image.pl With this, shipping >>>> padded images should be unnecessary. >>> >>> Ok, if that is preferred, it's fine with me. But we should probably add some >>> note somewhere, that in order to test this images in QEMU (x86, armvirt, >>> malta), the images should be padded with this script in order to provide >>> usable images. > > Most other targets ship image artifacts which are usable ootb, requiring > one extra step to pad the combined images is a waste of user resources > every single time. It also causes recurring confusion among users > wanting to use x86 builds since the need for padding is neither > documented, nor obvious while a combined.img.gz makes it obvious that it > is an image file which requires decompression. > > I really don't see the huge problem with conservatively padding the > combined image artifacts to something like 32 or 48MB, it must not even > be 256M or more. I build large squashfs images (16GB, 32GB) for x86_64 running on Xeon processors for high workload gateways running VPN, Squid, Snort etc. Till OpenWrt 19.07, I was able to build 32GB images on a machine with only 8GB of RAM and 8GB of swap. sysupgrade also worked fine on those images. But since 21.02, I am getting the *memory exhausted* error. Below is the build log snippet for 23.05.2: <log> dd if=/home/devuser/auto-build/hopbox-os/hopbox-openwrt/build_dir/target-x86_64_musl/linux-x86_64/root.squashfs >> /home/devuser/auto-build/hopbox-os/hopbox-openwrt/build_dir/target-x86_64_musl/linux-x86_64/tmp/hopbox-arthur-23.05.2.1-x86-64-generic-squashfs-rootfs.img.gz 39891+1 records in 39891+1 records out 20424365 bytes (20 MB, 19 MiB) copied, 0.0445292 s, 459 MB/s dd if=/home/devuser/auto-build/hopbox-os/hopbox-openwrt/build_dir/target-x86_64_musl/linux-x86_64/tmp/hopbox-arthur-23.05.2.1-x86-64-generic-squashfs-rootfs.img.gz of=/home/devuser/auto-build/hopbox-os/hopbox-openwrt/build_dir/target-x86_64_musl/linux-x86_64/tmp/hopbox-arthur-23.05.2.1-x86-64-generic-squashfs-rootfs.img.gz.new bs=16642998272 conv=sync dd: memory exhausted by input buffer of size 16642998272 bytes (16 GiB) </log> Here the block size is set to 16GiB to pad rootfs and there are not enough system resources available on the build host to be able honour that. I don't see any special case to handle x86/x86_64 in image-commands.mk Is there a way I can disable padding for x86_64? Thanks in advance. Regards, Nishant
Nishant Sharma <codemarauder@gmail.com> [2024-02-29 20:11:19]: Hi, > dd if=/home/devuser/auto-build/hopbox-os/hopbox-openwrt/build_dir/target-x86_64_musl/linux-x86_64/tmp/hopbox-arthur-23.05.2.1-x86-64-generic-squashfs-rootfs.img.gz of=/home/devuser/auto-build/hopbox-os/hopbox-openwrt/build_dir/target-x86_64_musl/linux-x86_64/tmp/hopbox-arthur-23.05.2.1-x86-64-generic-squashfs-rootfs.img.gz.new > bs=16642998272 conv=sync > > dd: memory exhausted by input buffer of size 16642998272 bytes (16 GiB) can you check https://patchwork.ozlabs.org/project/openwrt/patch/20240401102511.495791-1-ynezz@true.cz/ ? Thanks! Cheers, Petr
Hi Petr, On 01/04/24 15:58, Petr Štetiar wrote: >> dd: memory exhausted by input buffer of size 16642998272 bytes (16 GiB) > > can you check https://patchwork.ozlabs.org/project/openwrt/patch/20240401102511.495791-1-ynezz@true.cz/ ? Thanks! Thanks a lot for the fix. I tested it by building a 16GB squashfs EFI image for x86_64 on a host with just 8GB of RAM and build was successful without any errors. But, it takes a lot of time (around 20 minutes) writing this image to the device, which I suspect is due to padding. Earlier, it used to take less than 2 minutes. Is it possible to get "CONFIG_TARGET_IMAGES_PAD" back or some other switch that can be flipped to disable padding for images? Regards, Nishant
diff --git a/include/image-commands.mk b/include/image-commands.mk index 8251a81..06c084c 100644 --- a/include/image-commands.mk +++ b/include/image-commands.mk @@ -230,8 +230,7 @@ define Build/append-uboot endef define Build/pad-to - dd if=$@ of=$@.new bs=$(1) conv=sync - mv $@.new $@ + $(call Image/pad-to,$@,$(1)) endef define Build/pad-extra diff --git a/include/image.mk b/include/image.mk index 6d9e347..8b84c8c 100644 --- a/include/image.mk +++ b/include/image.mk @@ -180,6 +180,15 @@ ifeq ($(strip $(call kernel_patchver_ge,4.17.0)),1) -Wno-unique_unit_address endif +define Image/pad-to + dd if=$(1) of=$(1).new bs=$(2) conv=sync + mv $(1).new $(1) +endef + +define Image/pad-root-squashfs + $(call Image/pad-to,$(KDIR)/root.squashfs,$(if $(1),$(1),$(CONFIG_TARGET_ROOTFS_PARTSIZE)M)) +endef + # $(1) source dts file # $(2) target dtb file # $(3) extra CPP flags
In order to share common functionality across platforms. Signed-off-by: Petr Štetiar <ynezz@true.cz> --- include/image-commands.mk | 3 +-- include/image.mk | 9 +++++++++ 2 files changed, 10 insertions(+), 2 deletions(-)