From 52f1c19a1754cf9b83395d42de1c37ebc2eadd14 Mon Sep 17 00:00:00 2001 From: Akilesh Kailash Date: Thu, 16 Nov 2023 16:15:15 -0800 Subject: [PATCH] Allow direct reads on source device Allow O_DIRECT reads on source block device. This will further cut down the Active and Inactive file pages during partition verification. On Pixel 6 after incremental OTA - Post OTA reboot: Without patch With patch Delta -------------------------------------------------------- Inactive(File): 4992MB 3887MB ~22% Active(File): 1465MB 1014MB ~30% Boot time however increases from 25 to 30 seconds. This is not yet enabled. This will be behind a sysprop flag or for low memory devices and will be enabled later. Additionally, set the priority of worker threads to normal. Merge threads priority is reduced. This will help low memory devices as tested on Pixel watch. Bug: 311233916 Test: OTA on Pixel 6 Change-Id: Icacdef08d68e28d3062611477703e7cf393a9f10 Signed-off-by: Akilesh Kailash --- .../snapuserd/user-space-merge/extractor.cpp | 4 +- .../user-space-merge/handler_manager.cpp | 9 ++-- .../user-space-merge/handler_manager.h | 6 ++- .../user-space-merge/merge_worker.cpp | 2 +- .../user-space-merge/read_worker.cpp | 38 +++++++++++++-- .../snapuserd/user-space-merge/read_worker.h | 5 +- .../user-space-merge/snapuserd_core.cpp | 5 +- .../user-space-merge/snapuserd_core.h | 6 +-- .../user-space-merge/snapuserd_readahead.cpp | 2 +- .../user-space-merge/snapuserd_server.cpp | 5 +- .../user-space-merge/snapuserd_server.h | 3 +- .../user-space-merge/snapuserd_test.cpp | 46 ++++++++++++++----- 12 files changed, 97 insertions(+), 34 deletions(-) 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