From f1649b2da540f8e942689bc5d5b8f03902956a25 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 6 Aug 2021 10:19:53 -0700 Subject: Rename two local variables Improve code readability by renaming 'device' into 'loop_device' and 'device_fd' into 'loop_fd'. Bug: 194894000 Test: Built Android images and installed these on an Android device. Merged-In: Ia9c2d7a525e727f8706e66631b97fc4678c6a4d9 Change-Id: I3fa0c9ca53277b621bb5b81aca394a3079c6e0a3 Ignore-AOSP-First: Already in AOSP. Signed-off-by: Bart Van Assche (cherry picked from commit c554240e647c14a5067537fcf8ef88e92b431e12) --- fs_mgr/fs_mgr.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/fs_mgr/fs_mgr.cpp b/fs_mgr/fs_mgr.cpp index af71fe6d5..6b8e39eec 100644 --- a/fs_mgr/fs_mgr.cpp +++ b/fs_mgr/fs_mgr.cpp @@ -2065,22 +2065,22 @@ static bool PrepareZramBackingDevice(off64_t size) { // Allocate loop device and attach it to file_path. LoopControl loop_control; - std::string device; - if (!loop_control.Attach(target_fd.get(), 5s, &device)) { + std::string loop_device; + if (!loop_control.Attach(target_fd.get(), 5s, &loop_device)) { return false; } // set block size & direct IO - unique_fd device_fd(TEMP_FAILURE_RETRY(open(device.c_str(), O_RDWR | O_CLOEXEC))); - if (device_fd.get() == -1) { - PERROR << "Cannot open " << device; + unique_fd loop_fd(TEMP_FAILURE_RETRY(open(loop_device.c_str(), O_RDWR | O_CLOEXEC))); + if (loop_fd.get() == -1) { + PERROR << "Cannot open " << loop_device; return false; } - if (!LoopControl::EnableDirectIo(device_fd.get())) { + if (!LoopControl::EnableDirectIo(loop_fd.get())) { return false; } - return InstallZramDevice(device); + return InstallZramDevice(loop_device); } bool fs_mgr_swapon_all(const Fstab& fstab) { -- cgit v1.2.3 From 58ba70e045173041bde28ca1cf7b747d15add6b9 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Thu, 12 Aug 2021 10:56:44 -0700 Subject: libdm: Make ExtractBlockDeviceName() return its result From https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.md: "Prefer using return values over output parameters: they improve readability, and often provide the same or better performance (see the C++ Primer)." Implement this advice for ExtractBlockDeviceName(). This patch does not change any functionality. Bug: 194450129 Test: mm libfs_mgr libdm_test Merged-In: I6363781163eba08e6128507b403200f472f68a59 Change-Id: I7d340b33281ebccded0836cd0b5a293e941f4043 Ignore-AOSP-First: Already in AOSP. Signed-off-by: Bart Van Assche (cherry picked from commit 96b21cc589349d346e4c587d92867ddee66f45dd) --- fs_mgr/libdm/dm.cpp | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/fs_mgr/libdm/dm.cpp b/fs_mgr/libdm/dm.cpp index c4874b8d1..9a518c696 100644 --- a/fs_mgr/libdm/dm.cpp +++ b/fs_mgr/libdm/dm.cpp @@ -560,34 +560,30 @@ std::string DeviceMapper::GetTargetType(const struct dm_target_spec& spec) { return std::string{spec.target_type, sizeof(spec.target_type)}; } -static bool ExtractBlockDeviceName(const std::string& path, std::string* name) { +static std::optional ExtractBlockDeviceName(const std::string& path) { static constexpr std::string_view kDevBlockPrefix("/dev/block/"); if (android::base::StartsWith(path, kDevBlockPrefix)) { - *name = path.substr(kDevBlockPrefix.length()); - return true; + return path.substr(kDevBlockPrefix.length()); } - return false; + return {}; } bool DeviceMapper::IsDmBlockDevice(const std::string& path) { - std::string name; - if (!ExtractBlockDeviceName(path, &name)) { - return false; - } - return android::base::StartsWith(name, "dm-"); + std::optional name = ExtractBlockDeviceName(path); + return name && android::base::StartsWith(*name, "dm-"); } std::optional DeviceMapper::GetDmDeviceNameByPath(const std::string& path) { - std::string name; - if (!ExtractBlockDeviceName(path, &name)) { + std::optional name = ExtractBlockDeviceName(path); + if (!name) { LOG(WARNING) << path << " is not a block device"; return std::nullopt; } - if (!android::base::StartsWith(name, "dm-")) { + if (!android::base::StartsWith(*name, "dm-")) { LOG(WARNING) << path << " is not a dm device"; return std::nullopt; } - std::string dm_name_file = "/sys/block/" + name + "/dm/name"; + std::string dm_name_file = "/sys/block/" + *name + "/dm/name"; std::string dm_name; if (!android::base::ReadFileToString(dm_name_file, &dm_name)) { PLOG(ERROR) << "Failed to read file " << dm_name_file; @@ -598,16 +594,16 @@ std::optional DeviceMapper::GetDmDeviceNameByPath(const std::string } std::optional DeviceMapper::GetParentBlockDeviceByPath(const std::string& path) { - std::string name; - if (!ExtractBlockDeviceName(path, &name)) { + std::optional name = ExtractBlockDeviceName(path); + if (!name) { LOG(WARNING) << path << " is not a block device"; return std::nullopt; } - if (!android::base::StartsWith(name, "dm-")) { + if (!android::base::StartsWith(*name, "dm-")) { // Reached bottom of the device mapper stack. return std::nullopt; } - auto slaves_dir = "/sys/block/" + name + "/slaves"; + auto slaves_dir = "/sys/block/" + *name + "/slaves"; auto dir = std::unique_ptr(opendir(slaves_dir.c_str()), closedir); if (dir == nullptr) { PLOG(ERROR) << "Failed to open: " << slaves_dir; -- cgit v1.2.3 From 0b73cdcb3837e1e4e7f4fda67ab7fbaa0be1875c Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Thu, 12 Aug 2021 12:04:07 -0700 Subject: libdm: Export ExtractBlockDeviceName() Make this function available to libdm users. A caller outside libdm will be added by a later patch. Bug: 194450129 Test: mm libfs_mgr libdm_test Merged-In: I3e3560f3cdef8978eac644d5b53cf3851209c0c2 Change-Id: Ic05cc84565952662178bb649ec97cad6f76dcf92 Ignore-AOSP-First: Already in AOSP. Signed-off-by: Bart Van Assche (cherry picked from commit 9e54a90e546b195dba1bfcd14df6d71bfbbdd60f) --- fs_mgr/libdm/dm.cpp | 2 +- fs_mgr/libdm/include/libdm/dm.h | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/fs_mgr/libdm/dm.cpp b/fs_mgr/libdm/dm.cpp index 9a518c696..e43c00b44 100644 --- a/fs_mgr/libdm/dm.cpp +++ b/fs_mgr/libdm/dm.cpp @@ -560,7 +560,7 @@ std::string DeviceMapper::GetTargetType(const struct dm_target_spec& spec) { return std::string{spec.target_type, sizeof(spec.target_type)}; } -static std::optional ExtractBlockDeviceName(const std::string& path) { +std::optional ExtractBlockDeviceName(const std::string& path) { static constexpr std::string_view kDevBlockPrefix("/dev/block/"); if (android::base::StartsWith(path, kDevBlockPrefix)) { return path.substr(kDevBlockPrefix.length()); diff --git a/fs_mgr/libdm/include/libdm/dm.h b/fs_mgr/libdm/include/libdm/dm.h index 70b14fa46..bdbbf9112 100644 --- a/fs_mgr/libdm/include/libdm/dm.h +++ b/fs_mgr/libdm/include/libdm/dm.h @@ -49,6 +49,10 @@ enum class DmDeviceState { INVALID, SUSPENDED, ACTIVE }; static constexpr uint64_t kSectorSize = 512; +// Returns `path` without /dev/block prefix if and only if `path` starts with +// that prefix. +std::optional ExtractBlockDeviceName(const std::string& path); + class DeviceMapper final { public: class DmBlockDevice final { -- cgit v1.2.3 From 3a9c1794385ae80c7835a8ec496f4a2efdf4de76 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 6 Aug 2021 10:21:12 -0700 Subject: Reduce the queue depth of loop devices used by the zram driver Make the queue depth of loop devices identical to that of the underlying storage device. This patch reduces latency by lowering the queue depth. With this patch applied I see the following: # cat /sys/block/loop30/queue/nr_requests 32 Bug: 194450129 Test: Built Android images, installed these and verified that the queue depth of loop devices is 32 instead of 256. Merged-In: Ifa16084c7df3a54d9559c2388abc4a8392ff88c6 Change-Id: Icc89e1f88d2f0ade2805999afef556b15b7ff8eb Ignore-AOSP-First: Already in AOSP. Signed-off-by: Bart Van Assche (cherry picked from commit 1a9cad816de296b3f5ee026869cab4413ec7527c) --- fs_mgr/Android.bp | 1 + fs_mgr/blockdev.cpp | 159 ++++++++++++++++++++++++++++++++++++++++++++++++++++ fs_mgr/blockdev.h | 21 +++++++ fs_mgr/fs_mgr.cpp | 3 + 4 files changed, 184 insertions(+) create mode 100644 fs_mgr/blockdev.cpp create mode 100644 fs_mgr/blockdev.h diff --git a/fs_mgr/Android.bp b/fs_mgr/Android.bp index 5356b006d..3c83aabba 100644 --- a/fs_mgr/Android.bp +++ b/fs_mgr/Android.bp @@ -63,6 +63,7 @@ cc_defaults { "-D_FILE_OFFSET_BITS=64", ], srcs: [ + "blockdev.cpp", "file_wait.cpp", "fs_mgr.cpp", "fs_mgr_format.cpp", diff --git a/fs_mgr/blockdev.cpp b/fs_mgr/blockdev.cpp new file mode 100644 index 000000000..14b217ca8 --- /dev/null +++ b/fs_mgr/blockdev.cpp @@ -0,0 +1,159 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include "blockdev.h" + +using android::base::Basename; +using android::base::ErrnoError; +using android::base::Error; +using android::base::Result; +using android::base::ResultError; +using android::base::StartsWith; +using android::base::StringPrintf; +using android::base::unique_fd; +using android::dm::DeviceMapper; + +// Return the parent device of a partition. Converts e.g. "sda26" into "sda". +static std::string PartitionParent(const std::string& blockdev) { + if (blockdev.find('/') != std::string::npos) { + LOG(ERROR) << __func__ << ": invalid argument " << blockdev; + return blockdev; + } + auto dir = std::unique_ptr{opendir("/sys/class/block"), closedir}; + if (!dir) { + return blockdev; + } + for (struct dirent* ent = readdir(dir.get()); ent; ent = readdir(dir.get())) { + if (ent->d_name[0] == '.') { + continue; + } + std::string path = StringPrintf("/sys/class/block/%s/%s", ent->d_name, blockdev.c_str()); + struct stat statbuf; + if (stat(path.c_str(), &statbuf) >= 0) { + return ent->d_name; + } + } + return blockdev; +} + +// Convert a major:minor pair into a block device name. +static std::string BlockdevName(dev_t dev) { + auto dir = std::unique_ptr{opendir("/dev/block"), closedir}; + if (!dir) { + return {}; + } + for (struct dirent* ent = readdir(dir.get()); ent; ent = readdir(dir.get())) { + if (ent->d_name[0] == '.') { + continue; + } + const std::string path = std::string("/dev/block/") + ent->d_name; + struct stat statbuf; + if (stat(path.c_str(), &statbuf) >= 0 && dev == statbuf.st_rdev) { + return ent->d_name; + } + } + return {}; +} + +// Trim whitespace from the end of a string. +static void rtrim(std::string& s) { + s.erase(s.find_last_not_of('\n') + 1, s.length()); +} + +// For file `file_path`, retrieve the block device backing the filesystem on +// which the file exists and return the queue depth of the block device. +static Result BlockDeviceQueueDepth(const std::string& file_path) { + struct stat statbuf; + int res = stat(file_path.c_str(), &statbuf); + if (res < 0) { + return ErrnoError() << "stat(" << file_path << ")"; + } + std::string blockdev = "/dev/block/" + BlockdevName(statbuf.st_dev); + LOG(DEBUG) << __func__ << ": " << file_path << " -> " << blockdev; + if (blockdev.empty()) { + const std::string err_msg = + StringPrintf("Failed to convert %u:%u (path %s)", major(statbuf.st_dev), + minor(statbuf.st_dev), file_path.c_str()); + return ResultError(err_msg, 0); + } + auto& dm = DeviceMapper::Instance(); + for (;;) { + std::optional child = dm.GetParentBlockDeviceByPath(blockdev); + if (!child) { + break; + } + LOG(DEBUG) << __func__ << ": " << blockdev << " -> " << *child; + blockdev = *child; + } + std::optional maybe_blockdev = android::dm::ExtractBlockDeviceName(blockdev); + if (!maybe_blockdev) { + return ResultError("Failed to remove /dev/block/ prefix from " + blockdev, 0); + } + blockdev = PartitionParent(*maybe_blockdev); + LOG(DEBUG) << __func__ << ": " + << "Partition parent: " << blockdev; + const std::string nr_tags_path = + StringPrintf("/sys/class/block/%s/mq/0/nr_tags", blockdev.c_str()); + std::string nr_tags; + if (!android::base::ReadFileToString(nr_tags_path, &nr_tags)) { + return ResultError("Failed to read " + nr_tags_path, 0); + } + rtrim(nr_tags); + LOG(DEBUG) << __func__ << ": " << file_path << " is backed by /dev/" << blockdev + << " and that block device supports queue depth " << nr_tags; + return strtol(nr_tags.c_str(), NULL, 0); +} + +// Set 'nr_requests' of `loop_device_path` to the queue depth of the block +// device backing `file_path`. +Result ConfigureQueueDepth(const std::string& loop_device_path, + const std::string& file_path) { + if (!StartsWith(loop_device_path, "/dev/")) { + return Error() << "Invalid argument " << loop_device_path; + } + + const std::string loop_device_name = Basename(loop_device_path); + + const Result qd = BlockDeviceQueueDepth(file_path); + if (!qd.ok()) { + LOG(DEBUG) << __func__ << ": " + << "BlockDeviceQueueDepth() returned " << qd.error(); + return ResultError(qd.error()); + } + const std::string nr_requests = StringPrintf("%u", *qd); + const std::string sysfs_path = + StringPrintf("/sys/class/block/%s/queue/nr_requests", loop_device_name.c_str()); + unique_fd sysfs_fd(open(sysfs_path.c_str(), O_RDWR | O_CLOEXEC)); + if (sysfs_fd == -1) { + return ErrnoError() << "Failed to open " << sysfs_path; + } + + const int res = write(sysfs_fd.get(), nr_requests.data(), nr_requests.length()); + if (res < 0) { + return ErrnoError() << "Failed to write to " << sysfs_path; + } + return {}; +} diff --git a/fs_mgr/blockdev.h b/fs_mgr/blockdev.h new file mode 100644 index 000000000..2c0d68a77 --- /dev/null +++ b/fs_mgr/blockdev.h @@ -0,0 +1,21 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include + +android::base::Result ConfigureQueueDepth(const std::string& loop_device_path, + const std::string& file_path); diff --git a/fs_mgr/fs_mgr.cpp b/fs_mgr/fs_mgr.cpp index 6b8e39eec..21df8af8c 100644 --- a/fs_mgr/fs_mgr.cpp +++ b/fs_mgr/fs_mgr.cpp @@ -70,6 +70,7 @@ #include #include +#include "blockdev.h" #include "fs_mgr_priv.h" #define KEY_LOC_PROP "ro.crypto.keyfile.userdata" @@ -2070,6 +2071,8 @@ static bool PrepareZramBackingDevice(off64_t size) { return false; } + ConfigureQueueDepth(loop_device, "/"); + // set block size & direct IO unique_fd loop_fd(TEMP_FAILURE_RETRY(open(loop_device.c_str(), O_RDWR | O_CLOEXEC))); if (loop_fd.get() == -1) { -- cgit v1.2.3