From 9e95a02a87ebd8c0e14edc4904facdc8e0352fcc Mon Sep 17 00:00:00 2001 From: David Anderson Date: Tue, 31 Aug 2021 21:32:45 -0700 Subject: [PATCH] Don't sparse right-sized ext4 and erofs images. When we introduced Dynamic Partitions, we stopped giving readonly partitions fixed sizes. In addition we introduced deduplication for ext4. These two factors greatly reduce the impact of sparse images, since there aren't many fill blocks to optimize. This patch disables sparsing for images that are rightsized and do not explicitly specify extra reserved space. This makes the images a little easier to work with from an engineering perspective. They no longer have to be unsparsed to interact with any tooling. It also eases a potential source of bugs, as b/184225422 is not reproducible with sparsing off. On Pixel, the difference between the sparsed partitions and unsparsed is 12M (out of roughly 4G). Bug: 198001223 Test: make, treehugger, make target-files-package dynamic partitions are no longer sparse images Change-Id: I74459f8abe74a15a24ba5a40cf701e6af2db8179 --- core/Makefile | 21 +++++++++++++++++++++ tools/releasetools/build_image.py | 24 ++++++++++++++++++------ tools/releasetools/common.py | 6 +++++- 3 files changed, 44 insertions(+), 7 deletions(-) diff --git a/core/Makefile b/core/Makefile index f2d75851e2..63ec1a6d57 100644 --- a/core/Makefile +++ b/core/Makefile @@ -1601,6 +1601,18 @@ endif endif # PRODUCT_USE_DYNAMIC_PARTITIONS +# $(1) the partition variable component (eg SYSTEM) +# $(2) the partition prefix in properties (eg system) +# $(3) the image prop file +# $(4) optional "other" size +define add-sparse-flags-to-image-props +$(eval _size := $(BOARD_$(1)IMAGE_PARTITION_SIZE)) +$(eval _reserved := $(BOARD_$(1)IMAGE_PARTITION_RESERVED_SIZE)) +$(eval _headroom := $(PRODUCT_$(1)_HEADROOM)) +$(if $(or $(_size), $(_reserved), $(_headroom), $(4)),, + $(hide) echo "$(2)_disable_sparse=true" >> $(3)) +endef + # $(1): the path of the output dictionary file # $(2): a subset of "system vendor cache userdata product system_ext oem odm vendor_dlkm odm_dlkm" # $(3): additional "key=value" pairs to append to the dictionary file. @@ -1621,11 +1633,14 @@ $(if $(filter $(2),system),\ $(if $(PRODUCT_SYSTEM_BASE_FS_PATH),$(hide) echo "system_base_fs_file=$(PRODUCT_SYSTEM_BASE_FS_PATH)" >> $(1)) $(if $(PRODUCT_SYSTEM_HEADROOM),$(hide) echo "system_headroom=$(PRODUCT_SYSTEM_HEADROOM)" >> $(1)) $(if $(BOARD_SYSTEMIMAGE_PARTITION_RESERVED_SIZE),$(hide) echo "system_reserved_size=$(BOARD_SYSTEMIMAGE_PARTITION_RESERVED_SIZE)" >> $(1)) + $(call add-sparse-flags-to-image-props,SYSTEM,system,$(1)) $(hide) echo "system_selinux_fc=$(SELINUX_FC)" >> $(1) $(hide) echo "building_system_image=$(BUILDING_SYSTEM_IMAGE)" >> $(1) ) $(if $(filter $(2),system_other),\ $(hide) echo "building_system_other_image=$(BUILDING_SYSTEM_OTHER_IMAGE)" >> $(1) + $(if $(INTERNAL_SYSTEM_OTHER_PARTITION_SIZE),, + $(hide) echo "system_other_disable_sparse=true" >> $(1)) ) $(if $(filter $(2),userdata),\ $(if $(BOARD_USERDATAIMAGE_FILE_SYSTEM_TYPE),$(hide) echo "userdata_fs_type=$(BOARD_USERDATAIMAGE_FILE_SYSTEM_TYPE)" >> $(1)) @@ -1656,6 +1671,7 @@ $(if $(filter $(2),vendor),\ $(if $(BOARD_VENDORIMAGE_SQUASHFS_DISABLE_4K_ALIGN),$(hide) echo "vendor_squashfs_disable_4k_align=$(BOARD_VENDORIMAGE_SQUASHFS_DISABLE_4K_ALIGN)" >> $(1)) $(if $(PRODUCT_VENDOR_BASE_FS_PATH),$(hide) echo "vendor_base_fs_file=$(PRODUCT_VENDOR_BASE_FS_PATH)" >> $(1)) $(if $(BOARD_VENDORIMAGE_PARTITION_RESERVED_SIZE),$(hide) echo "vendor_reserved_size=$(BOARD_VENDORIMAGE_PARTITION_RESERVED_SIZE)" >> $(1)) + $(call add-sparse-flags-to-image-props,VENDOR,vendor,$(1)) $(hide) echo "vendor_selinux_fc=$(SELINUX_FC)" >> $(1) $(hide) echo "building_vendor_image=$(BUILDING_VENDOR_IMAGE)" >> $(1) ) @@ -1673,6 +1689,7 @@ $(if $(filter $(2),product),\ $(if $(BOARD_PRODUCTIMAGE_SQUASHFS_DISABLE_4K_ALIGN),$(hide) echo "product_squashfs_disable_4k_align=$(BOARD_PRODUCTIMAGE_SQUASHFS_DISABLE_4K_ALIGN)" >> $(1)) $(if $(PRODUCT_PRODUCT_BASE_FS_PATH),$(hide) echo "product_base_fs_file=$(PRODUCT_PRODUCT_BASE_FS_PATH)" >> $(1)) $(if $(BOARD_PRODUCTIMAGE_PARTITION_RESERVED_SIZE),$(hide) echo "product_reserved_size=$(BOARD_PRODUCTIMAGE_PARTITION_RESERVED_SIZE)" >> $(1)) + $(call add-sparse-flags-to-image-props,PRODUCT,product,$(1)) $(hide) echo "product_selinux_fc=$(SELINUX_FC)" >> $(1) $(hide) echo "building_product_image=$(BUILDING_PRODUCT_IMAGE)" >> $(1) ) @@ -1689,6 +1706,7 @@ $(if $(filter $(2),system_ext),\ $(if $(BOARD_SYSTEM_EXTIMAGE_SQUASHFS_BLOCK_SIZE),$(hide) echo "system_ext_squashfs_block_size=$(BOARD_SYSTEM_EXTIMAGE_SQUASHFS_BLOCK_SIZE)" >> $(1)) $(if $(BOARD_SYSTEM_EXTIMAGE_SQUASHFS_DISABLE_4K_ALIGN),$(hide) echo "system_ext_squashfs_disable_4k_align=$(BOARD_SYSTEM_EXTIMAGE_SQUASHFS_DISABLE_4K_ALIGN)" >> $(1)) $(if $(BOARD_SYSTEM_EXTIMAGE_PARTITION_RESERVED_SIZE),$(hide) echo "system_ext_reserved_size=$(BOARD_SYSTEM_EXTIMAGE_PARTITION_RESERVED_SIZE)" >> $(1)) + $(call add-sparse-flags-to-image-props,SYSTEM_EXT,system_ext,$(1)) $(hide) echo "system_ext_selinux_fc=$(SELINUX_FC)" >> $(1) $(hide) echo "building_system_ext_image=$(BUILDING_SYSTEM_EXT_IMAGE)" >> $(1) ) @@ -1704,6 +1722,7 @@ $(if $(filter $(2),odm),\ $(if $(BOARD_ODMIMAGE_SQUASHFS_DISABLE_4K_ALIGN),$(hide) echo "odm_squashfs_disable_4k_align=$(BOARD_ODMIMAGE_SQUASHFS_DISABLE_4K_ALIGN)" >> $(1)) $(if $(PRODUCT_ODM_BASE_FS_PATH),$(hide) echo "odm_base_fs_file=$(PRODUCT_ODM_BASE_FS_PATH)" >> $(1)) $(if $(BOARD_ODMIMAGE_PARTITION_RESERVED_SIZE),$(hide) echo "odm_reserved_size=$(BOARD_ODMIMAGE_PARTITION_RESERVED_SIZE)" >> $(1)) + $(call add-sparse-flags-to-image-props,ODM,odm,$(1)) $(hide) echo "odm_selinux_fc=$(SELINUX_FC)" >> $(1) $(hide) echo "building_odm_image=$(BUILDING_ODM_IMAGE)" >> $(1) ) @@ -1720,6 +1739,7 @@ $(if $(filter $(2),vendor_dlkm),\ $(if $(BOARD_VENDOR_DLKMIMAGE_SQUASHFS_BLOCK_SIZE),$(hide) echo "vendor_dlkm_squashfs_block_size=$(BOARD_VENDOR_DLKMIMAGE_SQUASHFS_BLOCK_SIZE)" >> $(1)) $(if $(BOARD_VENDOR_DLKMIMAGE_SQUASHFS_DISABLE_4K_ALIGN),$(hide) echo "vendor_dlkm_squashfs_disable_4k_align=$(BOARD_VENDOR_DLKMIMAGE_SQUASHFS_DISABLE_4K_ALIGN)" >> $(1)) $(if $(BOARD_VENDOR_DLKMIMAGE_PARTITION_RESERVED_SIZE),$(hide) echo "vendor_dlkm_reserved_size=$(BOARD_VENDOR_DLKMIMAGE_PARTITION_RESERVED_SIZE)" >> $(1)) + $(call add-sparse-flags-to-image-props,VENDOR_DLKM,vendor_dlkm,$(1)) $(hide) echo "vendor_dlkm_selinux_fc=$(SELINUX_FC)" >> $(1) $(hide) echo "building_vendor_dlkm_image=$(BUILDING_VENDOR_DLKM_IMAGE)" >> $(1) ) @@ -1734,6 +1754,7 @@ $(if $(filter $(2),odm_dlkm),\ $(if $(BOARD_ODM_DLKMIMAGE_SQUASHFS_BLOCK_SIZE),$(hide) echo "odm_dlkm_squashfs_block_size=$(BOARD_ODM_DLKMIMAGE_SQUASHFS_BLOCK_SIZE)" >> $(1)) $(if $(BOARD_ODM_DLKMIMAGE_SQUASHFS_DISABLE_4K_ALIGN),$(hide) echo "odm_dlkm_squashfs_disable_4k_align=$(BOARD_ODM_DLKMIMAGE_SQUASHFS_DISABLE_4K_ALIGN)" >> $(1)) $(if $(BOARD_ODM_DLKMIMAGE_PARTITION_RESERVED_SIZE),$(hide) echo "odm_dlkm_reserved_size=$(BOARD_ODM_DLKMIMAGE_PARTITION_RESERVED_SIZE)" >> $(1)) + $(call add-sparse-flags-to-image-props,ODM_DLKM,odm_dlkm,$(1)) $(hide) echo "odm_dlkm_selinux_fc=$(SELINUX_FC)" >> $(1) $(hide) echo "building_odm_dlkm_image=$(BUILDING_ODM_DLKM_IMAGE)" >> $(1) ) diff --git a/tools/releasetools/build_image.py b/tools/releasetools/build_image.py index f2ba3219f4..189da37ef5 100755 --- a/tools/releasetools/build_image.py +++ b/tools/releasetools/build_image.py @@ -256,9 +256,11 @@ def BuildImageMkfs(in_dir, prop_dict, out_file, target_out, fs_config): needs_casefold = prop_dict.get("needs_casefold", 0) needs_compress = prop_dict.get("needs_compress", 0) + disable_sparse = "disable_sparse" in prop_dict + if fs_type.startswith("ext"): build_command = [prop_dict["ext_mkuserimg"]] - if "extfs_sparse_flag" in prop_dict: + if "extfs_sparse_flag" in prop_dict and not disable_sparse: build_command.append(prop_dict["extfs_sparse_flag"]) run_e2fsck = True build_command.extend([in_dir, out_file, fs_type, @@ -303,7 +305,7 @@ def BuildImageMkfs(in_dir, prop_dict, out_file, target_out, fs_config): elif fs_type.startswith("erofs"): build_command = ["mkerofsimage.sh"] build_command.extend([in_dir, out_file]) - if "erofs_sparse_flag" in prop_dict: + if "erofs_sparse_flag" in prop_dict and not disable_sparse: build_command.extend([prop_dict["erofs_sparse_flag"]]) build_command.extend(["-m", prop_dict["mount_point"]]) if target_out: @@ -319,7 +321,7 @@ def BuildImageMkfs(in_dir, prop_dict, out_file, target_out, fs_config): elif fs_type.startswith("squash"): build_command = ["mksquashfsimage.sh"] build_command.extend([in_dir, out_file]) - if "squashfs_sparse_flag" in prop_dict: + if "squashfs_sparse_flag" in prop_dict and not disable_sparse: build_command.extend([prop_dict["squashfs_sparse_flag"]]) build_command.extend(["-m", prop_dict["mount_point"]]) if target_out: @@ -341,7 +343,7 @@ def BuildImageMkfs(in_dir, prop_dict, out_file, target_out, fs_config): elif fs_type.startswith("f2fs"): build_command = ["mkf2fsuserimg.sh"] build_command.extend([out_file, prop_dict["image_size"]]) - if "f2fs_sparse_flag" in prop_dict: + if "f2fs_sparse_flag" in prop_dict and not disable_sparse: build_command.extend([prop_dict["f2fs_sparse_flag"]]) if fs_config: build_command.extend(["-C", fs_config]) @@ -448,6 +450,8 @@ def BuildImage(in_dir, prop_dict, out_file, target_out=None): # or None if not applicable. verity_image_builder = verity_utils.CreateVerityImageBuilder(prop_dict) + disable_sparse = "disable_sparse" in prop_dict + if (prop_dict.get("use_dynamic_partition_size") == "true" and "partition_size" not in prop_dict): # If partition_size is not defined, use output of `du' + reserved_size. @@ -481,7 +485,7 @@ def BuildImage(in_dir, prop_dict, out_file, target_out=None): size // BYTES_IN_MB, prop_dict["extfs_inode_count"]) BuildImageMkfs(in_dir, prop_dict, out_file, target_out, fs_config) sparse_image = False - if "extfs_sparse_flag" in prop_dict: + if "extfs_sparse_flag" in prop_dict and not disable_sparse: sparse_image = True fs_dict = GetFilesystemCharacteristics(fs_type, out_file, sparse_image) os.remove(out_file) @@ -525,7 +529,7 @@ def BuildImage(in_dir, prop_dict, out_file, target_out=None): prop_dict["image_size"] = str(size) BuildImageMkfs(in_dir, prop_dict, out_file, target_out, fs_config) sparse_image = False - if "f2fs_sparse_flag" in prop_dict: + if "f2fs_sparse_flag" in prop_dict and not disable_sparse: sparse_image = True fs_dict = GetFilesystemCharacteristics(fs_type, out_file, sparse_image) os.remove(out_file) @@ -642,6 +646,7 @@ def ImagePropFromGlobalDict(glob_dict, mount_point): d["extfs_rsv_pct"] = "0" copy_prop("system_reserved_size", "partition_reserved_size") copy_prop("system_selinux_fc", "selinux_fc") + copy_prop("system_disable_sparse", "disable_sparse") elif mount_point == "system_other": # We inherit the selinux policies of /system since we contain some of its # files. @@ -668,6 +673,7 @@ def ImagePropFromGlobalDict(glob_dict, mount_point): d["extfs_rsv_pct"] = "0" copy_prop("system_reserved_size", "partition_reserved_size") copy_prop("system_selinux_fc", "selinux_fc") + copy_prop("system_disable_sparse", "disable_sparse") elif mount_point == "data": # Copy the generic fs type first, override with specific one if available. copy_prop("fs_type", "fs_type") @@ -708,6 +714,7 @@ def ImagePropFromGlobalDict(glob_dict, mount_point): d["extfs_rsv_pct"] = "0" copy_prop("vendor_reserved_size", "partition_reserved_size") copy_prop("vendor_selinux_fc", "selinux_fc") + copy_prop("vendor_disable_sparse", "disable_sparse") elif mount_point == "product": copy_prop("avb_product_hashtree_enable", "avb_hashtree_enable") copy_prop("avb_product_add_hashtree_footer_args", @@ -733,6 +740,7 @@ def ImagePropFromGlobalDict(glob_dict, mount_point): d["extfs_rsv_pct"] = "0" copy_prop("product_reserved_size", "partition_reserved_size") copy_prop("product_selinux_fc", "selinux_fc") + copy_prop("product_disable_sparse", "disable_sparse") elif mount_point == "system_ext": copy_prop("avb_system_ext_hashtree_enable", "avb_hashtree_enable") copy_prop("avb_system_ext_add_hashtree_footer_args", @@ -760,6 +768,7 @@ def ImagePropFromGlobalDict(glob_dict, mount_point): d["extfs_rsv_pct"] = "0" copy_prop("system_ext_reserved_size", "partition_reserved_size") copy_prop("system_ext_selinux_fc", "selinux_fc") + copy_prop("system_ext_disable_sparse", "disable_sparse") elif mount_point == "odm": copy_prop("avb_odm_hashtree_enable", "avb_hashtree_enable") copy_prop("avb_odm_add_hashtree_footer_args", @@ -783,6 +792,7 @@ def ImagePropFromGlobalDict(glob_dict, mount_point): d["extfs_rsv_pct"] = "0" copy_prop("odm_reserved_size", "partition_reserved_size") copy_prop("odm_selinux_fc", "selinux_fc") + copy_prop("odm_disable_sparse", "disable_sparse") elif mount_point == "vendor_dlkm": copy_prop("avb_vendor_dlkm_hashtree_enable", "avb_hashtree_enable") copy_prop("avb_vendor_dlkm_add_hashtree_footer_args", @@ -808,6 +818,7 @@ def ImagePropFromGlobalDict(glob_dict, mount_point): d["extfs_rsv_pct"] = "0" copy_prop("vendor_dlkm_reserved_size", "partition_reserved_size") copy_prop("vendor_dlkm_selinux_fc", "selinux_fc") + copy_prop("vendor_dlkm_disable_sparse", "disable_sparse") elif mount_point == "odm_dlkm": copy_prop("avb_odm_dlkm_hashtree_enable", "avb_hashtree_enable") copy_prop("avb_odm_dlkm_add_hashtree_footer_args", @@ -831,6 +842,7 @@ def ImagePropFromGlobalDict(glob_dict, mount_point): d["extfs_rsv_pct"] = "0" copy_prop("odm_dlkm_reserved_size", "partition_reserved_size") copy_prop("odm_dlkm_selinux_fc", "selinux_fc") + copy_prop("odm_dlkm_disable_sparse", "disable_sparse") elif mount_point == "oem": copy_prop("fs_type", "fs_type") copy_prop("oem_size", "partition_size") diff --git a/tools/releasetools/common.py b/tools/releasetools/common.py index f30e382ea8..c6800e83b8 100644 --- a/tools/releasetools/common.py +++ b/tools/releasetools/common.py @@ -1983,6 +1983,8 @@ def GetUserImage(which, tmpdir, input_zip, info_dict = LoadInfoDict(input_zip) is_sparse = info_dict.get("extfs_sparse_flag") + if info_dict.get(which + "_disable_sparse"): + is_sparse = False # When target uses 'BOARD_EXT4_SHARE_DUP_BLOCKS := true', images may contain # shared blocks (i.e. some blocks will show up in multiple files' block @@ -3851,12 +3853,14 @@ def GetCareMap(which, imgname): if not image_size: return None + disable_sparse = OPTIONS.info_dict.get(which + "_disable_sparse") + image_blocks = int(image_size) // 4096 - 1 assert image_blocks > 0, "blocks for {} must be positive".format(which) # For sparse images, we will only check the blocks that are listed in the care # map, i.e. the ones with meaningful data. - if "extfs_sparse_flag" in OPTIONS.info_dict: + if "extfs_sparse_flag" in OPTIONS.info_dict and not disable_sparse: simg = sparse_img.SparseImage(imgname) care_map_ranges = simg.care_map.intersect( rangelib.RangeSet("0-{}".format(image_blocks)))