From dfca1f129eee397bd5641f35559337d4afbe5680 Mon Sep 17 00:00:00 2001 From: Kelvin Zhang Date: Thu, 5 Sep 2024 22:56:36 +0000 Subject: [PATCH] Reland Skip F2FS formatting for dev option enabled devices F2FS does not support page_size!=block_size configuration, and dev option devices need to toggle between 4K/16K mode, hence F2FS requires a data wipe every time page size changes. This is inconveinent, skip F2FS formatting instead. The original CL had a bug where `iter->blk_device` is accessed before checking if `iter` is out of bounds. Fixed by flipping order of two conditions in an && operator. Test: th Bug: 353436188 This reverts commit 642a98e2a77d3398269d588ca16e99dcdaeb68d5. Change-Id: I89551f219efc44bad29a190edff5412a5cc67840 --- fs_mgr/fs_mgr.cpp | 67 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 49 insertions(+), 18 deletions(-) diff --git a/fs_mgr/fs_mgr.cpp b/fs_mgr/fs_mgr.cpp index 76578dd2c..fbd990b96 100644 --- a/fs_mgr/fs_mgr.cpp +++ b/fs_mgr/fs_mgr.cpp @@ -38,10 +38,8 @@ #include #include -#include #include #include -#include #include #include #include @@ -66,6 +64,7 @@ #include #include #include +#include #include #include #include @@ -82,7 +81,7 @@ #define F2FS_FSCK_BIN "/system/bin/fsck.f2fs" #define MKSWAP_BIN "/system/bin/mkswap" #define TUNE2FS_BIN "/system/bin/tune2fs" -#define RESIZE2FS_BIN "/system/bin/resize2fs" +#define RESIZE2FS_BIN "/system/bin/resize2fs" #define FSCK_LOG_FILE "/dev/fscklogs/log" @@ -138,8 +137,8 @@ enum FsStatFlags { static void log_fs_stat(const std::string& blk_device, int fs_stat) { std::string msg = android::base::StringPrintf("\nfs_stat,%s,0x%x\n", blk_device.c_str(), fs_stat); - android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(FSCK_LOG_FILE, O_WRONLY | O_CLOEXEC | - O_APPEND | O_CREAT, 0664))); + android::base::unique_fd fd(TEMP_FAILURE_RETRY( + open(FSCK_LOG_FILE, O_WRONLY | O_CLOEXEC | O_APPEND | O_CREAT, 0664))); if (fd == -1 || !android::base::WriteStringToFd(msg, fd)) { LWARNING << __FUNCTION__ << "() cannot log " << msg; } @@ -593,7 +592,7 @@ static void tune_metadata_csum(const std::string& blk_device, const FstabEntry& // Must give `-T now` to prevent last_fsck_time from growing too large, // otherwise, tune2fs won't enable metadata_csum. - const char* tune2fs_args[] = {TUNE2FS_BIN, "-O", "metadata_csum,64bit,extent", + const char* tune2fs_args[] = {TUNE2FS_BIN, "-O", "metadata_csum,64bit,extent", "-T", "now", blk_device.c_str()}; const char* resize2fs_args[] = {RESIZE2FS_BIN, "-b", blk_device.c_str()}; @@ -1430,6 +1429,37 @@ bool WasMetadataEncryptionInterrupted(const FstabEntry& entry) { return access(fs_mgr_metadata_encryption_in_progress_file_name(entry).c_str(), R_OK) == 0; } +static FstabEntry* LocateFormattableEntry(FstabEntry* const begin, FstabEntry* const end) { + if (begin == end) { + return nullptr; + } + const bool dev_option_enabled = + android::base::GetBoolProperty("ro.product.build.16k_page.enabled", false); + FstabEntry* f2fs_entry = nullptr; + for (auto iter = begin; iter != end && iter->blk_device == begin->blk_device; iter++) { + if (iter->fs_mgr_flags.formattable) { + if (getpagesize() != 4096 && is_f2fs(iter->fs_type) && dev_option_enabled) { + f2fs_entry = iter; + continue; + } + if (f2fs_entry) { + LOG(INFO) << "Skipping F2FS format for block device " << iter->blk_device << " @ " + << iter->mount_point + << " in non-4K mode for dev option enabled devices, " + "as these devices need to toggle between 4K/16K mode, and F2FS does " + "not support page_size != block_size configuration."; + } + return iter; + } + } + if (f2fs_entry) { + LOG(INFO) << "Using F2FS for " << f2fs_entry->blk_device << " @ " << f2fs_entry->mount_point + << " even though we are in non-4K mode. Device might require a data wipe after " + "going back to 4K mode, as F2FS does not support page_size != block_size"; + } + return f2fs_entry; +} + // When multiple fstab records share the same mount_point, it will try to mount each // one in turn, and ignore any duplicates after a first successful mount. // Returns -1 on error, and FS_MGR_MNTALL_* otherwise. @@ -1540,8 +1570,8 @@ int fs_mgr_mount_all(Fstab* fstab, int mount_mode) { } } - int last_idx_inspected; - int top_idx = i; + int last_idx_inspected = -1; + const int top_idx = i; int attempted_idx = -1; bool encryption_interrupted = WasMetadataEncryptionInterrupted(current_entry); @@ -1591,7 +1621,8 @@ int fs_mgr_mount_all(Fstab* fstab, int mount_mode) { // Success! Go get the next one. continue; } - + auto formattable_entry = + LocateFormattableEntry(fstab->data() + top_idx, fstab->data() + fstab->size()); // Mounting failed, understand why and retry. wiped = partition_wiped(current_entry.blk_device.c_str()); if (mount_errno != EBUSY && mount_errno != EACCES && @@ -1619,12 +1650,12 @@ int fs_mgr_mount_all(Fstab* fstab, int mount_mode) { encryptable = FS_MGR_MNTALL_DEV_IS_METADATA_ENCRYPTED; set_type_property(encryptable); - if (!call_vdc({"cryptfs", "encryptFstab", current_entry.blk_device, - current_entry.mount_point, "true" /* shouldFormat */, - current_entry.fs_type, - current_entry.fs_mgr_flags.is_zoned ? "true" : "false", - std::to_string(current_entry.length), - android::base::Join(current_entry.user_devices, ' ')}, + if (!call_vdc({"cryptfs", "encryptFstab", formattable_entry->blk_device, + formattable_entry->mount_point, "true" /* shouldFormat */, + formattable_entry->fs_type, + formattable_entry->fs_mgr_flags.is_zoned ? "true" : "false", + std::to_string(formattable_entry->length), + android::base::Join(formattable_entry->user_devices, ' ')}, nullptr)) { LERROR << "Encryption failed"; } else { @@ -1633,7 +1664,7 @@ int fs_mgr_mount_all(Fstab* fstab, int mount_mode) { } } - if (fs_mgr_do_format(current_entry) == 0) { + if (fs_mgr_do_format(*formattable_entry) == 0) { // Let's replay the mount actions. i = top_idx - 1; continue; @@ -1749,12 +1780,12 @@ int fs_mgr_do_mount_one(const FstabEntry& entry, const std::string& alt_mount_po int ret = prepare_fs_for_mount(entry.blk_device, entry, mount_point); // Wiped case doesn't require to try __mount below. if (ret & FS_STAT_INVALID_MAGIC) { - return FS_MGR_DOMNT_FAILED; + return FS_MGR_DOMNT_FAILED; } ret = __mount(entry.blk_device, mount_point, entry); if (ret) { - ret = (errno == EBUSY) ? FS_MGR_DOMNT_BUSY : FS_MGR_DOMNT_FAILED; + ret = (errno == EBUSY) ? FS_MGR_DOMNT_BUSY : FS_MGR_DOMNT_FAILED; } return ret;