New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 780291 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocked on:
issue 782453



Sign in to add a comment

large part of delta payload is generated from free-space

Project Member Reported by ahass...@chromium.org, Oct 31 2017

Issue description

According to aosp/system/update_engine/payload_generator/ext2_filesystem.h:54 <free-space> is reserved of unallocated blocks. In theory for the purpose of our signed rootfs file system, these unallocated blocks should be zero. If they are not, then the delta-generator will try to generate a patch for all these unallocated blocks. Some experiments shows that on average 15MB of a delta payload is dedicated to these unallocated blocks. That's sometimes almost half of the whole delta payload. The reason is since these blocks are not zero, the delta-generator creates SOURCE_BSDIFF operation for them and the result can be large.

I guess the reason that these unallocated blocks are non-zero is that we sometimes write files into the partition and then remove them later. Or the partition is not zero'd in the first place.

Is there any way we can zero out these unallocated blocks so we can get smaller payloads?

Please, CC anyone whom you might think can help.
 
Looking at an image:
./mount_gpt_image.sh -f ../build/images/eve/latest/ -i chromiumos_test_image.bin

/dev/loop0 is the root partition:
With "sudo debugfs /dev/loop0", and 'ffb <X>' command we can identify the first X free blocks.

After collecting all the free blocks, 54783 out of 101851 (53%) were not free.
-- script used --
zero=0
for b in $(cat /tmp/all_ffb.txt); do
  if [ $(sudo dd if=/dev/loop0 skip=$b count=1 bs=4K status=none| cksum | cut -d ' ' -f 1) -ne 3018728591 ] ; then
   : $(( zero += 1 ))
  fi
done
echo $zero
---

Obviously zero_free_space() is not doing its job. Digging to find out why it is necessary to begin with.


Owner: gwendal@chromium.org
Status: Started (was: Untriaged)
Using truncate to prepare the image did not help (although it reduces the image size on the disk). CL incoming.
zero_free_space as-is is a noop, it does not help with zeroing more free blocks. 
I try not reaching the error, still a noop.
fstrim did not help, although it claims it discarded all the free blocks. 
However reading after the umount, all the free blocked are zero'ed, the fact the loop device is mounted may alter the result. 
@gwendal: Thanks for working on this. If this gets fixed, it will easily shed off 15MB off the delta payloads. That's a lot of junk. I'm guessing the saving would be even larger for full payloads.
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/crosutils/+/6e81f5b149bc06e7d426bbb902d57c71b3a38305

commit 6e81f5b149bc06e7d426bbb902d57c71b3a38305
Author: Gwendal Grignou <gwendal@chromium.org>
Date: Mon Nov 06 11:12:34 2017

build_image: reclain empty block with fstrim

The current method - writing a file with 0s until the root partition is full and
then deleting it - is not working. Using the script below, ~50% of the blocks
are reported as non 0s.
fstrim is able to reclaim those blocks, > 98% of free blocks are 0s.

BUG=chromium:780291
TEST=Check code is ran:
/mnt/host/source/src/build/images/eve/R64-10090.0.2017_11_02_1153-a1/rootfs:
349.6 MiB (366538752 bytes) trimmed

Check most of the empty blocks are empty with the following sequence:
Load the root partition
sudo losetup -o 163577856 -f image.bin
List all the free blocks:
sudo debugfs -R 'ffb 10000000000' /dev/loop2 | sed 's/Free blocks found: //' > /tmp/all_fb.txt
Count the number of zero'd free blocks:
zero=0
for b in $(cat /tmp/all_fb.txt); doi
  if [ $(sudo dd if=image.bin skip=$(( b + 39936 )) count=1 bs=4K status=none| cksum | cut -d ' ' ^C 1) -eq 3018728591 ] ; then
    : $(( zero += 1 ))
  fi
done
echo $zero

// 39936 * 4096 == 163577856 the usual location of the root partition.
// 3018728591 cksum of a 4K sectors full of 0s

Old image: 50223 out of 90619 are zero'd
New image: 99821 out of 100255 are zero'd

Change-Id: Iba5c86862e52482722e8cb66452bc39989d5223d
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/751809
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/6e81f5b149bc06e7d426bbb902d57c71b3a38305/build_library/base_image_util.sh

Thanks gwendal@ for fixing this. I just checked and the kevin-paladin builds and all the ones after your change have their full payload reduced by about 25MB. That's great. It would be around 15MB for the delta payloads :)

Cc: senj@chromium.org
senj@: You might want to check this in AOSP too to make sure it doesn't have the same problem.
Owner: senj@chromium.org
Status: Assigned (was: Started)

Comment 9 by senj@chromium.org, Nov 6 2017

