liblp: Remove alignment_offset handling.
This code is complex and alignment_offset is not useful for
optimization. Remove it, and add another test that misaligned extents do
not cause overlapping partitions.
Bug: 154646936
Test: liblp_test gtests
Change-Id: I563122282e940e07a7ece97ed1a9846ad1f3253c
diff --git a/fs_mgr/liblp/builder.cpp b/fs_mgr/liblp/builder.cpp
index 2f516fa..dc3b985 100644
--- a/fs_mgr/liblp/builder.cpp
+++ b/fs_mgr/liblp/builder.cpp
@@ -401,8 +401,8 @@
// untouched to be compatible code that looks for an MBR. Thus we
// start counting free sectors at sector 1, not 0.
uint64_t free_area_start = LP_SECTOR_SIZE;
- if (out.alignment || out.alignment_offset) {
- free_area_start = AlignTo(free_area_start, out.alignment, out.alignment_offset);
+ if (out.alignment) {
+ free_area_start = AlignTo(free_area_start, out.alignment);
} else {
free_area_start = AlignTo(free_area_start, logical_block_size);
}
@@ -442,7 +442,7 @@
// Compute the first free sector, factoring in alignment.
uint64_t free_area_start = total_reserved;
if (super.alignment || super.alignment_offset) {
- free_area_start = AlignTo(free_area_start, super.alignment, super.alignment_offset);
+ free_area_start = AlignTo(free_area_start, super.alignment);
} else {
free_area_start = AlignTo(free_area_start, logical_block_size);
}
@@ -930,7 +930,7 @@
// Note: when reading alignment info from the Kernel, we don't assume it
// is aligned to the sector size, so we round up to the nearest sector.
uint64_t lba = sector * LP_SECTOR_SIZE;
- uint64_t aligned = AlignTo(lba, block_device.alignment, block_device.alignment_offset);
+ uint64_t aligned = AlignTo(lba, block_device.alignment);
return AlignTo(aligned, LP_SECTOR_SIZE) / LP_SECTOR_SIZE;
}
diff --git a/fs_mgr/liblp/builder_test.cpp b/fs_mgr/liblp/builder_test.cpp
index 977ebe3..52a3217 100644
--- a/fs_mgr/liblp/builder_test.cpp
+++ b/fs_mgr/liblp/builder_test.cpp
@@ -174,7 +174,7 @@
ASSERT_NE(exported, nullptr);
super_device = GetMetadataSuperBlockDevice(*exported.get());
ASSERT_NE(super_device, nullptr);
- EXPECT_EQ(super_device->first_logical_sector, 1472);
+ EXPECT_EQ(super_device->first_logical_sector, 1536);
// Alignment offset without alignment doesn't mean anything.
device_info.alignment = 0;
@@ -190,7 +190,7 @@
ASSERT_NE(exported, nullptr);
super_device = GetMetadataSuperBlockDevice(*exported.get());
ASSERT_NE(super_device, nullptr);
- EXPECT_EQ(super_device->first_logical_sector, 174);
+ EXPECT_EQ(super_device->first_logical_sector, 168);
// Test a small alignment with no alignment offset.
device_info.alignment = 11 * 1024;
@@ -200,7 +200,7 @@
ASSERT_NE(exported, nullptr);
super_device = GetMetadataSuperBlockDevice(*exported.get());
ASSERT_NE(super_device, nullptr);
- EXPECT_EQ(super_device->first_logical_sector, 160);
+ EXPECT_EQ(super_device->first_logical_sector, 154);
}
TEST_F(BuilderTest, InternalPartitionAlignment) {
@@ -229,12 +229,12 @@
EXPECT_EQ(extent.num_sectors, 80);
uint64_t lba = extent.target_data * LP_SECTOR_SIZE;
- uint64_t aligned_lba = AlignTo(lba, device_info.alignment, device_info.alignment_offset);
+ uint64_t aligned_lba = AlignTo(lba, device_info.alignment);
EXPECT_EQ(lba, aligned_lba);
}
// Sanity check one extent.
- EXPECT_EQ(exported->extents.back().target_data, 3008);
+ EXPECT_EQ(exported->extents.back().target_data, 3072);
}
TEST_F(BuilderTest, UseAllDiskSpace) {
@@ -652,7 +652,7 @@
};
unique_ptr<MetadataBuilder> builder = MetadataBuilder::New(partitions, "system_a", 65536, 2);
ASSERT_NE(builder, nullptr);
- EXPECT_EQ(builder->AllocatableSpace(), 467238912);
+ EXPECT_EQ(builder->AllocatableSpace(), 467402752);
// Create a partition that spans 3 devices.
Partition* p = builder->AddPartition("system_a", 0);
@@ -675,17 +675,17 @@
EXPECT_EQ(metadata->block_devices[2].alignment, 786432);
EXPECT_EQ(metadata->block_devices[2].alignment_offset, 753664);
ASSERT_EQ(metadata->extents.size(), 3);
- EXPECT_EQ(metadata->extents[0].num_sectors, 522304);
+ EXPECT_EQ(metadata->extents[0].num_sectors, 522752);
EXPECT_EQ(metadata->extents[0].target_type, LP_TARGET_TYPE_LINEAR);
- EXPECT_EQ(metadata->extents[0].target_data, 1984);
+ EXPECT_EQ(metadata->extents[0].target_data, 1536);
EXPECT_EQ(metadata->extents[0].target_source, 0);
- EXPECT_EQ(metadata->extents[1].num_sectors, 260672);
+ EXPECT_EQ(metadata->extents[1].num_sectors, 260608);
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_data, 1536);
EXPECT_EQ(metadata->extents[1].target_source, 1);
- EXPECT_EQ(metadata->extents[2].num_sectors, 129088);
+ EXPECT_EQ(metadata->extents[2].num_sectors, 128704);
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_data, 1536);
EXPECT_EQ(metadata->extents[2].target_source, 2);
}
@@ -1019,3 +1019,35 @@
EXPECT_FALSE(extent.OverlapsWith(LinearExtent{20, 1, 15}));
EXPECT_FALSE(extent.OverlapsWith(LinearExtent{20, 1, 10}));
}
+
+TEST_F(BuilderTest, AlignFreeRegion) {
+ BlockDeviceInfo super("super", 8_GiB, 786432, 0, 4096);
+ std::vector<BlockDeviceInfo> block_devices = {super};
+
+ unique_ptr<MetadataBuilder> builder = MetadataBuilder::New(block_devices, "super", 65536, 2);
+ ASSERT_NE(builder, nullptr);
+
+ Partition* p = builder->AddPartition("system", "default", 0);
+ ASSERT_NE(p, nullptr);
+ ASSERT_TRUE(builder->AddLinearExtent(p, "super", 64, (super.alignment + 4096) / 512));
+
+ p = builder->AddPartition("vendor", "default", 0);
+ ASSERT_NE(p, nullptr);
+ ASSERT_TRUE(builder->ResizePartition(p, 2_GiB));
+
+ const auto& extents = p->extents();
+ ASSERT_EQ(extents.size(), 2);
+
+ LinearExtent* e1 = extents[0]->AsLinearExtent();
+ ASSERT_NE(e1, nullptr);
+ LinearExtent* e2 = extents[1]->AsLinearExtent();
+ ASSERT_NE(e2, nullptr);
+
+ // The misaligned partition starting at sector 1544 should not cause any
+ // overlap with previous extents. We should see vendor punch a hole where
+ // "system" is, extending the hole up to the next aligned block.
+ EXPECT_EQ(e1->physical_sector(), 1536);
+ EXPECT_EQ(e1->end_sector(), 1544);
+ EXPECT_EQ(e2->physical_sector(), 3072);
+ EXPECT_EQ(e2->end_sector(), 4197368);
+}
diff --git a/fs_mgr/liblp/device_test.cpp b/fs_mgr/liblp/device_test.cpp
index 99bff6e..6af9d94 100644
--- a/fs_mgr/liblp/device_test.cpp
+++ b/fs_mgr/liblp/device_test.cpp
@@ -50,16 +50,7 @@
// Sanity check that the device doesn't give us some weird inefficient
// alignment.
EXPECT_EQ(device_info.alignment % LP_SECTOR_SIZE, 0);
- EXPECT_EQ(device_info.alignment_offset % LP_SECTOR_SIZE, 0);
- EXPECT_LE(device_info.alignment_offset, INT_MAX);
EXPECT_EQ(device_info.logical_block_size % LP_SECTOR_SIZE, 0);
-
- // Having an alignment offset > alignment doesn't really make sense.
- EXPECT_LT(device_info.alignment_offset, device_info.alignment);
-
- if (IPropertyFetcher::GetInstance()->GetBoolProperty("ro.virtual_ab.enabled", false)) {
- EXPECT_EQ(device_info.alignment_offset, 0);
- }
}
TEST_F(DeviceTest, ReadSuperPartitionCurrentSlot) {
diff --git a/fs_mgr/liblp/utility.h b/fs_mgr/liblp/utility.h
index 0661769..f210eaf 100644
--- a/fs_mgr/liblp/utility.h
+++ b/fs_mgr/liblp/utility.h
@@ -78,17 +78,6 @@
return base + (alignment - remainder);
}
-// Same as the above |AlignTo|, except that |base| is only aligned when added to
-// |alignment_offset|.
-constexpr uint64_t AlignTo(uint64_t base, uint32_t alignment, uint32_t alignment_offset) {
- uint64_t aligned = AlignTo(base, alignment) + alignment_offset;
- if (aligned - alignment >= base) {
- // We overaligned (base < alignment_offset).
- return aligned - alignment;
- }
- return aligned;
-}
-
// Update names from C++ strings.
bool UpdateBlockDevicePartitionName(LpMetadataBlockDevice* device, const std::string& name);
bool UpdatePartitionGroupName(LpMetadataPartitionGroup* group, const std::string& name);
diff --git a/fs_mgr/liblp/utility_test.cpp b/fs_mgr/liblp/utility_test.cpp
index cac3989..b64861d 100644
--- a/fs_mgr/liblp/utility_test.cpp
+++ b/fs_mgr/liblp/utility_test.cpp
@@ -64,9 +64,9 @@
EXPECT_EQ(AlignTo(555, 1024), 1024);
EXPECT_EQ(AlignTo(555, 1000), 1000);
EXPECT_EQ(AlignTo(0, 1024), 0);
- EXPECT_EQ(AlignTo(54, 32, 30), 62);
- EXPECT_EQ(AlignTo(32, 32, 30), 62);
- EXPECT_EQ(AlignTo(17, 32, 30), 30);
+ EXPECT_EQ(AlignTo(54, 32), 64);
+ EXPECT_EQ(AlignTo(32, 32), 32);
+ EXPECT_EQ(AlignTo(17, 32), 32);
}
TEST(liblp, GetPartitionSlotSuffix) {