Merge "Cleanup package_string() and its users"
diff --git a/base/Android.bp b/base/Android.bp
index aeb8864..8351461 100644
--- a/base/Android.bp
+++ b/base/Android.bp
@@ -149,6 +149,7 @@
"logging_test.cpp",
"macros_test.cpp",
"mapped_file_test.cpp",
+ "no_destructor_test.cpp",
"parsedouble_test.cpp",
"parsebool_test.cpp",
"parseint_test.cpp",
diff --git a/base/include/android-base/no_destructor.h b/base/include/android-base/no_destructor.h
new file mode 100644
index 0000000..ce0dc9f
--- /dev/null
+++ b/base/include/android-base/no_destructor.h
@@ -0,0 +1,94 @@
+#pragma once
+
+/*
+ * Copyright (C) 2019 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 <utility>
+
+#include "android-base/macros.h"
+
+namespace android {
+namespace base {
+
+// A wrapper that makes it easy to create an object of type T with static
+// storage duration that:
+// - is only constructed on first access
+// - never invokes the destructor
+// in order to satisfy the styleguide ban on global constructors and
+// destructors.
+//
+// Runtime constant example:
+// const std::string& GetLineSeparator() {
+// // Forwards to std::string(size_t, char, const Allocator&) constructor.
+// static const base::NoDestructor<std::string> s(5, '-');
+// return *s;
+// }
+//
+// More complex initialization with a lambda:
+// const std::string& GetSessionNonce() {
+// static const base::NoDestructor<std::string> nonce([] {
+// std::string s(16);
+// crypto::RandString(s.data(), s.size());
+// return s;
+// }());
+// return *nonce;
+// }
+//
+// NoDestructor<T> stores the object inline, so it also avoids a pointer
+// indirection and a malloc. Also note that since C++11 static local variable
+// initialization is thread-safe and so is this pattern. Code should prefer to
+// use NoDestructor<T> over:
+// - A function scoped static T* or T& that is dynamically initialized.
+// - A global base::LazyInstance<T>.
+//
+// Note that since the destructor is never run, this *will* leak memory if used
+// as a stack or member variable. Furthermore, a NoDestructor<T> should never
+// have global scope as that may require a static initializer.
+template <typename T>
+class NoDestructor {
+ public:
+ // Not constexpr; just write static constexpr T x = ...; if the value should
+ // be a constexpr.
+ template <typename... Args>
+ explicit NoDestructor(Args&&... args) {
+ new (storage_) T(std::forward<Args>(args)...);
+ }
+
+ // Allows copy and move construction of the contained type, to allow
+ // construction from an initializer list, e.g. for std::vector.
+ explicit NoDestructor(const T& x) { new (storage_) T(x); }
+ explicit NoDestructor(T&& x) { new (storage_) T(std::move(x)); }
+
+ NoDestructor(const NoDestructor&) = delete;
+ NoDestructor& operator=(const NoDestructor&) = delete;
+
+ ~NoDestructor() = default;
+
+ const T& operator*() const { return *get(); }
+ T& operator*() { return *get(); }
+
+ const T* operator->() const { return get(); }
+ T* operator->() { return get(); }
+
+ const T* get() const { return reinterpret_cast<const T*>(storage_); }
+ T* get() { return reinterpret_cast<T*>(storage_); }
+
+ private:
+ alignas(T) char storage_[sizeof(T)];
+};
+
+} // namespace base
+} // namespace android
diff --git a/base/no_destructor_test.cpp b/base/no_destructor_test.cpp
new file mode 100644
index 0000000..f19468a
--- /dev/null
+++ b/base/no_destructor_test.cpp
@@ -0,0 +1,66 @@
+/*
+ * Copyright (C) 2019 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 "android-base/no_destructor.h"
+
+#include <gtest/gtest.h>
+
+struct __attribute__((packed)) Bomb {
+ Bomb() : magic_(123) {}
+
+ ~Bomb() { exit(42); }
+
+ int get() const { return magic_; }
+
+ private:
+ [[maybe_unused]] char padding_;
+ int magic_;
+};
+
+TEST(no_destructor, bomb) {
+ ASSERT_EXIT(({
+ {
+ Bomb b;
+ if (b.get() != 123) exit(1);
+ }
+
+ exit(0);
+ }),
+ ::testing::ExitedWithCode(42), "");
+}
+
+TEST(no_destructor, defused) {
+ ASSERT_EXIT(({
+ {
+ android::base::NoDestructor<Bomb> b;
+ if (b->get() != 123) exit(1);
+ }
+
+ exit(0);
+ }),
+ ::testing::ExitedWithCode(0), "");
+}
+
+TEST(no_destructor, operators) {
+ android::base::NoDestructor<Bomb> b;
+ const android::base::NoDestructor<Bomb>& c = b;
+ ASSERT_EQ(123, b.get()->get());
+ ASSERT_EQ(123, b->get());
+ ASSERT_EQ(123, (*b).get());
+ ASSERT_EQ(123, c.get()->get());
+ ASSERT_EQ(123, c->get());
+ ASSERT_EQ(123, (*c).get());
+}
diff --git a/fs_mgr/libdm/dm.cpp b/fs_mgr/libdm/dm.cpp
index 824ee43..809318c 100644
--- a/fs_mgr/libdm/dm.cpp
+++ b/fs_mgr/libdm/dm.cpp
@@ -16,12 +16,14 @@
#include "libdm/dm.h"
+#include <linux/dm-ioctl.h>
#include <sys/ioctl.h>
#include <sys/sysmacros.h>
#include <sys/types.h>
#include <chrono>
#include <functional>
+#include <string_view>
#include <thread>
#include <android-base/file.h>
@@ -504,5 +506,74 @@
return std::string{spec.target_type, sizeof(spec.target_type)};
}
+static bool ExtractBlockDeviceName(const std::string& path, std::string* name) {
+ static constexpr std::string_view kDevBlockPrefix("/dev/block/");
+ if (android::base::StartsWith(path, kDevBlockPrefix)) {
+ *name = path.substr(kDevBlockPrefix.length());
+ return true;
+ }
+ return false;
+}
+
+bool DeviceMapper::IsDmBlockDevice(const std::string& path) {
+ std::string name;
+ if (!ExtractBlockDeviceName(path, &name)) {
+ return false;
+ }
+ return android::base::StartsWith(name, "dm-");
+}
+
+std::optional<std::string> DeviceMapper::GetDmDeviceNameByPath(const std::string& path) {
+ std::string name;
+ if (!ExtractBlockDeviceName(path, &name)) {
+ LOG(WARNING) << path << " is not a block device";
+ return std::nullopt;
+ }
+ 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;
+ if (!android::base::ReadFileToString(dm_name_file, &dm_name)) {
+ PLOG(ERROR) << "Failed to read file " << dm_name_file;
+ return std::nullopt;
+ }
+ dm_name = android::base::Trim(dm_name);
+ return dm_name;
+}
+
+std::optional<std::string> DeviceMapper::GetParentBlockDeviceByPath(const std::string& path) {
+ std::string name;
+ if (!ExtractBlockDeviceName(path, &name)) {
+ LOG(WARNING) << path << " is not a block device";
+ return std::nullopt;
+ }
+ if (!android::base::StartsWith(name, "dm-")) {
+ // Reached bottom of the device mapper stack.
+ return std::nullopt;
+ }
+ auto slaves_dir = "/sys/block/" + name + "/slaves";
+ auto dir = std::unique_ptr<DIR, decltype(&closedir)>(opendir(slaves_dir.c_str()), closedir);
+ if (dir == nullptr) {
+ PLOG(ERROR) << "Failed to open: " << slaves_dir;
+ return std::nullopt;
+ }
+ std::string sub_device_name = "";
+ for (auto entry = readdir(dir.get()); entry; entry = readdir(dir.get())) {
+ if (entry->d_type != DT_LNK) continue;
+ if (!sub_device_name.empty()) {
+ LOG(ERROR) << "Too many slaves in " << slaves_dir;
+ return std::nullopt;
+ }
+ sub_device_name = entry->d_name;
+ }
+ if (sub_device_name.empty()) {
+ LOG(ERROR) << "No slaves in " << slaves_dir;
+ return std::nullopt;
+ }
+ return "/dev/block/" + sub_device_name;
+}
+
} // namespace dm
} // namespace android
diff --git a/fs_mgr/libdm/dm_test.cpp b/fs_mgr/libdm/dm_test.cpp
index 1695953..b7f31bc 100644
--- a/fs_mgr/libdm/dm_test.cpp
+++ b/fs_mgr/libdm/dm_test.cpp
@@ -29,6 +29,7 @@
#include <thread>
#include <android-base/file.h>
+#include <android-base/strings.h>
#include <android-base/unique_fd.h>
#include <gtest/gtest.h>
#include <libdm/dm.h>
@@ -544,3 +545,63 @@
ASSERT_NE(0, access(path.c_str(), F_OK));
ASSERT_EQ(ENOENT, errno);
}
+
+TEST(libdm, IsDmBlockDevice) {
+ unique_fd tmp(CreateTempFile("file_1", 4096));
+ ASSERT_GE(tmp, 0);
+ LoopDevice loop(tmp, 10s);
+ ASSERT_TRUE(loop.valid());
+ ASSERT_TRUE(android::base::StartsWith(loop.device(), "/dev/block"));
+
+ DmTable table;
+ ASSERT_TRUE(table.Emplace<DmTargetLinear>(0, 1, loop.device(), 0));
+ ASSERT_TRUE(table.valid());
+
+ TempDevice dev("libdm-test-dm-linear", table);
+ ASSERT_TRUE(dev.valid());
+
+ DeviceMapper& dm = DeviceMapper::Instance();
+ ASSERT_TRUE(dm.IsDmBlockDevice(dev.path()));
+ ASSERT_FALSE(dm.IsDmBlockDevice(loop.device()));
+}
+
+TEST(libdm, GetDmDeviceNameByPath) {
+ unique_fd tmp(CreateTempFile("file_1", 4096));
+ ASSERT_GE(tmp, 0);
+ LoopDevice loop(tmp, 10s);
+ ASSERT_TRUE(loop.valid());
+ ASSERT_TRUE(android::base::StartsWith(loop.device(), "/dev/block"));
+
+ DmTable table;
+ ASSERT_TRUE(table.Emplace<DmTargetLinear>(0, 1, loop.device(), 0));
+ ASSERT_TRUE(table.valid());
+
+ TempDevice dev("libdm-test-dm-linear", table);
+ ASSERT_TRUE(dev.valid());
+
+ DeviceMapper& dm = DeviceMapper::Instance();
+ // Not a dm device, GetDmDeviceNameByPath will return std::nullopt.
+ ASSERT_FALSE(dm.GetDmDeviceNameByPath(loop.device()));
+ auto name = dm.GetDmDeviceNameByPath(dev.path());
+ ASSERT_EQ("libdm-test-dm-linear", *name);
+}
+
+TEST(libdm, GetParentBlockDeviceByPath) {
+ unique_fd tmp(CreateTempFile("file_1", 4096));
+ ASSERT_GE(tmp, 0);
+ LoopDevice loop(tmp, 10s);
+ ASSERT_TRUE(loop.valid());
+ ASSERT_TRUE(android::base::StartsWith(loop.device(), "/dev/block"));
+
+ DmTable table;
+ ASSERT_TRUE(table.Emplace<DmTargetLinear>(0, 1, loop.device(), 0));
+ ASSERT_TRUE(table.valid());
+
+ TempDevice dev("libdm-test-dm-linear", table);
+ ASSERT_TRUE(dev.valid());
+
+ DeviceMapper& dm = DeviceMapper::Instance();
+ ASSERT_FALSE(dm.GetParentBlockDeviceByPath(loop.device()));
+ auto sub_block_device = dm.GetParentBlockDeviceByPath(dev.path());
+ ASSERT_EQ(loop.device(), *sub_block_device);
+}
diff --git a/fs_mgr/libdm/include/libdm/dm.h b/fs_mgr/libdm/include/libdm/dm.h
index 830c5e8..418210c 100644
--- a/fs_mgr/libdm/include/libdm/dm.h
+++ b/fs_mgr/libdm/include/libdm/dm.h
@@ -214,6 +214,19 @@
static std::string GetTargetType(const struct dm_target_spec& spec);
+ // Returns true if given path is a path to a dm block device.
+ bool IsDmBlockDevice(const std::string& path);
+
+ // Returns name of a dm-device with the given path, or std::nulloptr if given path is not a
+ // dm-device.
+ std::optional<std::string> GetDmDeviceNameByPath(const std::string& path);
+
+ // Returns a parent block device of a dm device with the given path, or std::nullopt if:
+ // * Given path doesn't correspond to a dm device.
+ // * A dm device is based on top of more than one block devices.
+ // * A failure occurred.
+ std::optional<std::string> GetParentBlockDeviceByPath(const std::string& path);
+
private:
// Maximum possible device mapper targets registered in the kernel.
// This is only used to read the list of targets from kernel so we allocate
diff --git a/init/persistent_properties.cpp b/init/persistent_properties.cpp
index baa9ad4..3b5a41d 100644
--- a/init/persistent_properties.cpp
+++ b/init/persistent_properties.cpp
@@ -31,10 +31,11 @@
#include "util.h"
+using android::base::Dirname;
using android::base::ReadFdToString;
using android::base::StartsWith;
-using android::base::WriteStringToFd;
using android::base::unique_fd;
+using android::base::WriteStringToFd;
namespace android {
namespace init {
@@ -191,6 +192,18 @@
unlink(temp_filename.c_str());
return Error(saved_errno) << "Unable to rename persistent property file";
}
+
+ // rename() is atomic with regards to the kernel's filesystem buffers, but the parent
+ // directories must be fsync()'ed otherwise, the rename is not necessarily written to storage.
+ // Note in this case, that the source and destination directories are the same, so only one
+ // fsync() is required.
+ auto dir = Dirname(persistent_property_filename);
+ auto dir_fd = unique_fd{open(dir.c_str(), O_DIRECTORY | O_RDONLY)};
+ if (dir_fd < 0) {
+ return ErrnoError() << "Unable to open persistent properties directory for fsync()";
+ }
+ fsync(dir_fd);
+
return {};
}
diff --git a/rootdir/init.rc b/rootdir/init.rc
index ff68734..e2ecad4 100644
--- a/rootdir/init.rc
+++ b/rootdir/init.rc
@@ -770,6 +770,11 @@
write /sys/fs/f2fs/${dev.mnt.blk.data}/cp_interval 200
write /sys/fs/f2fs/${dev.mnt.blk.data}/gc_urgent_sleep_time 50
+ # limit discard size to 128MB in order to avoid long IO latency
+ # for filesystem tuning first (dm or sda)
+ # Note that, if dm-<num> is used, sda/mmcblk0 should be tuned in vendor/init.rc
+ write /sys/devices/virtual/block/${dev.mnt.blk.data}/queue/discard_max_bytes 134217728
+
# Permissions for System Server and daemons.
chown system system /sys/power/autosleep