Status: Fixed (was: Assigned)
I just tried generating a delta payload in AOSP and I don't see any <free-space> in the log, probably because Android uses make_ext4fs to make system image instead of mounting loop device.
Blockedon: 782453
Project Member

Comment 11 by bugdroid1@chromium.org, Nov 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/crosutils/+/33e68f9e22a5433852d669c099630a1b410bc5f6

commit 33e68f9e22a5433852d669c099630a1b410bc5f6
Author: Amin Hassani <ahassani@google.com>
Date: Wed Nov 08 23:10:10 2017

build_image: Do not do fstrim for unmounted filesystems

CL:751809 removed a check to bypass zeroing unallocated blocks when the
filesystem is unmounted like squashfs or ubifs partitions. This CL puts
that check back.

BUG=chromium:780291,chromium:782453,b:69042809
TEST=precq runs

Change-Id: Ia81926c3d1821e1ee2ea84b7cabe846f8fdbde2d
Reviewed-on: https://chromium-review.googlesource.com/757158
Commit-Ready: Ian Coolidge <icoolidge@google.com>
Tested-by: Ian Coolidge <icoolidge@google.com>
Reviewed-by: Gwendal Grignou <gwendal@google.com>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/33e68f9e22a5433852d669c099630a1b410bc5f6/build_library/base_image_util.sh

Status: (was: Fixed)
So, I just made a delta out of two recent recovery images and I still see free spaces are not zeros. I think we fixed it for base images, but when modifying it for recovery images, we still remove some other files and not zeroing them. The extra size in delta payload is about 10 MB now for recovery image. So, If the recovery image are served to users (delta or full), then we need to fix it.
I'll look into the mod_image_for_recovery.sh to see if we can use the same script there too.
Owner: ahass...@chromium.org
Status: Started
creating a recovery image doesn't touch the rootfs, only the kernel slots

if you're looking at official images, you'll want to check platform/vboot_reference/scripts/image_signing/
Project Member

Comment 15 by bugdroid1@chromium.org, Jan 3 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/vboot_reference/+/2de354af77fe2737374845afe20f963a24f68cb4

commit 2de354af77fe2737374845afe20f963a24f68cb4
Author: Amin Hassani <ahassani@google.com>
Date: Wed Jan 03 22:23:26 2018

image_signing: fix zeroing free space

We are not zeroing the free space properly before signing official images. This
patch fixes it by using fstrim instead of dd. More info can be found in
CL:751809.

BRANCH=none
BUG=chromium:780291
TEST=used sign_official_build.sh to sign two recovery images (these images produced <zero-space> file) with dev keys. Then generated delta update between the two new images. This time there was no <zero-space> file between the two images.

Change-Id: Ib97fb206f5c8bcfd97c43d075990c7fcdaad6f7e
Reviewed-on: https://chromium-review.googlesource.com/848237
Commit-Ready: Amin Hassani <ahassani@chromium.org>
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/2de354af77fe2737374845afe20f963a24f68cb4/scripts/image_signing/strip_boot_from_image.sh

Project Member

Comment 16 by bugdroid1@chromium.org, Jan 11 2018

Labels: merge-merged-firmware-fizz-10139.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/vboot_reference/+/0ac5d4c357bf68a13bfbb7b3dc72b40f5300f231

commit 0ac5d4c357bf68a13bfbb7b3dc72b40f5300f231
Author: Amin Hassani <ahassani@google.com>
Date: Thu Jan 11 08:23:13 2018

image_signing: fix zeroing free space

We are not zeroing the free space properly before signing official images. This
patch fixes it by using fstrim instead of dd. More info can be found in
CL:751809.

BRANCH=none
BUG=chromium:780291
TEST=used sign_official_build.sh to sign two recovery images (these images produced <zero-space> file) with dev keys. Then generated delta update between the two new images. This time there was no <zero-space> file between the two images.

Change-Id: Ib97fb206f5c8bcfd97c43d075990c7fcdaad6f7e
Reviewed-on: https://chromium-review.googlesource.com/848237
Commit-Ready: Amin Hassani <ahassani@chromium.org>
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
(cherry picked from commit 2de354af77fe2737374845afe20f963a24f68cb4)
Reviewed-on: https://chromium-review.googlesource.com/861603
Reviewed-by: Shelley Chen <shchen@chromium.org>
Commit-Queue: Shelley Chen <shchen@chromium.org>
Tested-by: Shelley Chen <shchen@chromium.org>

[modify] https://crrev.com/0ac5d4c357bf68a13bfbb7b3dc72b40f5300f231/scripts/image_signing/strip_boot_from_image.sh

ooops. It turned out the latest change did not affect anything either. I have to delve into it more to figure out why.

Sign in to add a comment