From de2ec0b4277e18daa00ecbd94125064b8b0b781f Mon Sep 17 00:00:00 2001 From: David Anderson Date: Mon, 1 Nov 2021 12:58:07 -0700 Subject: [PATCH] libfiemap: Only call FS_IOC_FIEMAP once. The kernel can return different compatible ranges on each call, depending on how it decides to merge contiguous extents in the results. To avoid the complexity of requerying the ioctl, just do one query up to the maximum allowed extent size. Bug: 204536075 Test: install DSU on cuttlefish fiemap_test Change-Id: I4d569e3e6feed14c91a5f500296623888060dcad --- fs_mgr/libfiemap/fiemap_writer.cpp | 52 ++++-------------------------- 1 file changed, 7 insertions(+), 45 deletions(-) diff --git a/fs_mgr/libfiemap/fiemap_writer.cpp b/fs_mgr/libfiemap/fiemap_writer.cpp index 8acb88536..71dadd001 100644 --- a/fs_mgr/libfiemap/fiemap_writer.cpp +++ b/fs_mgr/libfiemap/fiemap_writer.cpp @@ -498,24 +498,6 @@ bool FiemapWriter::HasPinnedExtents(const std::string& file_path) { return IsFilePinned(fd, file_path, sfs.f_type); } -static bool CountFiemapExtents(int file_fd, const std::string& file_path, uint32_t* num_extents) { - struct fiemap fiemap = {}; - fiemap.fm_start = 0; - fiemap.fm_length = UINT64_MAX; - fiemap.fm_flags = FIEMAP_FLAG_SYNC; - fiemap.fm_extent_count = 0; - - if (ioctl(file_fd, FS_IOC_FIEMAP, &fiemap)) { - PLOG(ERROR) << "Failed to get FIEMAP from the kernel for file: " << file_path; - return false; - } - - if (num_extents) { - *num_extents = fiemap.fm_mapped_extents; - } - return true; -} - static bool IsValidExtent(const fiemap_extent* extent, std::string_view file_path) { if (extent->fe_flags & kUnsupportedExtentFlags) { LOG(ERROR) << "Extent at location " << extent->fe_logical << " of file " << file_path @@ -530,12 +512,12 @@ static bool IsLastExtent(const fiemap_extent* extent) { } static bool FiemapToExtents(struct fiemap* fiemap, std::vector* extents, - uint32_t num_extents, std::string_view file_path) { - if (num_extents == 0) return false; - + std::string_view file_path) { + uint32_t num_extents = fiemap->fm_mapped_extents; const struct fiemap_extent* last_extent = &fiemap->fm_extents[num_extents - 1]; if (!IsLastExtent(last_extent)) { - LOG(ERROR) << "FIEMAP did not return a final extent for file: " << file_path; + LOG(ERROR) << "FIEMAP did not return a final extent for file: " << file_path + << " num_extents=" << num_extents << " max_extents=" << kMaxExtents; return false; } @@ -577,21 +559,7 @@ static bool FiemapToExtents(struct fiemap* fiemap, std::vector* extents) { - uint32_t num_extents; - if (!CountFiemapExtents(file_fd, file_path, &num_extents)) { - return false; - } - if (num_extents == 0) { - LOG(ERROR) << "File " << file_path << " has zero extents"; - return false; - } - if (num_extents > kMaxExtents) { - LOG(ERROR) << "File has " << num_extents << ", maximum is " << kMaxExtents << ": " - << file_path; - return false; - } - - uint64_t fiemap_size = sizeof(struct fiemap) + num_extents * sizeof(struct fiemap_extent); + uint64_t fiemap_size = sizeof(struct fiemap) + kMaxExtents * sizeof(struct fiemap_extent); auto buffer = std::unique_ptr(calloc(1, fiemap_size), free); if (buffer == nullptr) { LOG(ERROR) << "Failed to allocate memory for fiemap"; @@ -603,19 +571,13 @@ static bool ReadFiemap(int file_fd, const std::string& file_path, fiemap->fm_length = UINT64_MAX; // make sure file is synced to disk before we read the fiemap fiemap->fm_flags = FIEMAP_FLAG_SYNC; - fiemap->fm_extent_count = num_extents; + fiemap->fm_extent_count = kMaxExtents; if (ioctl(file_fd, FS_IOC_FIEMAP, fiemap)) { PLOG(ERROR) << "Failed to get FIEMAP from the kernel for file: " << file_path; return false; } - if (fiemap->fm_mapped_extents != num_extents) { - LOG(ERROR) << "FIEMAP returned unexpected extent count (" << num_extents - << " expected, got " << fiemap->fm_mapped_extents << ") for file: " << file_path; - return false; - } - - return FiemapToExtents(fiemap, extents, num_extents, file_path); + return FiemapToExtents(fiemap, extents, file_path); } static bool ReadFibmap(int file_fd, const std::string& file_path,