liblp: Reclaim wasted space from unaligned partitions.
When allocating a partition with a size that is unaligned (to the
optimal alignment), the remaining sectors are wasted since they are
never reallocated. This is because the free list is guaranteed to only
contain optimally-aligned regions. Unfortunately this means when a
partition is resized, we are wasting a small amount of space each time.
On a non-A/B device, this could wind up being significant.
For example, with an alignment of 512KiB, a 4KiB partition at offset 0
will waste 508KiB of space. The next extent to be allocated by any
partition will start at the next 512KiB.
To address this, we round up extents to the optimal alignment. This
means partitions may wind up slightly over-allocated, versus before,
where they would waste space by making it unavailable.
Bug: 120434950
Test: liblp_test gtest
Change-Id: I555209b301058555526cc4309f7049ae81cf877d
diff --git a/fs_mgr/liblp/builder.cpp b/fs_mgr/liblp/builder.cpp
index 699b9e7..07f9d66 100644
--- a/fs_mgr/liblp/builder.cpp
+++ b/fs_mgr/liblp/builder.cpp
@@ -606,14 +606,23 @@
}
uint64_t sectors = std::min(sectors_needed, region.length());
+ if (sectors < region.length()) {
+ const auto& block_device = block_devices_[region.device_index];
+ if (block_device.alignment) {
+ const uint64_t alignment = block_device.alignment / LP_SECTOR_SIZE;
+ sectors = AlignTo(sectors, alignment);
+ sectors = std::min(sectors, region.length());
+ }
+ }
CHECK(sectors % sectors_per_block == 0);
auto extent = std::make_unique<LinearExtent>(sectors, region.device_index, region.start);
new_extents.push_back(std::move(extent));
- sectors_needed -= sectors;
- if (!sectors_needed) {
+ if (sectors >= sectors_needed) {
+ sectors_needed = 0;
break;
}
+ sectors_needed -= sectors;
}
if (sectors_needed) {
LERROR << "Not enough free space to expand partition: " << partition->name();
diff --git a/fs_mgr/liblp/builder_test.cpp b/fs_mgr/liblp/builder_test.cpp
index 7833a25..3793964 100644
--- a/fs_mgr/liblp/builder_test.cpp
+++ b/fs_mgr/liblp/builder_test.cpp
@@ -209,8 +209,8 @@
ASSERT_TRUE(builder->ResizePartition(a, a->size() + 4096));
ASSERT_TRUE(builder->ResizePartition(b, b->size() + 4096));
}
- EXPECT_EQ(a->size(), 40960);
- EXPECT_EQ(b->size(), 40960);
+ EXPECT_EQ(a->size(), 7864320);
+ EXPECT_EQ(b->size(), 7864320);
unique_ptr<LpMetadata> exported = builder->Export();
ASSERT_NE(exported, nullptr);
@@ -218,7 +218,7 @@
// Check that each starting sector is aligned.
for (const auto& extent : exported->extents) {
ASSERT_EQ(extent.target_type, LP_TARGET_TYPE_LINEAR);
- EXPECT_EQ(extent.num_sectors, 8);
+ EXPECT_EQ(extent.num_sectors, 1536);
uint64_t lba = extent.target_data * LP_SECTOR_SIZE;
uint64_t aligned_lba = AlignTo(lba, device_info.alignment, device_info.alignment_offset);
@@ -645,7 +645,7 @@
EXPECT_EQ(metadata->extents[1].target_type, LP_TARGET_TYPE_LINEAR);
EXPECT_EQ(metadata->extents[1].target_data, 1472);
EXPECT_EQ(metadata->extents[1].target_source, 1);
- EXPECT_EQ(metadata->extents[2].num_sectors, 129088);
+ EXPECT_EQ(metadata->extents[2].num_sectors, 129600);
EXPECT_EQ(metadata->extents[2].target_type, LP_TARGET_TYPE_LINEAR);
EXPECT_EQ(metadata->extents[2].target_data, 1472);
EXPECT_EQ(metadata->extents[2].target_source, 2);
@@ -744,17 +744,41 @@
EXPECT_EQ(system_a->extents().size(), static_cast<size_t>(1));
EXPECT_EQ(system_b->extents().size(), static_cast<size_t>(1));
ASSERT_TRUE(builder->ResizePartition(system_b, 6_GiB));
- EXPECT_EQ(system_b->extents().size(), static_cast<size_t>(3));
+ EXPECT_EQ(system_b->extents().size(), static_cast<size_t>(2));
unique_ptr<LpMetadata> exported = builder->Export();
ASSERT_NE(exported, nullptr);
- ASSERT_EQ(exported->extents.size(), static_cast<size_t>(4));
+ ASSERT_EQ(exported->extents.size(), static_cast<size_t>(3));
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);
+ EXPECT_EQ(exported->extents[0].num_sectors, 10483712);
+ EXPECT_EQ(exported->extents[1].target_data, 6292992);
+ EXPECT_EQ(exported->extents[1].num_sectors, 2099712);
+ EXPECT_EQ(exported->extents[2].target_data, 1536);
+ EXPECT_EQ(exported->extents[2].num_sectors, 6291456);
+}
+
+TEST_F(BuilderTest, PartialExtents) {
+ // super has a minimum extent size of 768KiB.
+ BlockDeviceInfo device_info("super", 1_GiB, 768 * 1024, 0, 4096);
+ auto builder = MetadataBuilder::New(device_info, 65536, 1);
+ ASSERT_NE(builder, nullptr);
+ Partition* system = builder->AddPartition("system", 0);
+ ASSERT_NE(system, nullptr);
+ Partition* vendor = builder->AddPartition("vendor", 0);
+ ASSERT_NE(vendor, nullptr);
+ ASSERT_TRUE(builder->ResizePartition(system, device_info.alignment + 4096));
+ ASSERT_TRUE(builder->ResizePartition(vendor, device_info.alignment));
+ ASSERT_EQ(system->size(), device_info.alignment * 2);
+ ASSERT_EQ(vendor->size(), device_info.alignment);
+
+ ASSERT_TRUE(builder->ResizePartition(system, device_info.alignment * 2));
+ ASSERT_EQ(system->extents().size(), static_cast<size_t>(1));
+
+ unique_ptr<LpMetadata> exported = builder->Export();
+ ASSERT_NE(exported, nullptr);
+ ASSERT_EQ(exported->extents.size(), static_cast<size_t>(2));
+ EXPECT_EQ(exported->extents[0].target_data, 1536);
+ EXPECT_EQ(exported->extents[0].num_sectors, 3072);
+ EXPECT_EQ(exported->extents[1].target_data, 4608);
+ EXPECT_EQ(exported->extents[1].num_sectors, 1536);
}