Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 171681ca authored by David Anderson's avatar David Anderson
Browse files

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
parent 9c66e53a

fs_mgr/liblp/builder.cpp

+4 −4
Original line number Diff line number Diff line
@@ -401,8 +401,8 @@ bool MetadataBuilder::Init(const std::vector<BlockDeviceInfo>& block_devices, 
        // 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 @@ bool MetadataBuilder::Init(const std::vector<BlockDeviceInfo>& block_devices, 
    // 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 @@ uint64_t MetadataBuilder::AlignSector(const LpMetadataBlockDevice& block_device, 
    // 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;

}

fs_mgr/liblp/builder_test.cpp

+44 −12
Original line number Diff line number Diff line
@@ -174,7 +174,7 @@ TEST_F(BuilderTest, InternalAlignment) { 
    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 @@ TEST_F(BuilderTest, InternalAlignment) { 
    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 @@ TEST_F(BuilderTest, InternalAlignment) { 
    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 @@ TEST_F(BuilderTest, InternalPartitionAlignment) { 
        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 @@ TEST_F(BuilderTest, MultipleBlockDevices) { 
    };

    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 @@ TEST_F(BuilderTest, MultipleBlockDevices) { 
    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 @@ TEST_F(BuilderTest, LinearExtentOverlap) { 
    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);

}

fs_mgr/liblp/device_test.cpp

+0 −9
Original line number Diff line number Diff line
@@ -50,16 +50,7 @@ TEST_F(DeviceTest, BlockDeviceInfo) { 
    // 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) {

fs_mgr/liblp/utility.h

+0 −11
Original line number Diff line number Diff line
@@ -78,17 +78,6 @@ constexpr uint64_t AlignTo(uint64_t base, uint32_t alignment) { 
    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);

fs_mgr/liblp/utility_test.cpp

+3 −3
Original line number Diff line number Diff line
@@ -64,9 +64,9 @@ TEST(liblp, AlignTo) { 
    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) {