A JoinCgroup action for a v2 cgroup controller migrates a process or task
from the uid_%d/pid_%d cgroup into another cgroup, e.g. the root cgroup.
This may make services unkillable because Service::Stop() only stops a
service if the uid_%d/pid_%d cgroup still exists when Service::Stop() is
called.
Bug: 309674654
Change-Id: I20b797afdf596125ff5a6ed41cb33fe59b84ac88
Signed-off-by: Bart Van Assche <bvanassche@google.com>
For log messages like the following it is not possible to derive why
this message has been logged:
E libprocessgroup: AddTidToCgroup failed to write '3949'; fd=55: Operation not supported on transport endpoint
Hence include the cgroup path and the tid type in the log message.
Change-Id: I057711fe576b82f6454456b7284186ddeece33c3
Signed-off-by: Bart Van Assche <bvanassche@google.com>
Current implementation of SetAttributeAction::ExecuteForProcess reuses
SetAttributeAction::ExecuteForTask while not utilizing available uid/pid
information. This results in a call to GetPathForTask() which is an
expensive function due to it reading and parsing /proc/$pid/cgroups.
This can be avoided if we utilize available uid/pid info and the fact
that cgroup v2 attributes share the cgroup v2 hierarchy as process
groups, which use a known path template.
Bug: 292636609
Change-Id: I02e3046bd85d0dfebc68ab444f1796bb54cc69c7
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
ProfileAttribute::Reset does not reset file_v2_name, fix that. Also
provide ProfileAttribute::file_name() to consolidate the code.
Bug: 292636609
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Change-Id: I5b33ca47b4fa5cabf582c8804bd13f72f6e58411
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: Ibb52ba2503e30e2f20770b7d23629167e38d076a
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 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
PLOG reports the value of errno. These four PLOG statements are
after functions that are not syscalls, leading to confusing logs
such as "Failed to apply Foo task profile: Success".
Bug: 271196526
Test: N/A
Change-Id: Iede5274d1ceebabec8432527112291ba63dca090
Change two PLOG() statements into LOG() statements since PLOG() should
only be used if errno has been set. Make it easier to find the code that
logs an error message.
Bug: 213617178
Signed-off-by: Bart Van Assche <bvanassche@google.com>
Change-Id: I73443f3adb7d7ba3fc0d39a55777f0b132529fbd
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>
Do not force callers to create an std::string object. This patch
implements the following advice from go/totw/1: "Google’s preferred
option for accepting such string parameters is through a string_view."
Use std::less<> as comparison type to prevent that std::string_view
objects have to be converted into std::string objects for lookups in
std::map<>.
Bug: 213617178
Change-Id: I08125a02220a8c003d9202a7e177be776c3b9829
Signed-off-by: Bart Van Assche <bvanassche@google.com>
Since it is nontrivial which modifies `errno` is modified in
ExecuteForTask(), add a comment that explains this.
Bug: 213617178
Test: Compile-tested only.
Change-Id: I49ce9c8054fdc59e61b2e5f9ffe6f16743a94401
Signed-off-by: Bart Van Assche <bvanassche@google.com>
The Linux kernel returns error code EACCES (Permission denied) if either
a cgroup attribute does not exist or if the process that is trying to
write into a cgroup attribute does not have permission to write. In
other words, it is not possible to tell the difference between these two
scenarios by checking the value of 'errno'. Hence this patch that adds a
stat() call to check whether or not a cgroup attribute exists. This
patch makes lines like the following disappear from logcat for optional
cgroup attributes:
03-22 23:16:04.816 616 649 E libprocessgroup: Failed to write '0' to /sys/fs/cgroup/./uid_10077/p
id_1695/io.group_idle: Permission denied
Bug: 213617178
Test: Booted Android in Cuttlefish and inspected logcat.
Change-Id: I6e041dfc34a52c9bdb75a8c70d99ad79b06eee06
Signed-off-by: Bart Van Assche <bvanassche@google.com>
Some but not all memcg v1 attributes exist in the v2 hierarchy. The
following information comes from Shakeel Butt and Kamil Yurtsever,
"cgroup v2 migration at Google", Linux Plumbers Conference 2018
( https://lpc.events/event/2/contributions/204/attachments/143/378/LPC2018-cgroup-v2.pdf ):
v1 v2
-------------------------- ----------
memory.limit_in_bytes memory.max
memory.soft_limit_in_bytes memory.low
Add support for cgroup attributes that have different names in the v1
and v2 hierarchies. Also add those memcg attributes that are used in
lmkd.
Bug: 213617178
Test: Tested lmkd with v1 and v2 memcg.
Change-Id: Ia5c5f02974f3b783d5cdaad85b33169ced8db55f
Signed-off-by: Bart Van Assche <bvanassche@google.com>
Cgroup attributes like io.weight or io.bfq.weight only exist if the
corresponding I/O scheduler has been loaded. Hence add support for
ignoring failures to write to a cgroup attribute that does not exist.
Bug: 213617178
Test: This patch has been tested in combination with patch "Migrate the blkio controller to the cgroup v2 hierarchy".
Change-Id: I5debafabc4dd430a1b98ea343bf3740c764edcc3
Signed-off-by: Bart Van Assche <bvanassche@google.com>
Currently, there is no distinguish between SetTaskProfiles and
SetProcessProfiles in WriteFileAction because they use the same task
path. Add ProcFilePath attribute so that WriteFileAction could use
a separate path for process in SetProcessProfiles.
Bug: 218684257
Test: function works
Change-Id: I004b8c8799240c54f1ef1fed0566fa5057341cf4
Add support for querying the task profile and action names at runtime.
Log more information with verbose logging enabled. The additional
logging statements make it easier to verify whether changes in
task_profiles.json work as expected.
Bug: 213617178
Test: Booted Android in Cuttlefish.
Change-Id: I175e46d142bb015732b135952fdbeb9986f20ee3
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>
Unify the Darwin and Linux class declarations. Move all #ifdefs for this
class into the .cpp file. This patch does not change any functionality.
Bug: 213617178
Test: Booted Android in Cuttlefish.
Change-Id: I6a3cedc75c2dbf8aa062edda76cae9959767a763
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
Refactor file descriptor caching code and move it into FdCacheHelper
because later on when we introduce fd caching for SetProcessProfiles
the children of CachedFdProfileAction become different enough that
sharing the same parent becomes a hindrance.
Bug: 215557553
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Change-Id: If3812a0090c81a29e25f0888b0511cfaf48edea3
Using WriteStringToFile will hold kernfs_mutex which is a big lock,
so use WriteStringToFd instead. Besides, also support fd cache for it.
Bug: 206970384
Test: build pass
Change-Id: Id79f9e1095f52079393c58edb9a4d526f4cc6b5e
UserLifecycleTests test disables all Little cores in the course of the
test, which causes attempts to add a process into /dev/cpuset/restricted
cpuset cgroup to fail with ENOSPC error code, indicating that a process
is joining a cpuset cgroup with no online cpus. Current libprocessgroup
implementation will log an error on each such occurrence, which spams
the logs and makes it hard to analyze test results. Because this
situation does not happen in production environment (we do not offline
cpus), we can prevent flooding the logs by identifying this case,
logging an appropriate error one time and ignore all later similar errors.
Bug: 158766131
Test: adb shell "echo 0 > /sys/devices/system/cpu/cpu[0-3]/online"
Test: start some apps, observe libprocessgroup errors in the logcat
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Change-Id: Ia91d8839d86787569c255481bde077be51c43d93
The inner profile should print error message if needed.
Bug: 178070122
Test: Build
Signed-off-by: Wei Wang <wvw@google.com>
Change-Id: I2d9d54e21fb064678feccdbae453a398806d51d6
If there are known fail pattern, we could disable printing logs to
reduce log spam. Use "LogFailures": true or "LogFailures": false.
Default will be true if not specified.
Bug: 178070122
Test: function works
Change-Id: Ia4081a10daf738294937f459f006fbcc3171481b
Current per-API level task profiles replace the latest task profiles
rather than overriding the diffs. This poses an issue when a new
feature is added which is applicable to older devices as well we
have to change all per-API level profiles. Change the per-API task
profile support to load the latest task profiles first and then override
with per-API task profiles, similar to how vendor task profiles override
system task profiles. This minimizes the changes we need to do to the
per-API level task profiles going forward and is more consistent with
how vendor task profiles work.
Bug: 170507876
Test: boot with per-API task profiles
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Change-Id: I08cc4b374cede7363cf96a30ac96971d49bb7847
When task profiles changes happen, devices released under older API
levels might have to use the same profiles as before. This might be
due to for missing kernel features or some other reasons. Add support
for per-API task profiles to accommodate this scenario. With this
mechanism when ro.product.first_api_level is non-empty, the system
looks for /system/etc/profiles/task_profiles_<api level>.json file
and uses it if it exists. If ro.product.first_api_level is not defined
or per-API task profiles file does not exist the system falls back to
the default /system/etc/profiles/task_profiles.json file.
As before, these task profiles can be competely overwritten using
/vendor/etc/task_profiles.json if needed.
Bug: 172066799
Test: boot with per-API task profiles
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Change-Id: I1671b341b3a0fb5e99d0b0788315c61088326b09
There may be some use cases that it needs to write string to a
file. This patch support write "Value" to "FilePath", where
both parameters could use special string "<uid>" and "<pid>" to
represent actual uid and pid.
Bug: 170507963
Test: function works
Change-Id: I543846f523518a9bcb3dd1b3437163a1b3157d95
In the current implementation vendor profile attributes do not override
system ones and instead generate a warning. Fix that by overriding existing
attribute if a new definition is found.
Bug: 160318642
Test: add vendor attributes and confirm no warnings
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Change-Id: I71a2ee4d4b3c585e7c9a01b791e973390d409cbc
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>
Currently when vendor overrides a profile the profile object is being
replaced with a new one. However the old profile might have been
referenced by an aggregate profile and with such profile replacement
the aggregate profile is left referencing a stale object. Fix this by
replacing the content of the old profile with the content from the new
one instead of replacing the object itself.
Bug: 148311066
Test: override profiles referenced in aggregate profile and verify
Test: correct replacement
Change-Id: Iabddbf3580455e5263fedad6665cf52fb323e50a
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
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
while enable fdsan (file descriptor sanitizer),
fdsan report use-after-close error after boot complete (sedom).
Because, in SetCgroupAction::EnableResourceCaching() currently has a data race against all the
use fd_ functions like SetCgroupAction::ExecuteForProcess(uid_t uid, pid_t pid) etc.
ThreadA | ThreadB
-------------------------------------------------------------------------------------------------
in SetCgroupAction::EnableResourceCaching() | in SetCgroupAction::ExecuteForProcess(...)
-------------------------------------------------------------------------------------------------
| in SetCgroupAction::AddTidToCgroup(int tid, int fd)
-------------------------------------------------------------------------------------------------
fd_ = std::move(fd); /*modified fd_ value*/ |
-------------------------------------------------------------------------------------------------
| write(fd) /* crash here, fd is closed by ThreadA*/
-------------------------------------------------------------------------------------------------
So, add mutex lock to protect fd_ data race.
Bug: 134120826
Test: auto test, run the adb reboot test 100 times and no fdsan error report on libprocessgroup
Change-Id: Iccf2f705e030f79324f1164509e715dc5be825de
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
This avoids a race between destruction on process exit and concurrent
access from another thread.
Bug: 129023204
Change-Id: Iecafa67a78e2cce53687156e7f91547b4a23ec13
Convert Json::Value to const refs to prevent creation of a null member
if specified element does not exist. Fix variable naming in the parser
while we are at it.
Change-Id: I7dabfd1c73983b465984dd6c411c9fe48743a348
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
