From 9d344969b03fd19a6f5c485ddd7c8dd22b7d3a9d Mon Sep 17 00:00:00 2001 From: Jinguang Dong Date: Tue, 13 Jun 2017 10:20:34 +0800 Subject: [PATCH] fs_mgr: Adding logs when failing to wait for a device file During mount operations, fs_mgr_wait_for_file() is invoked to ensure the device file exists before starting to mount it. Adding logs when the wait fails and also skip mounting as it won't be successful. Also merge fs_mgr_test_access() and wait_for_file() as fs_mgr_wait_for_file(). Test: Boot device and manually trigger the timeout issue Test: Check and confirm whether timeout log info is inside ksmg. Change-Id: Ide6d7fdca41e03e169e4400f91b7dea327985aaf --- fs_mgr/fs_mgr.cpp | 72 +++++++++++++++------------------------ fs_mgr/fs_mgr_avb.cpp | 2 +- fs_mgr/fs_mgr_avb_ops.cpp | 6 ++-- fs_mgr/fs_mgr_priv.h | 13 ++++--- fs_mgr/fs_mgr_verity.cpp | 2 +- 5 files changed, 40 insertions(+), 55 deletions(-) diff --git a/fs_mgr/fs_mgr.cpp b/fs_mgr/fs_mgr.cpp index 8c19a812c..a13fe0fac 100644 --- a/fs_mgr/fs_mgr.cpp +++ b/fs_mgr/fs_mgr.cpp @@ -32,6 +32,7 @@ #include #include +#include #include #include @@ -87,34 +88,22 @@ enum FsStatFlags { FS_STAT_EXT4_INVALID_MAGIC = 0x0800, }; -/* - * gettime() - returns the time in seconds of the system's monotonic clock or - * zero on error. - */ -static time_t gettime(void) -{ - struct timespec ts; - int ret; +// TODO: switch to inotify() +bool fs_mgr_wait_for_file(const std::string& filename, + const std::chrono::milliseconds relative_timeout) { + auto start_time = std::chrono::steady_clock::now(); - ret = clock_gettime(CLOCK_MONOTONIC, &ts); - if (ret < 0) { - PERROR << "clock_gettime(CLOCK_MONOTONIC) failed"; - return 0; + while (true) { + if (!access(filename.c_str(), F_OK) || errno != ENOENT) { + return true; + } + + std::this_thread::sleep_for(50ms); + + auto now = std::chrono::steady_clock::now(); + auto time_elapsed = std::chrono::duration_cast(now - start_time); + if (time_elapsed > relative_timeout) return false; } - - return ts.tv_sec; -} - -static int wait_for_file(const char *filename, int timeout) -{ - struct stat info; - time_t timeout_time = gettime() + timeout; - int ret = -1; - - while (gettime() < timeout_time && ((ret = stat(filename, &info)) < 0)) - usleep(10000); - - return ret; } static void log_fs_stat(const char* blk_device, int fs_stat) @@ -744,19 +733,6 @@ static int handle_encryptable(const struct fstab_rec* rec) } } -// TODO: add ueventd notifiers if they don't exist. -// This is just doing a wait_for_device for maximum of 1s -int fs_mgr_test_access(const char *device) { - int tries = 25; - while (tries--) { - if (!access(device, F_OK) || errno != ENOENT) { - return 0; - } - usleep(40 * 1000); - } - return -1; -} - bool is_device_secure() { int ret = -1; char value[PROP_VALUE_MAX]; @@ -827,8 +803,10 @@ int fs_mgr_mount_all(struct fstab *fstab, int mount_mode) } } - if (fstab->recs[i].fs_mgr_flags & MF_WAIT) { - wait_for_file(fstab->recs[i].blk_device, WAIT_TIMEOUT); + if (fstab->recs[i].fs_mgr_flags & MF_WAIT && + !fs_mgr_wait_for_file(fstab->recs[i].blk_device, 20s)) { + LERROR << "Skipping '" << fstab->recs[i].blk_device << "' during mount_all"; + continue; } if (fstab->recs[i].fs_mgr_flags & MF_AVB) { @@ -1031,8 +1009,9 @@ int fs_mgr_do_mount(struct fstab *fstab, const char *n_name, char *n_blk_device, } /* First check the filesystem if requested */ - if (fstab->recs[i].fs_mgr_flags & MF_WAIT) { - wait_for_file(n_blk_device, WAIT_TIMEOUT); + if (fstab->recs[i].fs_mgr_flags & MF_WAIT && !fs_mgr_wait_for_file(n_blk_device, 20s)) { + LERROR << "Skipping mounting '" << n_blk_device << "'"; + continue; } int fs_stat = 0; @@ -1205,8 +1184,11 @@ int fs_mgr_swapon_all(struct fstab *fstab) fclose(zram_fp); } - if (fstab->recs[i].fs_mgr_flags & MF_WAIT) { - wait_for_file(fstab->recs[i].blk_device, WAIT_TIMEOUT); + if (fstab->recs[i].fs_mgr_flags & MF_WAIT && + !fs_mgr_wait_for_file(fstab->recs[i].blk_device, 20s)) { + LERROR << "Skipping mkswap for '" << fstab->recs[i].blk_device << "'"; + ret = -1; + continue; } /* Initialize the swap area */ diff --git a/fs_mgr/fs_mgr_avb.cpp b/fs_mgr/fs_mgr_avb.cpp index 2c99aa7c5..dac36b4a1 100644 --- a/fs_mgr/fs_mgr_avb.cpp +++ b/fs_mgr/fs_mgr_avb.cpp @@ -397,7 +397,7 @@ static bool hashtree_dm_verity_setup(struct fstab_rec* fstab_entry, fstab_entry->blk_device = strdup(verity_blk_name.c_str()); // Makes sure we've set everything up properly. - if (wait_for_verity_dev && fs_mgr_test_access(verity_blk_name.c_str()) < 0) { + if (wait_for_verity_dev && !fs_mgr_wait_for_file(verity_blk_name, 1s)) { return false; } diff --git a/fs_mgr/fs_mgr_avb_ops.cpp b/fs_mgr/fs_mgr_avb_ops.cpp index ba1262fde..43879fe56 100644 --- a/fs_mgr/fs_mgr_avb_ops.cpp +++ b/fs_mgr/fs_mgr_avb_ops.cpp @@ -142,10 +142,8 @@ AvbIOResult FsManagerAvbOps::ReadFromPartition(const char* partition, int64_t of } std::string path = iter->second; - // Ensures the device path (a symlink created by init) is ready to - // access. fs_mgr_test_access() will test a few iterations if the - // path doesn't exist yet. - if (fs_mgr_test_access(path.c_str()) < 0) { + // Ensures the device path (a symlink created by init) is ready to access. + if (!fs_mgr_wait_for_file(path, 1s)) { return AVB_IO_RESULT_ERROR_NO_SUCH_PARTITION; } diff --git a/fs_mgr/fs_mgr_priv.h b/fs_mgr/fs_mgr_priv.h index c985462af..81ab9408b 100644 --- a/fs_mgr/fs_mgr_priv.h +++ b/fs_mgr/fs_mgr_priv.h @@ -17,8 +17,12 @@ #ifndef __CORE_FS_MGR_PRIV_H #define __CORE_FS_MGR_PRIV_H +#include +#include + #include -#include + +#include "fs_mgr.h" #include "fs_mgr_priv_boot_config.h" /* The CHECK() in logging.h will use program invocation name as the tag. @@ -43,8 +47,6 @@ #define CRYPTO_TMPFS_OPTIONS "size=256m,mode=0771,uid=1000,gid=1000" -#define WAIT_TIMEOUT 20 - /* fstab has the following format: * * Any line starting with a # is a comment and ignored @@ -110,8 +112,11 @@ #define DM_BUF_SIZE 4096 +using namespace std::chrono_literals; + int fs_mgr_set_blk_ro(const char *blockdev); -int fs_mgr_test_access(const char *device); +bool fs_mgr_wait_for_file(const std::string& filename, + const std::chrono::milliseconds relative_timeout); bool fs_mgr_update_for_slotselect(struct fstab *fstab); bool is_dt_compatible(); bool is_device_secure(); diff --git a/fs_mgr/fs_mgr_verity.cpp b/fs_mgr/fs_mgr_verity.cpp index 8fa93705e..75e5174a8 100644 --- a/fs_mgr/fs_mgr_verity.cpp +++ b/fs_mgr/fs_mgr_verity.cpp @@ -929,7 +929,7 @@ loaded: } // make sure we've set everything up properly - if (wait_for_verity_dev && fs_mgr_test_access(fstab->blk_device) < 0) { + if (wait_for_verity_dev && !fs_mgr_wait_for_file(fstab->blk_device, 1s)) { goto out; }