We missed two incorrect specifiers in the previous commit with this same
title.
We use the %d format specificier for uid_t, which maps to
__kernel_uid32_t, which is unsigned. [1] This is undefined behavior
which can lead to paths with negative UIDs when erroneously large
values are passed for uid:
E libprocessgroup: No such cgroup attribute: /sys/fs/cgroup/uid_-89846/cgroup.freeze
Fix it with %u.
[1] https://cs.android.com/search?q=typedef.*__kernel_uid32_t&ss=android%2Fplatform%2Fsuperproject%2Fmain
Change-Id: Ica04b03526bd2e156f026a2797fe9912b259cd9f
Global UID level cgroup removal was eliminated because of a race
between app launch and app killing using the same directory name. [1]
However isolated app UIDs are assigned sequentially, and are
basically never reused until we wrap around the large range of
isolated UIDs. This leaves thousands of isolated cgroup directories
unused, which consumes kernel memory and increases memory reclaim
overhead. Remove this subset of UID level cgroup directories when
killing process groups.
[1] d0464b0c01
Test: 50 cycle ACT leaves 1000 fewer empty isolated cgroups
Bug: 290953668
Change-Id: If7d2a7b8eec14561a72208049b74ff785ca961bd
Provide profile validity check functions for cases when user wants to
check whether a profile can be successfully applied before actually
applying it. Add test cases to cover new APIs.
Also add a wrapper function for framework code to call it.
Bug: 277233783
Test: atest task_profiles_test
Test: manually verify freezer with outdated cgroup configuration
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Signed-off-by: Li Li <dualli@google.com>
Change-Id: Iefb321dead27adbe67721972f164efea213c06cb
This variable is no longer used.
Fixes: d0464b0c01 ("libprocessgroup: Do not remove uid cgroups directory")
Change-Id: I2b606d953722cf38cc865d91ea00a3b08236675b
This reverts commit aee11b0a3d.
This change was originally reverted because its only user was reverted
under b/243096961 at ag/19679188. We bring it back now with a fixed user.
Bug: 236708592
Bug: 148425913
Ignore-AOSP-First: Topic with AMS changes which is developed on git_master
Change-Id: I2a8ae0d9faabe7950b758a09870d128889be4d0a
Merged-In: I2a8ae0d9faabe7950b758a09870d128889be4d0a
Add a function which sends signals to all members of a process group,
but does not wait for the processes to exit, or for the associated
cgroup to be removed.
Bug: 274646058
Ignore-AOSP-First: Dependency of ActivityManager change which developed on interal git_master
Test: Force-stop of chrome with 15 tabs completes ~500ms faster
Test: Full Play store update causes no ANR
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:d87b6018d25cbbd33b345dc58c634718bf5d0def)
Merged-In: I37dbdecb3394101abbee8495e71f6912b3c031f5
Change-Id: I37dbdecb3394101abbee8495e71f6912b3c031f5
NOTE FOR REVIEWERS - original patch and result patch are not identical.
PLEASE REVIEW CAREFULLY.
Diffs between the patches:
37,6 +537,15 @@
return KillProcessGroup(uid, initialPid, signal, 0 /*retries*/, max_processes);
}
+int sendSignalToProcessGroup(uid_t uid, int initialPid, int signal) {
+ std::string hierarchy_root_path;
+ if (CgroupsAvailable()) {
+ CgroupGetControllerPath(CGROUPV2_CONTROLLER_NAME, &hierarchy_root_path);
+ }
+ const char* cgroup = hierarchy_root_path.c_str();
+ return DoKillProcessGroupOnce(cgroup, uid, initialPid, signal);
+}
+
static int createProcessGroupInternal(uid_t uid, int initialPid, std::string cgroup,
bool activate_controllers) {
auto uid_path = ConvertUidToPath(cgroup.c_str(), uid);
Original patch:
From d87b6018d2 Mon Sep 17 00:00:00 2001
From: T.J. Mercier <tjmercier@google.com>
Date: Tue, 04 Apr 2023 18:41:13 +0000
Subject: [PATCH] libprocessgroup: Add sendSignalToProcessGroup
Add a function which sends signals to all members of a process group,
but does not wait for the processes to exit, or for the associated
cgroup to be removed.
Bug: 274646058
Ignore-AOSP-First: Dependency of ActivityManager change which developed on interal git_master
Test: Force-stop of chrome with 15 tabs completes ~500ms faster
Test: Full Play store update causes no ANR
Change-Id: I37dbdecb3394101abbee8495e71f6912b3c031f5
---
diff --git a/libprocessgroup/include/processgroup/processgroup.h b/libprocessgroup/include/processgroup/processgroup.h
index 8fa9fd5..48bc0b7 100644
--- a/libprocessgroup/include/processgroup/processgroup.h
+++ b/libprocessgroup/include/processgroup/processgroup.h
@@ -76,6 +76,11 @@
// that it only returns 0 in the case that the cgroup exists and it contains no processes.
int killProcessGroupOnce(uid_t uid, int initialPid, int signal, int* max_processes = nullptr);
+// Sends the provided signal to all members of a process group, but does not wait for processes to
+// exit, or for the cgroup to be removed. Callers should also ensure that killProcessGroup is called
+// later to ensure the cgroup is fully removed, otherwise system resources may leak.
+int sendSignalToProcessGroup(uid_t uid, int initialPid, int signal);
+
int createProcessGroup(uid_t uid, int initialPid, bool memControl = false);
// Set various properties of a process group. For these functions to work, the process group must
Change-Id: Ie479348dee8e8092b1959927a1143009632d3914
A user ID (uid) must be greater than or equal to zero to be valid. Only
strictly positive process IDs are valid. Add argument checks in
libprocessgroup of uid and pid arguments to make it easier to determine
the origin of invalid arguments.
Change-Id: I8a6d96ca4576bc9c329498c6a804dd05a02afca5
Signed-off-by: Bart Van Assche <bvanassche@google.com>
Apparently there is Java code that calls KillProcessGroup() with an
invalid initialPid argument. Hence this CL that makes KillProcessGroup()
fail early if one of its arguments is invalid.
Change-Id: I42f98eed139d9d0950428d04180e4613ba74b4e6
Signed-off-by: Bart Van Assche <bvanassche@google.com>
The way processes are accounted in DoKillProcessGroupOnce has been
changed recently, which affects retries in KillProcessGroup. More specifically, initialPid was not counted before and would not
cause a retry with 5ms sleep.
Restore previous behavior to avoid boot time regressions.
Bug: 271198843
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Change-Id: Ibc1bdd855898688a4a03806671e6ac31570aedf9
So the child processes in the process group won't be orphaned
when we decide to kill the process group of a given process but
find it's already dead.
Bug: 266633286
Test: atest MicrodroidDemoApp
Change-Id: Ib6f45b992566f0ab5cf152463c95294a306dd736
It makes no sense, because there are no cgroup procs file.
Bug: 257264124
Test: atest MicrodroidBenchmarkApp
Change-Id: I4e3a118d2237afc46aa8fbcbad055afb7d56f464
process_cgroup_empty_ is used to indicate that a service is already
killed or not. If cgroup support lacks, services cannot be killed
because process_cgroup_empty_ is always true.
This change fixes it by not assigning process_cgroup_empty_ as true.
Instead, make KillProcessGroup send signals even when cgroup is
disabled. Also DoKillProcessGroupOnce() is updated so it returns a number of killed processes, excluding already dead processes. This behavior agrees with its name (DoKillProcessOnce), and it prevents regression upon missing cgroups, because kill(-pgid) will always
"succeed" so KillProcessGroup will loop even when all processes are
already dead.
Bug: 257264124
Test: boot microdroid, see services are terminated
Change-Id: I19abf19ff1b70c666cd6f12d0a12956765174aaa
We are planning to remove cgroups from the Micrdroid kernel, since the
entire VM belongs exclusively to a single owner, and is in the control
of the cgroups on the host side.
This patch expoxes CgroupAvailable API from libprocessgroup, and changes
init to query the CgroupAvailable API before doing any
cgroups/task_profiles related work.
Bug: 239367015
Test: run MicrodroidDemoApp
Test: atest --test-mapping packages/modules/Virtualization:avf-presubmit
Change-Id: I82787141cd2a7f9309a4e9b24acbd92ca21c145b
Provide alternative versions that do not force callers to create
std::string objects. This patch has the intended side-effect that all
callers that pass a {string} initializer list to the 'profiles' argument
now call an std::initializer_list<> overload instead of the const
std::vector<std::string>& overload.
Additionally, add std::function<> arguments instead of calling
ExecuteForProcess() or ExecuteForTask() directly to make it easier to
write unit tests for SetTaskProfiles() and SetProcessProfiles().
Bug: 213617178
Change-Id: Ica61e944a66a17178ee43a113b8ca082f7eb834b
Signed-off-by: Bart Van Assche <bvanassche@google.com>
Cgroup removal fails with EBUSY if there are active processes or threads
still alive in the cgroup. Occasionally a thread or a process might be
stuck in an interruptible sleep and take some time during exit. In such
cases attempts to remove the cgroup it belongs to will fail. This
results in occasional leftover cgroups. These empty unused cgroups
consume memory.
Ensure RemoveProcessGroup always retries and increase the retries to
keep trying for 2 secs before giving up. In majority of cases only a few
retries are needed but in rare cases a thread can be blocked for longer
time, therefore the number of retries is set large enough to cover them.
Bug: 233319780
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Change-Id: I2e4bb1f7b7e19c904c85faea7bbabbfdef9c8125
When system_server and zygote crash or get killed, all apps also get
killed but their process groups are left empty. Provide a function to
remove all empty process groups so that init can purge them when this
even happens.
Bug: 228160715
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Change-Id: Ife38ca021e80cd38106f218ae13183e8c2631bf0
This CL fixes a bug that null names are passed when profiles are set
via android_set_process_profiles. This is because the `profiles_` vector
was initialized with the number of task profiles and then we append the
actual task profile names to the vector. As a result, when {"a", "b"}
was given, the vector ended up having {"", "", "a", "b"}. Fixing this by
correctly using reserve().
Bug: N/A
Test: m
Change-Id: I28d6c2e891b01a2d3a8a88d9d0652fe0dbffac96
The wrapper is to call SetProcessFiles (C++ API) from crosvm via FFI.
Bug: 223790172
Bug: 216788146
Test: m
Change-Id: If342ca0d19deb1cb7ee581bba2cc543385199cbe
This reverts commit 812d7698d8.
Reason for revert: this patch is suspected to have caused b/227337425.
Bug: 227337425
Bug: 227331047
Change-Id: I4ae26ccf61ad7c63dacc85da878ba0920736951c
Without this patch attempts to modify the blkio cgroup attributes by
/system/bin/mediaserver fail as follows:
03-23 09:27:59.542 517 1811 E libprocessgroup: Failed to write '100' to /sys/fs/cgroup/./uid_1013/pid_517/io.bfq.weight: Permission denied
This is because the mediaserver process is started as user 'media',
because the mediaserver process is not in the system group and hence
does not have permission to write into a directory with the following
owner, group and permissions:
vsoc_x86_64:/ # ls -ld /sys/fs/cgroup/./uid_1013/pid_517/io.bfq.weight
-rwxrwxr-x 1 system system 0 2022-03-23 09:27 /sys/fs/cgroup/./uid_1013/pid_517/io.bfq.weight
Bug: 213617178
Test: Booted Android in Cuttlefish and inspected logcat.
Change-Id: I788acc9a137ae29898177f492cae2f954a9c811c
Signed-off-by: Bart Van Assche <bvanassche@google.com>
From the stat() man page: "RETURN VALUE On success, zero is returned.
On error, -1 is returned, and errno is set appropriately." Hence check
for failure by checking whether the return value is negative instead of
1.
Bug: 213617178
Test: Booted Android in Cuttlefish and inspected logcat.
Fixes: 9e628a6b42 ("libprocessgroup: fix uid/pid hierarchy for recovery mode")
Change-Id: I774d142058b083403d32b3f6aae4a4b3de00192c
Signed-off-by: Bart Van Assche <bvanassche@google.com>
When using the v1 hierarchy per process memcg directories exist under
/dev/memcg/apps. When using the v2 hierarchy per process memcg
directories exist under /sys/fs/cgroup. Hence this patch that selects
the proper top-level directory depending on the memcg version.
Bug: 213617178
Test: Verified Android operation inside the Cuttlefish emulator.
Change-Id: I7373fb407cb6ad2b1181579691ff54886fd36c24
Signed-off-by: Bart Van Assche <bvanassche@google.com>
This patch makes the intent of the code more clear without changing any
functionality since uid_t and gid_t are both aliases for uint32_t on
Android systems. See also
https://cs.android.com/android/platform/superproject/+/master:bionic/libc/include/sys/types.h
Bug: 213617178
Test: Compile-tested only.
Change-Id: Ib5012456a7357e79368f00e71e9a280fd6f96063
Signed-off-by: Bart Van Assche <bvanassche@google.com>
This patch does not change any functionality but makes it easier to add
unit tests in a later patch.
Bug: 213617178
Test: Compile-tested only.
Change-Id: I6fbbb3297795c9d7ece8fb3263b3a9b0e5115b18
Signed-off-by: Bart Van Assche <bvanassche@google.com>
Provide context information to make it easier to map error messages to
the source code that reported these error messages.
Bug: 213617178
Test: Booted Android in Cuttlefish and verified the error messages in logcat.
Change-Id: I22e6d91476d91dcf32bafe5ead922e5652136584
Signed-off-by: Bart Van Assche <bvanassche@google.com>
Process profiles operating on paths that do not depend on pid or uid of
the process can cache the fd of the file they are operating on. Add
support for fd caching similar to how SetTaskProfiles caches the fd
of the file it needs to write to.
Bug: 215557553
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Change-Id: Ie73ebcbbf1919d90409f40c1f6b08743f4edf97c
Add new CgroupGetControllerFromPath function to retrieve the name of the
cgroup using a file path. If the file does not belong to any cgroup, the
function returns false.
Bug: 191283136
Test: build and boot
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Change-Id: Ic17a474cb25a80a3339b33ed8bc27b07af053abb
When creating uid/pid hierarchy, cgroup.subtree_control should be set at
every level of that hierarchy except for the leaf level.
Bug: 195149205
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Change-Id: Iedc2e859715b31db62158c85016708f252db2b70
When a new process is launched it ensures that all files under its uid/
and uid/pid hierarchy are accessible by the user/group of that process.
If the directory already exists that means the access permissions have
been already set before, therefore we do not need to reset them again.
This also avoids a race between two processes in the same uid with one
process being launched and walking the uid/ directory while the other
process is being killed and changing the content of that directory. In
such a race the process walking uid/ might find the uid/pid directory of
the process being killed but by the time it tries to set its permissions
the directory might be removed because the process got killed. The
change eliminates the possibility of this race.
Bug: 192421915
Bug: 192512069
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Change-Id: I182298c36f6b0b4580ab59e440bd3aea16f5fbfe
Merged-In: I182298c36f6b0b4580ab59e440bd3aea16f5fbfe
In some rare cases, race happens between 2 processes in the same uid.
1. Process A is dying
2. system_server calls RemoveProcessGroup() for A
3. Zygote forks Process B with the same uid of A
4. system_server calls MkdirAndChown(uid) for B
5. system_server calls MkdirAndChown(uid, pid) for B
As 2 & 4/5 belong to different threads, 2 might happens before or after
step 4/5, or even in the middle of 4/5. In such a case, 4 or 5 will
fail, leaving process B in wrong (Zygote) group.
The uid dir is only created when the corresponding apps have been
launched at least once. It's reasonable to assume one of them is going
to be launched again. Deleting and recreating the uid dir just slows
down applaunch.
Introducing a new lock in libprocessgroup can also solve the race issue.
But that will slow down the applaunch further.
Therefore, reusing the uid dir is an optimized way to solve the race.
Ignore-AOSP-First: Freezer is not a public feature yet
Bug: 192512069
Bug: 168907513
Test: Kill corresponding apps and check the uid cgroupfs dir
Merged-In: I2e91088f21f45e4eda6c709a4af65ace7e135801
Change-Id: I2e91088f21f45e4eda6c709a4af65ace7e135801
configure the cgroup v2 hierarchy for recovery mode, and create uid/pid
groups with attributes following the container cgroup directory.
Bug: 168907513
Test: verified correct pid migration in normal and recovery modes
Change-Id: Idc8b96b4db075383a6a2e523c241b0bc632c7030
This reverts commit 088924af2a.
Bug: 168907513
Test: verified correct function of the cgroup v2 hierarchy in normal and
recovery mode
Change-Id: I6e9d21ebe832326ed5a5b2c356fe8363c1546a80
a1a0497984
Bug: 151660495
Test: verified proper boot in regular mode and proper working of adb in
recovery
Change-Id: I1276796e982fee932cdea7eb145f20b3f1b3463d
Enable the uid/pid hierarchy for all groups when using cgroup v2. Mount
the hierarchy under the cgroup v2 root. Make sure that all files under
the hierarchy are accessible by the system user.
Test: booted the device, tested the freezer cgroup, manually verified
the working of the freezer from logs and by checking statuses of
processes.
Bug: 168907513
Test: Booted the device, verified no regressions on process group access
Change-Id: I73f3e767d377902af6e12facb503b9136fb39e08
Because we cache file descriptors associated with cgroup "tasks" file it
should not be used with SetProcessProfiles API which operates on entire
processes rather than tasks. Change SetProcessProfiles API to prevent
cache fd usage, modify ExecuteForProcess to not attempt to use cached
fd. Also fix unconditional calls to EnableResourceCaching from
ExecuteForTask which should be called only when SetTaskProfiles is used
with use_fd_cache set to true.
Bug: 149524788
Change-Id: I880efaf8217a4dd7ccfbb4fb167b2295cefc057a
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
~2007 a change was added that would allow oneshot services to
daemonize by not killing their process group. This was a hack at the
time, and should certainly not be needed now. I've resisted removing
the behavior however, as it hadn't caused any issues.
Recently, it was detected that the cgroups that these processes belong
to, would exist forever and therefore leak memory. Instead of simply
removing the cgroups when empty, this provides a good opportunity to
do the right thing and fix this behavior once and for all.
The new (correct) behavior only happens for devices with vendor images
built for Android R or later. Init will log a warning to dmesg when
it detects this difference in behavior has occurred.
Bug: 144545923
Test: boot CF/Coral and see no difference in behavior.
Test: boot CF with a service that daemonizes and see the warning.
Change-Id: I333a2e25a541ec0114ac50ab8ae7f1ea3f055447
To support setting multiple profiles with one call. The json format
is as below example.
"AggregateProfiles": [
...
{
"Name": "SCHED_SP_BACKGROUND",
"Profiles": [ "HighEnergySaving", "LowIoPriority", "TimerSlackHigh" ]
},
...
}
Bug: 139521784
Test: SetProfile works as expected
Change-Id: Ibe14ed57d5169cafcbcbbdb054df3ed171a2f6a2
A process can give up the permission to set cgroup. If we still
keep the fd that was cached before losing the permission, when
the process sets scheduling group, it will write to the cached
fd without checking if is accessible and lead to sepolicy denied.
Bug: 123043091
Test: Build and boot.
Test: A new process from zygote set cgroup and drop fd cache, and
then specializes to app domain. There is no sepolicy denied
when the process creates new thread.
(android::thread_data_t::trampoline)
Change-Id: I285ee91424ea965ea9c670fc0f6662948e3e2ce5
Controllers listed in cgroups.json file might fail to mount if kernel is
not configured to support them. We need a way to indicate whether a
controller was successfully mounted and is usable to avoid logging errors
and warnings when a controller that failed to mount is being used. Add
flags bitmask to cgrouprc controller descriptor and use a bit to indicate
that controller is successfully mounted. Modify cpusets_enabled() and
schedboost_enabled() functions to use this bit and report the actual
availability of the controller.
Bug: 124080437
Test: libcutils_test with cpuset and schedtune controllers disabled
Change-Id: I770cc39fe50465146e3205aacf77dc3c56923c5d
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
This reverts commit bee9f5718b
"libprocessgroup: Disable file descriptor caching temporarily" and adds
option to use SetTaskProfiles and SetProcessProfiles without file caching.
This option is used from JNI to avoid access denials because cached files
are not whitelisted for JNI usage.
Bug: 123868658
Bug: 123043091
Test: boot using svelte target
Change-Id: I76b9d6af8a1dd4464cb3cf3e6dc327980efdf361
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Use the LL-NDK library to read cgroup.rc.
As a side-effect, the CgroupController class is changed
to a wrapper of ACgroupController* from the library. The
class itself essentially becomes the pointer, and can be
trivially copied. Modify all client code for this change.
Test: builds and boots
Bug: 123664216
Change-Id: I635d9885db62e82f37421f6d20f59c84a6f4cbb2
Merged-In: I635d9885db62e82f37421f6d20f59c84a6f4cbb2
Only init uses SetupCgroups. This functionality is
moved from libprocessgroup to its own library, and only
init links to it.
Also, merge CgroupSetupCgroups() with CgroupMap::SetupCgroups()
because the former is just an alias of the latter, and
CgroupMap does not belong to libcgrouprc_setup.
Test: boots
Bug: 123664216
Change-Id: I941dc0c415e2b22ae663d43e30dc7a464687325e
Merged-In: I941dc0c415e2b22ae663d43e30dc7a464687325e
EBUSY is expected when removing process group path if process is still
active. ESRCH is expected when kill if process died already. ENOENT is
also expected when opening cgroup path if process died already.
This CL also skip removing parent path if child failed when remove
process group.
Bug: 125340804
Test: Build and boot
Change-Id: Ief3b9cb913035a4050f6cf79c8b1e2f098e18244
The expected memcg path for apps is
/dev/memcg/apps/uid_<uid>/pid_<pid>. Right now we are missing the
"apps" component. Fix it.
Bug: 124776663
Test: Boot on a Go device and does not see error from lmkd anymore.
Change-Id: I0e4c1d8520463fabb171ff4e61479034b6446548
Abstract usage of cgroups into task profiles that allows for changes
in cgroup hierarchy and version without affecting framework codebase.
Rework current processgroup and sched_policy API function implementations
to use task profiles instead of hardcoded paths and attributes.
Mount cgroups using information from cgroups.json rather than from init.rc
Exempt-From-Owner-Approval: already approved in internal master
Bug: 111307099
Test: builds, boots
Change-Id: If5532d6dc570add825cebd5b5148e00c7d688e32
Merged-In: If5532d6dc570add825cebd5b5148e00c7d688e32
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
