diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h index 08a79baed..d10286312 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h @@ -826,6 +826,9 @@ class SnapshotManager final : public ISnapshotManager { bool DeleteDeviceIfExists(const std::string& name, const std::chrono::milliseconds& timeout_ms = {}); + // Set read-ahead size during OTA + void SetReadAheadSize(const std::string& entry_block_device, off64_t size_kb); + android::dm::IDeviceMapper& dm_; std::unique_ptr device_; std::string metadata_dir_; diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index 79cebab78..f6a35a8f5 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -82,12 +83,28 @@ using RepeatedPtrField = google::protobuf::RepeatedPtrField; using std::chrono::duration_cast; using namespace std::chrono_literals; using namespace std::string_literals; +using android::base::Realpath; +using android::base::StringPrintf; static constexpr char kBootSnapshotsWithoutSlotSwitch[] = "/metadata/ota/snapshot-boot-without-slot-switch"; static constexpr char kBootIndicatorPath[] = "/metadata/ota/snapshot-boot"; static constexpr char kRollbackIndicatorPath[] = "/metadata/ota/rollback-indicator"; static constexpr auto kUpdateStateCheckInterval = 2s; +/* + * The readahead size is set to 32kb so that + * there is no significant memory pressure (/proc/pressure/memory) during boot. + * After OTA, during boot, partitions are scanned before marking slot as successful. + * This scan will trigger readahead both on source and COW block device thereby + * leading to Inactive(file) pages to be very high. + * + * A lower value may help reduce memory pressure further, however, that will + * increase the boot time. Thus, for device which don't care about OTA boot + * time, they could use O_DIRECT functionality wherein the I/O to the source + * block device will be O_DIRECT. + */ +static constexpr auto kCowReadAheadSizeKb = 32; +static constexpr auto kSourceReadAheadSizeKb = 32; // Note: IImageManager is an incomplete type in the header, so the default // destructor doesn't work. @@ -1748,6 +1765,9 @@ bool SnapshotManager::PerformInitTransition(InitTransition transition, snapuserd_argv->emplace_back(std::move(message)); } + SetReadAheadSize(cow_image_device, kCowReadAheadSizeKb); + SetReadAheadSize(source_device, kSourceReadAheadSizeKb); + // Do not attempt to connect to the new snapuserd yet, it hasn't // been started. We do however want to wait for the misc device // to have been created. @@ -4407,5 +4427,31 @@ bool SnapshotManager::PrepareDeviceToBootWithoutSnapshot() { return true; } +void SnapshotManager::SetReadAheadSize(const std::string& entry_block_device, off64_t size_kb) { + std::string block_device; + if (!Realpath(entry_block_device, &block_device)) { + PLOG(ERROR) << "Failed to realpath " << entry_block_device; + return; + } + + static constexpr std::string_view kDevBlockPrefix("/dev/block/"); + if (!android::base::StartsWith(block_device, kDevBlockPrefix)) { + LOG(ERROR) << block_device << " is not a block device"; + return; + } + + std::string block_name = block_device.substr(kDevBlockPrefix.length()); + std::string sys_partition = + android::base::StringPrintf("/sys/class/block/%s/partition", block_name.c_str()); + struct stat info; + if (lstat(sys_partition.c_str(), &info) == 0) { + block_name += "/.."; + } + std::string sys_ra = android::base::StringPrintf("/sys/class/block/%s/queue/read_ahead_kb", + block_name.c_str()); + std::string size = std::to_string(size_kb); + android::base::WriteStringToFile(size, sys_ra.c_str()); +} + } // namespace snapshot } // namespace android diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/extractor.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/extractor.cpp index c5718d5b7..c85331b34 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/extractor.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/extractor.cpp @@ -41,7 +41,7 @@ Extractor::Extractor(const std::string& base_path, const std::string& cow_path) bool Extractor::Init() { auto opener = factory_.CreateTestOpener(control_name_); handler_ = std::make_shared(control_name_, cow_path_, base_path_, base_path_, - opener, 1, false, false); + opener, 1, false, false, false); if (!handler_->InitCowDevice()) { return false; } @@ -50,7 +50,7 @@ bool Extractor::Init() { } read_worker_ = std::make_unique(cow_path_, base_path_, control_name_, base_path_, - handler_->GetSharedPtr(), opener); + handler_->GetSharedPtr(), opener, false); if (!read_worker_->Init()) { return false; } diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/handler_manager.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/handler_manager.cpp index ffd7a4b8b..711e70473 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/handler_manager.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/handler_manager.cpp @@ -51,10 +51,11 @@ SnapshotHandlerManager::SnapshotHandlerManager() { std::shared_ptr SnapshotHandlerManager::AddHandler( const std::string& misc_name, const std::string& cow_device_path, const std::string& backing_device, const std::string& base_path_merge, - std::shared_ptr opener, int num_worker_threads, bool use_iouring) { - auto snapuserd = std::make_shared(misc_name, cow_device_path, backing_device, - base_path_merge, opener, num_worker_threads, - use_iouring, perform_verification_); + std::shared_ptr opener, int num_worker_threads, bool use_iouring, + bool o_direct) { + auto snapuserd = std::make_shared( + misc_name, cow_device_path, backing_device, base_path_merge, opener, num_worker_threads, + use_iouring, perform_verification_, o_direct); if (!snapuserd->InitCowDevice()) { LOG(ERROR) << "Failed to initialize Snapuserd"; return nullptr; diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/handler_manager.h b/fs_mgr/libsnapshot/snapuserd/user-space-merge/handler_manager.h index ff6ee8f10..f23f07eb5 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/handler_manager.h +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/handler_manager.h @@ -57,7 +57,8 @@ class ISnapshotHandlerManager { const std::string& backing_device, const std::string& base_path_merge, std::shared_ptr opener, - int num_worker_threads, bool use_iouring) = 0; + int num_worker_threads, bool use_iouring, + bool o_direct) = 0; // Start serving requests on a snapshot handler. virtual bool StartHandler(const std::string& misc_name) = 0; @@ -96,7 +97,8 @@ class SnapshotHandlerManager final : public ISnapshotHandlerManager { const std::string& backing_device, const std::string& base_path_merge, std::shared_ptr opener, - int num_worker_threads, bool use_iouring) override; + int num_worker_threads, bool use_iouring, + bool o_direct) override; bool StartHandler(const std::string& misc_name) override; bool DeleteHandler(const std::string& misc_name) override; bool InitiateMerge(const std::string& misc_name) override; diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.cpp index 3a5666931..bcf9aabe9 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.cpp @@ -557,7 +557,7 @@ bool MergeWorker::Run() { return true; } - if (!SetThreadPriority(kNiceValueForMergeThreads)) { + if (!SetThreadPriority(ANDROID_PRIORITY_BACKGROUND)) { SNAP_PLOG(ERROR) << "Failed to set thread priority"; } diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/read_worker.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/read_worker.cpp index 431baf060..f1d406534 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/read_worker.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/read_worker.cpp @@ -31,16 +31,19 @@ using android::base::unique_fd; void ReadWorker::CloseFds() { block_server_ = {}; backing_store_fd_ = {}; + backing_store_direct_fd_ = {}; Worker::CloseFds(); } ReadWorker::ReadWorker(const std::string& cow_device, const std::string& backing_device, const std::string& misc_name, const std::string& base_path_merge, std::shared_ptr snapuserd, - std::shared_ptr opener) + std::shared_ptr opener, bool direct_read) : Worker(cow_device, misc_name, base_path_merge, snapuserd), backing_store_device_(backing_device), - block_server_opener_(opener) {} + direct_read_(direct_read), + block_server_opener_(opener), + aligned_buffer_(std::unique_ptr(nullptr, &::free)) {} // Start the replace operation. This will read the // internal COW format and if the block is compressed, @@ -61,6 +64,17 @@ bool ReadWorker::ReadFromSourceDevice(const CowOperation* cow_op, void* buffer) } SNAP_LOG(DEBUG) << " ReadFromBaseDevice...: new-block: " << cow_op->new_block << " Op: " << *cow_op; + + if (direct_read_ && IsBlockAligned(offset)) { + if (!android::base::ReadFullyAtOffset(backing_store_direct_fd_, aligned_buffer_.get(), + BLOCK_SZ, offset)) { + SNAP_PLOG(ERROR) << "O_DIRECT Read failed at offset: " << offset; + return false; + } + std::memcpy(buffer, aligned_buffer_.get(), BLOCK_SZ); + return true; + } + if (!android::base::ReadFullyAtOffset(backing_store_fd_, buffer, BLOCK_SZ, offset)) { std::string op; if (cow_op->type() == kCowCopyOp) @@ -201,6 +215,24 @@ bool ReadWorker::Init() { return false; } + if (direct_read_) { + backing_store_direct_fd_.reset(open(backing_store_device_.c_str(), O_RDONLY | O_DIRECT)); + if (backing_store_direct_fd_ < 0) { + SNAP_PLOG(ERROR) << "Open Failed with O_DIRECT: " << backing_store_direct_fd_; + direct_read_ = false; + } else { + void* aligned_addr; + ssize_t page_size = getpagesize(); + if (posix_memalign(&aligned_addr, page_size, page_size) < 0) { + direct_read_ = false; + SNAP_PLOG(ERROR) << "posix_memalign failed " + << " page_size: " << page_size << " read_sz: " << page_size; + } else { + aligned_buffer_.reset(aligned_addr); + } + } + } + block_server_ = block_server_opener_->Open(this, PAYLOAD_BUFFER_SZ); if (!block_server_) { SNAP_PLOG(ERROR) << "Unable to open block server"; @@ -214,7 +246,7 @@ bool ReadWorker::Run() { pthread_setname_np(pthread_self(), "ReadWorker"); - if (!SetThreadPriority(kNiceValueForMergeThreads)) { + if (!SetThreadPriority(ANDROID_PRIORITY_NORMAL)) { SNAP_PLOG(ERROR) << "Failed to set thread priority"; } diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/read_worker.h b/fs_mgr/libsnapshot/snapuserd/user-space-merge/read_worker.h index 6dbae812f..1aff50c08 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/read_worker.h +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/read_worker.h @@ -28,7 +28,7 @@ class ReadWorker : public Worker, public IBlockServer::Delegate { ReadWorker(const std::string& cow_device, const std::string& backing_device, const std::string& misc_name, const std::string& base_path_merge, std::shared_ptr snapuserd, - std::shared_ptr opener); + std::shared_ptr opener, bool direct_read = false); bool Run(); bool Init() override; @@ -59,11 +59,14 @@ class ReadWorker : public Worker, public IBlockServer::Delegate { std::string backing_store_device_; unique_fd backing_store_fd_; + unique_fd backing_store_direct_fd_; + bool direct_read_ = false; std::shared_ptr block_server_opener_; std::unique_ptr block_server_; std::basic_string xor_buffer_; + std::unique_ptr aligned_buffer_; }; } // namespace snapshot diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.cpp index 9b8c70d32..05ba047b5 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.cpp @@ -36,7 +36,7 @@ using android::base::unique_fd; SnapshotHandler::SnapshotHandler(std::string misc_name, std::string cow_device, std::string backing_device, std::string base_path_merge, std::shared_ptr opener, int num_worker_threads, - bool use_iouring, bool perform_verification) { + bool use_iouring, bool perform_verification, bool o_direct) { misc_name_ = std::move(misc_name); cow_device_ = std::move(cow_device); backing_store_device_ = std::move(backing_device); @@ -45,13 +45,14 @@ SnapshotHandler::SnapshotHandler(std::string misc_name, std::string cow_device, num_worker_threads_ = num_worker_threads; is_io_uring_enabled_ = use_iouring; perform_verification_ = perform_verification; + o_direct_ = o_direct; } bool SnapshotHandler::InitializeWorkers() { for (int i = 0; i < num_worker_threads_; i++) { auto wt = std::make_unique(cow_device_, backing_store_device_, misc_name_, base_path_merge_, GetSharedPtr(), - block_server_opener_); + block_server_opener_, o_direct_); if (!wt->Init()) { SNAP_LOG(ERROR) << "Thread initialization failed"; return false; diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.h b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.h index fa1e7a0ab..9b7238a5d 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.h +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.h @@ -47,6 +47,7 @@ #include #include #include +#include #include "snapuserd_readahead.h" #include "snapuserd_verify.h" @@ -62,8 +63,6 @@ static_assert(PAYLOAD_BUFFER_SZ >= BLOCK_SZ); static constexpr int kNumWorkerThreads = 4; -static constexpr int kNiceValueForMergeThreads = -5; - #define SNAP_LOG(level) LOG(level) << misc_name_ << ": " #define SNAP_PLOG(level) PLOG(level) << misc_name_ << ": " @@ -105,7 +104,7 @@ class SnapshotHandler : public std::enable_shared_from_this { public: SnapshotHandler(std::string misc_name, std::string cow_device, std::string backing_device, std::string base_path_merge, std::shared_ptr opener, - int num_workers, bool use_iouring, bool perform_verification); + int num_workers, bool use_iouring, bool perform_verification, bool o_direct); bool InitCowDevice(); bool Start(); @@ -247,6 +246,7 @@ class SnapshotHandler : public std::enable_shared_from_this { bool perform_verification_ = true; bool resume_merge_ = false; bool merge_complete_ = false; + bool o_direct_ = false; std::unique_ptr update_verify_; std::shared_ptr block_server_opener_; diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_readahead.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_readahead.cpp index 034cda163..c08c1b196 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_readahead.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_readahead.cpp @@ -778,7 +778,7 @@ bool ReadAhead::RunThread() { InitializeIouring(); - if (!SetThreadPriority(kNiceValueForMergeThreads)) { + if (!SetThreadPriority(ANDROID_PRIORITY_BACKGROUND)) { SNAP_PLOG(ERROR) << "Failed to set thread priority"; } diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_server.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_server.cpp index 6eee3571b..0b881b683 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_server.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_server.cpp @@ -346,7 +346,8 @@ void UserSnapshotServer::Interrupt() { std::shared_ptr UserSnapshotServer::AddHandler(const std::string& misc_name, const std::string& cow_device_path, const std::string& backing_device, - const std::string& base_path_merge) { + const std::string& base_path_merge, + const bool o_direct) { // We will need multiple worker threads only during // device boot after OTA. For all other purposes, // one thread is sufficient. We don't want to consume @@ -368,7 +369,7 @@ std::shared_ptr UserSnapshotServer::AddHandler(const std::string& auto opener = block_server_factory_->CreateOpener(misc_name); return handlers_->AddHandler(misc_name, cow_device_path, backing_device, base_path_merge, - opener, num_worker_threads, io_uring_enabled_); + opener, num_worker_threads, io_uring_enabled_, o_direct); } bool UserSnapshotServer::WaitForSocket() { diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_server.h b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_server.h index 99260713b..3013c47fc 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_server.h +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_server.h @@ -86,7 +86,8 @@ class UserSnapshotServer { std::shared_ptr AddHandler(const std::string& misc_name, const std::string& cow_device_path, const std::string& backing_device, - const std::string& base_path_merge); + const std::string& base_path_merge, + bool o_direct = false); bool StartHandler(const std::string& misc_name); void SetTerminating() { terminating_ = true; } diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_test.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_test.cpp index 73c3cbfbb..8ddb0f423 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_test.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_test.cpp @@ -59,10 +59,16 @@ using namespace std; using testing::AssertionFailure; using testing::AssertionResult; using testing::AssertionSuccess; +using ::testing::TestWithParam; -class SnapuserdTestBase : public ::testing::TestWithParam { +struct TestParam { + bool io_uring; + bool o_direct; +}; + +class SnapuserdTestBase : public ::testing::TestWithParam { protected: - void SetUp() override; + virtual void SetUp() override; void TearDown() override; void CreateBaseDevice(); void CreateCowDevice(); @@ -628,9 +634,10 @@ void SnapuserdTest::InitCowDevice() { auto factory = harness_->GetBlockServerFactory(); auto opener = factory->CreateOpener(system_device_ctrl_name_); handlers_->DisableVerification(); - auto handler = - handlers_->AddHandler(system_device_ctrl_name_, cow_system_->path, base_dev_->GetPath(), - base_dev_->GetPath(), opener, 1, GetParam()); + const TestParam params = GetParam(); + auto handler = handlers_->AddHandler(system_device_ctrl_name_, cow_system_->path, + base_dev_->GetPath(), base_dev_->GetPath(), opener, 1, + params.io_uring, params.o_direct); ASSERT_NE(handler, nullptr); ASSERT_NE(handler->snapuserd(), nullptr); #ifdef __ANDROID__ @@ -898,9 +905,10 @@ void HandlerTest::SetUp() { opener_ = factory_.CreateTestOpener(system_device_ctrl_name_); ASSERT_NE(opener_, nullptr); + const TestParam params = GetParam(); handler_ = std::make_shared(system_device_ctrl_name_, cow_system_->path, base_dev_->GetPath(), base_dev_->GetPath(), - opener_, 1, false, false); + opener_, 1, false, false, params.o_direct); ASSERT_TRUE(handler_->InitCowDevice()); ASSERT_TRUE(handler_->InitializeWorkers()); @@ -990,14 +998,28 @@ std::vector GetIoUringConfigs() { return {false, true}; } -std::string IoUringConfigName(const testing::TestParamInfo& info) { - return info.param ? "io_uring" : "sync"; +std::vector GetTestConfigs() { + std::vector testParams; + std::vector uring_configs = GetIoUringConfigs(); + + for (bool config : uring_configs) { + TestParam param; + param.io_uring = config; + param.o_direct = false; + testParams.push_back(std::move(param)); + } + + for (bool config : uring_configs) { + TestParam param; + param.io_uring = config; + param.o_direct = true; + testParams.push_back(std::move(param)); + } + return testParams; } -INSTANTIATE_TEST_SUITE_P(Io, SnapuserdTest, ::testing::ValuesIn(GetIoUringConfigs()), - IoUringConfigName); -INSTANTIATE_TEST_SUITE_P(Io, HandlerTest, ::testing::ValuesIn(GetIoUringConfigs()), - IoUringConfigName); +INSTANTIATE_TEST_SUITE_P(Io, SnapuserdTest, ::testing::ValuesIn(GetTestConfigs())); +INSTANTIATE_TEST_SUITE_P(Io, HandlerTest, ::testing::ValuesIn(GetTestConfigs())); } // namespace snapshot } // namespace android diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_verify.h b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_verify.h index d07d2f836..7c9908515 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_verify.h +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_verify.h @@ -48,10 +48,27 @@ class UpdateVerify { std::mutex m_lock_; std::condition_variable m_cv_; + /* + * Scanning of partitions is an expensive operation both in terms of memory + * and CPU usage. The goal here is to scan the partitions fast enough without + * significant increase in the boot time. + * + * Partitions such as system, product which may be huge and may need multiple + * threads to speed up the verification process. Using multiple threads for + * all partitions may increase CPU usage significantly. Hence, limit that to + * 1 thread per partition. + * + * These numbers were derived by monitoring the memory and CPU pressure + * (/proc/pressure/{cpu,memory}; and monitoring the Inactive(file) and + * Active(file) pages from /proc/meminfo. + * + * Additionally, for low memory devices, it is advisible to use O_DIRECT + * fucntionality for source block device. + */ int kMinThreadsToVerify = 1; - int kMaxThreadsToVerify = 4; - uint64_t kThresholdSize = 512_MiB; - uint64_t kBlockSizeVerify = 1_MiB; + int kMaxThreadsToVerify = 3; + uint64_t kThresholdSize = 750_MiB; + uint64_t kBlockSizeVerify = 2_MiB; bool IsBlockAligned(uint64_t read_size) { return ((read_size & (BLOCK_SZ - 1)) == 0); } void UpdatePartitionVerificationState(UpdateVerifyState state);