Look for phantom processes before setting the process group
Before this change, identification of phantom processes was done only
lazily. AppProfiler, in a background thread (ProcessCpuThread) ran the
identification periodically. At the moment, the period is longer than 3
min.
The identification of phantom processes is a heavy-weight task as it
iterates over "all" app processes in the system to read
/sys/fs/cgroup/uid_<uid>/pid_<pid>/cgroup.procs files. So, not running
it reactively makes much sense.
However, a big downside of this approach is that phantom processes
created in the last 3 min can escape from the adjustements made by
OomAdjuster. Specifically, it can escape from the process group adj.
introduced with I7115ce86d3363e83b1808fb2db5bd2c143bed885. An app can
keep using all CPUs (top-app cpuset) almost indefinitely by creating
some phantom processes while it is interacting with the user (top-app)
and then switches to any other app *in 3 min*.
This change fixes this issue in a somewhat conservative manner. Instead
of making sure that the list of phantom processes are up-to-date all the
time, it tries read the information only for the app process being
considered. In addition, the information is not updated to the phantom
process list, in order to avoid lock contention. We may revisit this in
the future though.
In other words, if we are going to change the process group of app A, we
collect only the phantom processes that belong to A. This is much more
lightweight compared to the full-scale update as it requires a single
read of the cgroup file.
Of course, a single file read is anyway an IO which should be avoided on
hot paths if possible. This change satisfies that because the
identification of the phantom processes and setting their process group
is still done in the handler thread (mProcessGroupHandler in
OomAdjuster).
Bug: 375950473
Bug: 375058190
Flag: com.android.server.am.phantom_processes_fix
Test: start the terminal app, move away from it (by going home for
example), and then execute `cat /proc/$(pgrep crosvm_debian)/cgroup.
Check that its cpuset is changed to FOREGROUND, not TOP-APP. (after
enabling the flag)
Change-Id: Ic333543b90102e112908446bad6c36a48876caa0
diff --git a/services/core/java/com/android/server/am/OomAdjuster.java b/services/core/java/com/android/server/am/OomAdjuster.java
index 08632fe..c067662 100644
--- a/services/core/java/com/android/server/am/OomAdjuster.java
+++ b/services/core/java/com/android/server/am/OomAdjuster.java
@@ -514,27 +514,11 @@
mLogger = new OomAdjusterDebugLogger(this, mService.mConstants);
mProcessGroupHandler = new Handler(adjusterThread.getLooper(), msg -> {
- final int pid = msg.arg1;
- final int group = msg.arg2;
- if (pid == ActivityManagerService.MY_PID) {
- // Skip setting the process group for system_server, keep it as default.
- return true;
- }
- final boolean traceEnabled = Trace.isTagEnabled(Trace.TRACE_TAG_ACTIVITY_MANAGER);
- if (traceEnabled) {
- Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, "setProcessGroup "
- + msg.obj + " to " + group);
- }
- try {
- android.os.Process.setProcessGroup(pid, group);
- } catch (Exception e) {
- if (DEBUG_ALL) {
- Slog.w(TAG, "Failed setting process group of " + pid + " to " + group, e);
- }
- } finally {
- if (traceEnabled) {
- Trace.traceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER);
- }
+ final int group = msg.what;
+ final ProcessRecord app = (ProcessRecord) msg.obj;
+ setProcessGroup(app.getPid(), group, app.processName);
+ if (Flags.phantomProcessesFix()) {
+ mService.mPhantomProcessList.setProcessGroupForPhantomProcessOfApp(app, group);
}
return true;
});
@@ -545,8 +529,31 @@
}
void setProcessGroup(int pid, int group, String processName) {
+ if (pid == ActivityManagerService.MY_PID) {
+ // Skip setting the process group for system_server, keep it as default.
+ return;
+ }
+ final boolean traceEnabled = Trace.isTagEnabled(Trace.TRACE_TAG_ACTIVITY_MANAGER);
+ if (traceEnabled) {
+ Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, "setProcessGroup "
+ + processName + " to " + group);
+ }
+ try {
+ android.os.Process.setProcessGroup(pid, group);
+ } catch (Exception e) {
+ if (DEBUG_ALL) {
+ Slog.w(TAG, "Failed setting process group of " + pid + " to " + group, e);
+ }
+ } finally {
+ if (traceEnabled) {
+ Trace.traceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER);
+ }
+ }
+ }
+
+ void setAppAndChildProcessGroup(ProcessRecord app, int group) {
mProcessGroupHandler.sendMessage(mProcessGroupHandler.obtainMessage(
- 0 /* unused */, pid, group, processName));
+ group, app));
}
void initSettings() {
@@ -3503,8 +3510,7 @@
processGroup = THREAD_GROUP_DEFAULT;
break;
}
- setProcessGroup(app.getPid(), processGroup, app.processName);
- mService.mPhantomProcessList.setProcessGroupForPhantomProcessOfApp(app, processGroup);
+ setAppAndChildProcessGroup(app, processGroup);
try {
final int renderThreadTid = app.getRenderThreadTid();
if (curSchedGroup == SCHED_GROUP_TOP_APP) {
diff --git a/services/core/java/com/android/server/am/PhantomProcessList.java b/services/core/java/com/android/server/am/PhantomProcessList.java
index bfdced7..123780f 100644
--- a/services/core/java/com/android/server/am/PhantomProcessList.java
+++ b/services/core/java/com/android/server/am/PhantomProcessList.java
@@ -548,6 +548,7 @@
*/
void setProcessGroupForPhantomProcessOfApp(final ProcessRecord app, final int group) {
synchronized (mLock) {
+ lookForPhantomProcessesLocked(app);
final SparseArray<PhantomProcessRecord> array = getPhantomProcessOfAppLocked(app);
if (array == null) {
return;
diff --git a/services/core/java/com/android/server/am/flags.aconfig b/services/core/java/com/android/server/am/flags.aconfig
index 7b4d6c7..5d5b35b 100644
--- a/services/core/java/com/android/server/am/flags.aconfig
+++ b/services/core/java/com/android/server/am/flags.aconfig
@@ -250,4 +250,14 @@
is_fixed_read_only: true
description: "Add +X to the prev scores according to their positions in the process LRU list"
bug: "359912586"
-}
\ No newline at end of file
+}
+
+flag {
+ name: "phantom_processes_fix"
+ namespace: "backstage_power"
+ description: "Make sure setProcessGroupForPhantomProcessOfApp deals with phantom processes properly"
+ bug: "375058190"
+ metadata {
+ purpose: PURPOSE_BUGFIX
+ }
+}