From 8acb5dc1c59382b12261a24aa28b4e8377796192 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Mon, 3 Dec 2018 13:39:33 -0800 Subject: [PATCH] liblp: Allocate "b" extents in the second half of super. When allocating "b" partitions on a non-retrofit A/B device, prioritize regions occuring in the second half of the super partition. To make this effective, the region covering the midpoint sector is split into two additional regions. This will allow OTAs to avoid unecessary fragmentation, since each slot's partitions will be grouped together, leaving a large chunk of contiguous space available when the OTA deletes the target slot. Since updates are not allowed to consume more than half of the super partition, this should guarantee one extent per partition. Note that, if this restriction is not in place (for example, a developer flashes a massive "system_b"), then an additional extent will be allocated due to the region that was split. Bug: 120433288 Test: liblp_test gtest Change-Id: I1797e59e14c8b0d4d0e6855a1d984e8159b21df2 --- fs_mgr/liblp/builder.cpp | 44 ++++++++++++++++++++++++++++ fs_mgr/liblp/builder_test.cpp | 38 ++++++++++++++++++++++++ fs_mgr/liblp/include/liblp/builder.h | 2 ++ 3 files changed, 84 insertions(+) diff --git a/fs_mgr/liblp/builder.cpp b/fs_mgr/liblp/builder.cpp index da86d75f3..699b9e701 100644 --- a/fs_mgr/liblp/builder.cpp +++ b/fs_mgr/liblp/builder.cpp @@ -578,6 +578,12 @@ bool MetadataBuilder::GrowPartition(Partition* partition, uint64_t aligned_size) CHECK_NE(sectors_per_block, 0); CHECK(sectors_needed % sectors_per_block == 0); + if (IsABDevice() && !IsRetrofitDevice() && GetPartitionSlotSuffix(partition->name()) == "_b") { + // Allocate "a" partitions top-down and "b" partitions bottom-up, to + // minimize fragmentation during OTA. + free_regions = PrioritizeSecondHalfOfSuper(free_regions); + } + // Find gaps that we can use for new extents. Note we store new extents in a // temporary vector, and only commit them if we are guaranteed enough free // space. @@ -621,6 +627,40 @@ bool MetadataBuilder::GrowPartition(Partition* partition, uint64_t aligned_size) return true; } +std::vector MetadataBuilder::PrioritizeSecondHalfOfSuper( + const std::vector& free_list) { + const auto& super = block_devices_[0]; + uint64_t first_sector = super.first_logical_sector; + uint64_t last_sector = super.size / LP_SECTOR_SIZE; + uint64_t midpoint = first_sector + (last_sector - first_sector) / 2; + + // Choose an aligned sector for the midpoint. This could lead to one half + // being slightly larger than the other, but this will not restrict the + // size of partitions (it might lead to one extra extent if "B" overflows). + midpoint = AlignSector(super, midpoint); + + std::vector first_half; + std::vector second_half; + for (const auto& region : free_list) { + // Note: deprioritze if not the main super partition. Even though we + // don't call this for retrofit devices, we will allow adding additional + // block devices on non-retrofit devices. + if (region.device_index != 0 || region.end <= midpoint) { + first_half.emplace_back(region); + continue; + } + if (region.start < midpoint && region.end > midpoint) { + // Split this into two regions. + first_half.emplace_back(region.device_index, region.start, midpoint); + second_half.emplace_back(region.device_index, midpoint, region.end); + } else { + second_half.emplace_back(region); + } + } + second_half.insert(second_half.end(), first_half.begin(), first_half.end()); + return second_half; +} + void MetadataBuilder::ShrinkPartition(Partition* partition, uint64_t aligned_size) { partition->ShrinkTo(aligned_size); } @@ -930,5 +970,9 @@ bool MetadataBuilder::IsABDevice() const { return android::base::GetBoolProperty("ro.build.ab_update", false); } +bool MetadataBuilder::IsRetrofitDevice() const { + return GetBlockDevicePartitionName(block_devices_[0]) != LP_METADATA_DEFAULT_PARTITION_NAME; +} + } // namespace fs_mgr } // namespace android diff --git a/fs_mgr/liblp/builder_test.cpp b/fs_mgr/liblp/builder_test.cpp index 926fe1257..7833a2529 100644 --- a/fs_mgr/liblp/builder_test.cpp +++ b/fs_mgr/liblp/builder_test.cpp @@ -720,3 +720,41 @@ TEST_F(BuilderTest, UnsuffixedPartitions) { ASSERT_EQ(builder->AddPartition("system", 0), nullptr); ASSERT_NE(builder->AddPartition("system_a", 0), nullptr); } + +TEST_F(BuilderTest, ABExtents) { + BlockDeviceInfo device_info("super", 10_GiB, 768 * 1024, 0, 4096); + + // A and B slots should be allocated from separate halves of the partition, + // to mitigate allocating too many extents. (b/120433288) + MetadataBuilder::OverrideABForTesting(true); + auto builder = MetadataBuilder::New(device_info, 65536, 2); + ASSERT_NE(builder, nullptr); + Partition* system_a = builder->AddPartition("system_a", 0); + ASSERT_NE(system_a, nullptr); + Partition* system_b = builder->AddPartition("system_b", 0); + ASSERT_NE(system_b, nullptr); + ASSERT_TRUE(builder->ResizePartition(system_a, 2_GiB)); + ASSERT_TRUE(builder->ResizePartition(system_b, 2_GiB)); + + builder->RemovePartition("system_a"); + system_a = builder->AddPartition("system_a", 0); + ASSERT_NE(system_a, nullptr); + ASSERT_TRUE(builder->ResizePartition(system_a, 3_GiB)); + + EXPECT_EQ(system_a->extents().size(), static_cast(1)); + EXPECT_EQ(system_b->extents().size(), static_cast(1)); + ASSERT_TRUE(builder->ResizePartition(system_b, 6_GiB)); + EXPECT_EQ(system_b->extents().size(), static_cast(3)); + + unique_ptr exported = builder->Export(); + ASSERT_NE(exported, nullptr); + ASSERT_EQ(exported->extents.size(), static_cast(4)); + EXPECT_EQ(exported->extents[0].target_data, 10487808); + EXPECT_EQ(exported->extents[0].num_sectors, 4194304); + EXPECT_EQ(exported->extents[1].target_data, 14682624); + EXPECT_EQ(exported->extents[1].num_sectors, 6288896); + EXPECT_EQ(exported->extents[2].target_data, 6292992); + EXPECT_EQ(exported->extents[2].num_sectors, 2099712); + EXPECT_EQ(exported->extents[3].target_data, 1536); + EXPECT_EQ(exported->extents[3].num_sectors, 6291456); +} diff --git a/fs_mgr/liblp/include/liblp/builder.h b/fs_mgr/liblp/include/liblp/builder.h index 4bb38d674..f477b4b1c 100644 --- a/fs_mgr/liblp/include/liblp/builder.h +++ b/fs_mgr/liblp/include/liblp/builder.h @@ -275,6 +275,7 @@ class MetadataBuilder { const LpMetadataPartition& source); bool ImportPartition(const LpMetadata& metadata, const LpMetadataPartition& source); bool IsABDevice() const; + bool IsRetrofitDevice() const; struct Interval { uint32_t device_index; @@ -294,6 +295,7 @@ class MetadataBuilder { std::vector GetFreeRegions() const; void ExtentsToFreeList(const std::vector& extents, std::vector* free_regions) const; + std::vector PrioritizeSecondHalfOfSuper(const std::vector& free_list); static bool sABOverrideValue; static bool sABOverrideSet;