From 3e7c17a8e7ad7275ea3a65642608b1c0becb38ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Wed, 20 Nov 2024 19:37:05 +0000 Subject: [PATCH 1/5] Reapply "libprocessgroup: Remove __BEGIN_DECLS and __END_DECLS" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit fdaaef95238d1de873040482938585cb01f724d6. Reason for revert: should be safe to reapply post https://android-review.git.corp.google.com/c/platform/packages/modules/Connectivity/+/3369257 Test: TreeHugger Bug: 379796721 Signed-off-by: Maciej Żenczykowski Change-Id: I1a24791713484461345312a34777c2266e443cee --- libprocessgroup/include/processgroup/processgroup.h | 8 -------- 1 file changed, 8 deletions(-) diff --git a/libprocessgroup/include/processgroup/processgroup.h b/libprocessgroup/include/processgroup/processgroup.h index d27b56895..6a026a717 100644 --- a/libprocessgroup/include/processgroup/processgroup.h +++ b/libprocessgroup/include/processgroup/processgroup.h @@ -16,7 +16,6 @@ #pragma once -#include #include #include #include @@ -24,8 +23,6 @@ #include #include -__BEGIN_DECLS - static constexpr std::string CGROUPV2_HIERARCHY_NAME = "cgroup2"; bool CgroupsAvailable(); @@ -39,8 +36,6 @@ bool SetTaskProfiles(pid_t tid, const std::vector& profiles, bool SetProcessProfiles(uid_t uid, pid_t pid, const std::vector& profiles); bool SetUserProfiles(uid_t uid, const std::vector& profiles); -__END_DECLS - bool SetTaskProfiles(pid_t tid, std::initializer_list profiles, bool use_fd_cache = false); bool SetProcessProfiles(uid_t uid, pid_t pid, std::initializer_list profiles); @@ -50,7 +45,6 @@ bool SetTaskProfiles(pid_t tid, std::span profiles, bool SetProcessProfiles(uid_t uid, pid_t pid, std::span profiles); #endif -__BEGIN_DECLS #ifndef __ANDROID_VNDK__ @@ -96,5 +90,3 @@ bool getAttributePathForTask(const std::string& attr_name, pid_t tid, std::strin bool isProfileValidForProcess(const std::string& profile_name, uid_t uid, pid_t pid); #endif // __ANDROID_VNDK__ - -__END_DECLS From 7bb484d405b42080e3bd08d9890d84d505ca5271 Mon Sep 17 00:00:00 2001 From: Akilesh Kailash Date: Wed, 20 Nov 2024 13:07:26 -0800 Subject: [PATCH 2/5] snapuserd: Lock the buffer during snapshot-merge Bug: 377819507 Test: Incremental OTA on Pixel Change-Id: I08fa7129282cc005a565987856166088c092f40a Signed-off-by: Akilesh Kailash --- .../user-space-merge/merge_worker.cpp | 16 +++++++++ .../user-space-merge/snapuserd_core.h | 4 +++ .../user-space-merge/snapuserd_readahead.cpp | 35 +++++++++++-------- 3 files changed, 41 insertions(+), 14 deletions(-) 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 a0c5c6658..febb4847d 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.cpp @@ -233,6 +233,11 @@ bool MergeWorker::MergeOrderedOpsAsync() { return false; } + std::optional> buffer_lock; + // Acquire the buffer lock at this point so that RA thread + // doesn't step into this buffer. See b/377819507 + buffer_lock.emplace(snapuserd_->GetBufferLock()); + snapuserd_->SetMergeInProgress(ra_block_index_); loff_t offset = 0; @@ -383,6 +388,9 @@ bool MergeWorker::MergeOrderedOpsAsync() { // Mark the block as merge complete snapuserd_->SetMergeCompleted(ra_block_index_); + // Release the buffer lock + buffer_lock.reset(); + // Notify RA thread that the merge thread is ready to merge the next // window snapuserd_->NotifyRAForMergeReady(); @@ -415,6 +423,11 @@ bool MergeWorker::MergeOrderedOps() { return false; } + std::optional> buffer_lock; + // Acquire the buffer lock at this point so that RA thread + // doesn't step into this buffer. See b/377819507 + buffer_lock.emplace(snapuserd_->GetBufferLock()); + snapuserd_->SetMergeInProgress(ra_block_index_); loff_t offset = 0; @@ -468,6 +481,9 @@ bool MergeWorker::MergeOrderedOps() { // Mark the block as merge complete snapuserd_->SetMergeCompleted(ra_block_index_); + // Release the buffer lock + buffer_lock.reset(); + // Notify RA thread that the merge thread is ready to merge the next // window snapuserd_->NotifyRAForMergeReady(); 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 c7de9951f..2340b0b20 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.h +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.h @@ -186,6 +186,7 @@ class SnapshotHandler : public std::enable_shared_from_this { bool IsIouringSupported(); bool CheckPartitionVerification(); + std::mutex& GetBufferLock() { return buffer_lock_; } private: bool ReadMetadata(); @@ -216,6 +217,9 @@ class SnapshotHandler : public std::enable_shared_from_this { std::mutex lock_; std::condition_variable cv; + // Lock the buffer used for snapshot-merge + std::mutex buffer_lock_; + void* mapped_addr_; size_t total_mapped_addr_length_; 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 3007d45f4..c7ae51926 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_readahead.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_readahead.cpp @@ -706,25 +706,32 @@ bool ReadAhead::ReadAheadIOStart() { return false; } - // Copy the data to scratch space - memcpy(metadata_buffer_, ra_temp_meta_buffer_.get(), snapuserd_->GetBufferMetadataSize()); - memcpy(read_ahead_buffer_, ra_temp_buffer_.get(), total_blocks_merged_ * BLOCK_SZ); + // Acquire buffer lock before doing memcpy to the scratch buffer. Although, + // by now snapshot-merge thread shouldn't be working on this scratch space + // but we take additional measure to ensure that the buffer is not being + // used by the merge thread at this point. see b/377819507 + { + std::lock_guard buffer_lock(snapuserd_->GetBufferLock()); + // Copy the data to scratch space + memcpy(metadata_buffer_, ra_temp_meta_buffer_.get(), snapuserd_->GetBufferMetadataSize()); + memcpy(read_ahead_buffer_, ra_temp_buffer_.get(), total_blocks_merged_ * BLOCK_SZ); - loff_t offset = 0; - std::unordered_map& read_ahead_buffer_map = snapuserd_->GetReadAheadMap(); - read_ahead_buffer_map.clear(); + loff_t offset = 0; + std::unordered_map& read_ahead_buffer_map = snapuserd_->GetReadAheadMap(); + read_ahead_buffer_map.clear(); - for (size_t block_index = 0; block_index < blocks_.size(); block_index++) { - void* bufptr = static_cast((char*)read_ahead_buffer_ + offset); - uint64_t new_block = blocks_[block_index]; + for (size_t block_index = 0; block_index < blocks_.size(); block_index++) { + void* bufptr = static_cast((char*)read_ahead_buffer_ + offset); + uint64_t new_block = blocks_[block_index]; - read_ahead_buffer_map[new_block] = bufptr; - offset += BLOCK_SZ; + read_ahead_buffer_map[new_block] = bufptr; + offset += BLOCK_SZ; + } + + total_ra_blocks_completed_ += total_blocks_merged_; + snapuserd_->SetMergedBlockCountForNextCommit(total_blocks_merged_); } - total_ra_blocks_completed_ += total_blocks_merged_; - snapuserd_->SetMergedBlockCountForNextCommit(total_blocks_merged_); - // Flush the scratch data - Technically, we should flush only for overlapping // blocks; However, since this region is mmap'ed, the dirty pages can still // get flushed to disk at any random point in time. Instead, make sure From b912e3e548c4879a015342be1910c12a629b8dd8 Mon Sep 17 00:00:00 2001 From: Shintaro Kawamura Date: Thu, 21 Nov 2024 16:28:30 +0900 Subject: [PATCH 3/5] Fix permission of zram writeback and idle file The default permission of "/sys/block/zram0/idle" and "/sys/block/zram0/writeback" are "0200". Adding read permission to the files does not make sense because reading those files fail as EIO error. We should keep permission as minimum as possible. Bug: 117682284 Bug: 375432468 Test: ls -la /sys/block/zram0 Change-Id: I11ed5d9eee257002f4698edcd81de39d2c317ea1 --- rootdir/init.rc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rootdir/init.rc b/rootdir/init.rc index eeafd4c85..82844352a 100644 --- a/rootdir/init.rc +++ b/rootdir/init.rc @@ -1127,9 +1127,9 @@ on boot # System server manages zram writeback chown root system /sys/block/zram0/idle - chmod 0664 /sys/block/zram0/idle + chmod 0220 /sys/block/zram0/idle chown root system /sys/block/zram0/writeback - chmod 0664 /sys/block/zram0/writeback + chmod 0220 /sys/block/zram0/writeback # to access F2FS sysfs on dm- directly mkdir /dev/sys/fs/by-name 0755 system system From 0701fed364e57ba7cb085c297ffe4641449c3d86 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Thu, 21 Nov 2024 18:46:28 +0000 Subject: [PATCH 4/5] Remove no longer necessary MS_LAZYTIME definitions. Change-Id: Idf2d947509731715f895518b541d55d55187c43d --- fs_mgr/libfstab/fstab.cpp | 4 ---- fs_mgr/tests/fs_mgr_test.cpp | 4 ---- 2 files changed, 8 deletions(-) diff --git a/fs_mgr/libfstab/fstab.cpp b/fs_mgr/libfstab/fstab.cpp index ca3599059..01e0e3d87 100644 --- a/fs_mgr/libfstab/fstab.cpp +++ b/fs_mgr/libfstab/fstab.cpp @@ -39,10 +39,6 @@ #include "fstab_priv.h" #include "logging_macros.h" -#if !defined(MS_LAZYTIME) -#define MS_LAZYTIME (1 << 25) -#endif - using android::base::EndsWith; using android::base::ParseByteCount; using android::base::ParseInt; diff --git a/fs_mgr/tests/fs_mgr_test.cpp b/fs_mgr/tests/fs_mgr_test.cpp index 6e050cf4b..8004977c9 100644 --- a/fs_mgr/tests/fs_mgr_test.cpp +++ b/fs_mgr/tests/fs_mgr_test.cpp @@ -37,10 +37,6 @@ using namespace android::fs_mgr; using namespace testing; -#if !defined(MS_LAZYTIME) -#define MS_LAZYTIME (1 << 25) -#endif - namespace { const std::string cmdline = From f00efa024227358d477999cb67d6b73a5967007e Mon Sep 17 00:00:00 2001 From: Wei Li Date: Fri, 22 Nov 2024 06:55:05 +0000 Subject: [PATCH 5/5] Remove system/core/METADATA This directory doesn't contain third-party code and it doesn't need a METADATA file. Bug: 380376695 Test: CIs Change-Id: Ie197786da09304476fb002b178c2a77d424f5a53 --- METADATA | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 METADATA diff --git a/METADATA b/METADATA deleted file mode 100644 index d97975ca3..000000000 --- a/METADATA +++ /dev/null @@ -1,3 +0,0 @@ -third_party { - license_type: NOTICE -}