From 5b55587f5bc298cbf9f5b140a1fe06b410cbb51b Mon Sep 17 00:00:00 2001 From: Li Li Date: Thu, 10 Nov 2022 09:59:24 -0800 Subject: [PATCH] Freezer: improve file lock exemption When a process becomes cached and holds file locks blocking other processes, check whether those blocked processes are also in cached mode. If they are also cached, freeze the current process. Otherwise, Skip the current process to avoid deadlock. Bug: 245994713 Bug: 253913470 Test: atest ProcLocksReaderTest Test: verify apps with deadlock can be frozen Change-Id: I8635bf877bbe4404cc66955f1946444265680246 --- .../android/internal/os/ProcLocksReader.java | 65 +++++++++++++----- .../internal/os/ProcLocksReaderTest.java | 68 +++++++++++-------- .../android/server/am/CachedAppOptimizer.java | 22 ++++-- 3 files changed, 106 insertions(+), 49 deletions(-) diff --git a/core/java/com/android/internal/os/ProcLocksReader.java b/core/java/com/android/internal/os/ProcLocksReader.java index 2143bc1b45325..9ddb8c75f5c81 100644 --- a/core/java/com/android/internal/os/ProcLocksReader.java +++ b/core/java/com/android/internal/os/ProcLocksReader.java @@ -16,6 +16,8 @@ package com.android.internal.os; +import android.util.IntArray; + import com.android.internal.util.ProcFileReader; import java.io.FileInputStream; @@ -35,6 +37,7 @@ import java.io.IOException; public class ProcLocksReader { private final String mPath; private ProcFileReader mReader = null; + private IntArray mPids = new IntArray(); public ProcLocksReader() { mPath = "/proc/locks"; @@ -51,9 +54,13 @@ public class ProcLocksReader { public interface ProcLocksReaderCallback { /** * Call the callback function of handleBlockingFileLocks(). - * @param pid Each process that hold file locks blocking other processes. + * @param pids Each process that hold file locks blocking other processes. + * pids[0] is the process blocking others + * pids[1..n-1] are the processes being blocked + * NOTE: pids are cleared immediately after onBlockingFileLock() returns. If the caller + * needs to cache it, please make a copy, e.g. by calling pids.toArray(). */ - void onBlockingFileLock(int pid); + void onBlockingFileLock(IntArray pids); } /** @@ -64,8 +71,7 @@ public class ProcLocksReader { public void handleBlockingFileLocks(ProcLocksReaderCallback callback) throws IOException { long last = -1; long id; // ordinal position of the lock in the list - int owner = -1; // the PID of the process that owns the lock - int pid = -1; // the PID of the process blocking others + int pid = -1; // the PID of the process being blocked if (mReader == null) { mReader = new ProcFileReader(new FileInputStream(mPath)); @@ -73,26 +79,49 @@ public class ProcLocksReader { mReader.rewind(); } + mPids.clear(); while (mReader.hasMoreData()) { id = mReader.nextLong(true); // lock id if (id == last) { - mReader.finishLine(); // blocked lock - if (pid < 0) { - pid = owner; // get pid from the previous line - callback.onBlockingFileLock(pid); + // blocked lock found + mReader.nextIgnored(); // -> + mReader.nextIgnored(); // lock type: POSIX? + mReader.nextIgnored(); // lock type: MANDATORY? + mReader.nextIgnored(); // lock type: RW? + + pid = mReader.nextInt(); // pid + if (pid > 0) { + mPids.add(pid); } - continue; + + mReader.finishLine(); } else { - pid = -1; // a new lock + // process blocking lock and move on to a new lock + if (mPids.size() > 1) { + callback.onBlockingFileLock(mPids); + mPids.clear(); + } + + // new lock found + mReader.nextIgnored(); // lock type: POSIX? + mReader.nextIgnored(); // lock type: MANDATORY? + mReader.nextIgnored(); // lock type: RW? + + pid = mReader.nextInt(); // pid + if (pid > 0) { + if (mPids.size() == 0) { + mPids.add(pid); + } else { + mPids.set(0, pid); + } + } + mReader.finishLine(); + last = id; } - - mReader.nextIgnored(); // lock type: POSIX? - mReader.nextIgnored(); // lock type: MANDATORY? - mReader.nextIgnored(); // lock type: RW? - - owner = mReader.nextInt(); // pid - mReader.finishLine(); - last = id; + } + // The last unprocessed blocking lock immediately before EOF + if (mPids.size() > 1) { + callback.onBlockingFileLock(mPids); } } } diff --git a/core/tests/coretests/src/com/android/internal/os/ProcLocksReaderTest.java b/core/tests/coretests/src/com/android/internal/os/ProcLocksReaderTest.java index b34554cd113db..c3d40eb092377 100644 --- a/core/tests/coretests/src/com/android/internal/os/ProcLocksReaderTest.java +++ b/core/tests/coretests/src/com/android/internal/os/ProcLocksReaderTest.java @@ -20,6 +20,7 @@ import static org.junit.Assert.assertTrue; import android.content.Context; import android.os.FileUtils; +import android.util.IntArray; import androidx.test.InstrumentationRegistry; import androidx.test.filters.SmallTest; @@ -33,13 +34,15 @@ import org.junit.runner.RunWith; import java.io.File; import java.nio.file.Files; import java.util.ArrayList; +import java.util.Arrays; @SmallTest @RunWith(AndroidJUnit4.class) public class ProcLocksReaderTest implements ProcLocksReader.ProcLocksReaderCallback { private File mProcDirectory; - private ArrayList mPids = new ArrayList<>(); + + private ArrayList mPids = new ArrayList<>(); @Before public void setUp() { @@ -54,41 +57,51 @@ public class ProcLocksReaderTest implements @Test public void testRunSimpleLocks() throws Exception { - String simpleLocks = - "1: POSIX ADVISORY READ 18403 fd:09:9070 1073741826 1073742335\n" + - "2: POSIX ADVISORY WRITE 18292 fd:09:34062 0 EOF\n"; + String simpleLocks = "1: POSIX ADVISORY READ 18403 fd:09:9070 1073741826 1073742335\n" + + "2: POSIX ADVISORY WRITE 18292 fd:09:34062 0 EOF\n"; runHandleBlockingFileLocks(simpleLocks); assertTrue(mPids.isEmpty()); } @Test public void testRunBlockingLocks() throws Exception { - String blockedLocks = - "1: POSIX ADVISORY READ 18403 fd:09:9070 1073741826 1073742335\n" + - "2: POSIX ADVISORY WRITE 18292 fd:09:34062 0 EOF\n" + - "2: -> POSIX ADVISORY WRITE 18291 fd:09:34062 0 EOF\n" + - "2: -> POSIX ADVISORY WRITE 18293 fd:09:34062 0 EOF\n" + - "3: POSIX ADVISORY READ 3888 fd:09:13992 128 128\n" + - "4: POSIX ADVISORY READ 3888 fd:09:14230 1073741826 1073742335\n"; + String blockedLocks = "1: POSIX ADVISORY READ 18403 fd:09:9070 1073741826 1073742335\n" + + "2: POSIX ADVISORY WRITE 18292 fd:09:34062 0 EOF\n" + + "2: -> POSIX ADVISORY WRITE 18291 fd:09:34062 0 EOF\n" + + "2: -> POSIX ADVISORY WRITE 18293 fd:09:34062 0 EOF\n" + + "3: POSIX ADVISORY READ 3888 fd:09:13992 128 128\n" + + "4: POSIX ADVISORY READ 3888 fd:09:14230 1073741826 1073742335\n"; runHandleBlockingFileLocks(blockedLocks); - assertTrue(mPids.remove(0).equals(18292)); + assertTrue(Arrays.equals(mPids.remove(0), new int[]{18292, 18291, 18293})); + assertTrue(mPids.isEmpty()); + } + + @Test + public void testRunLastBlockingLocks() throws Exception { + String blockedLocks = "1: POSIX ADVISORY READ 18403 fd:09:9070 1073741826 1073742335\n" + + "2: POSIX ADVISORY WRITE 18292 fd:09:34062 0 EOF\n" + + "2: -> POSIX ADVISORY WRITE 18291 fd:09:34062 0 EOF\n" + + "2: -> POSIX ADVISORY WRITE 18293 fd:09:34062 0 EOF\n"; + runHandleBlockingFileLocks(blockedLocks); + assertTrue(Arrays.equals(mPids.remove(0), new int[]{18292, 18291, 18293})); assertTrue(mPids.isEmpty()); } @Test public void testRunMultipleBlockingLocks() throws Exception { - String blockedLocks = - "1: POSIX ADVISORY READ 18403 fd:09:9070 1073741826 1073742335\n" + - "2: POSIX ADVISORY WRITE 18292 fd:09:34062 0 EOF\n" + - "2: -> POSIX ADVISORY WRITE 18291 fd:09:34062 0 EOF\n" + - "2: -> POSIX ADVISORY WRITE 18293 fd:09:34062 0 EOF\n" + - "3: POSIX ADVISORY READ 3888 fd:09:13992 128 128\n" + - "4: FLOCK ADVISORY WRITE 3840 fe:01:5111809 0 EOF\n" + - "4: -> FLOCK ADVISORY WRITE 3841 fe:01:5111809 0 EOF\n" + - "5: POSIX ADVISORY READ 3888 fd:09:14230 1073741826 1073742335\n"; + String blockedLocks = "1: POSIX ADVISORY READ 18403 fd:09:9070 1073741826 1073742335\n" + + "2: POSIX ADVISORY WRITE 18292 fd:09:34062 0 EOF\n" + + "2: -> POSIX ADVISORY WRITE 18291 fd:09:34062 0 EOF\n" + + "2: -> POSIX ADVISORY WRITE 18293 fd:09:34062 0 EOF\n" + + "3: POSIX ADVISORY READ 3888 fd:09:13992 128 128\n" + + "4: FLOCK ADVISORY WRITE 3840 fe:01:5111809 0 EOF\n" + + "4: -> FLOCK ADVISORY WRITE 3841 fe:01:5111809 0 EOF\n" + + "5: FLOCK ADVISORY READ 3888 fd:09:14230 0 EOF\n" + + "5: -> FLOCK ADVISORY READ 3887 fd:09:14230 0 EOF\n"; runHandleBlockingFileLocks(blockedLocks); - assertTrue(mPids.remove(0).equals(18292)); - assertTrue(mPids.remove(0).equals(3840)); + assertTrue(Arrays.equals(mPids.remove(0), new int[]{18292, 18291, 18293})); + assertTrue(Arrays.equals(mPids.remove(0), new int[]{3840, 3841})); + assertTrue(Arrays.equals(mPids.remove(0), new int[]{3888, 3887})); assertTrue(mPids.isEmpty()); } @@ -102,11 +115,12 @@ public class ProcLocksReaderTest implements /** * Call the callback function of handleBlockingFileLocks(). - * - * @param pid Each process that hold file locks blocking other processes. + * @param pids Each process that hold file locks blocking other processes. + * pids[0] is the process blocking others + * pids[1..n-1] are the processes being blocked */ @Override - public void onBlockingFileLock(int pid) { - mPids.add(pid); + public void onBlockingFileLock(IntArray pids) { + mPids.add(pids.toArray()); } } diff --git a/services/core/java/com/android/server/am/CachedAppOptimizer.java b/services/core/java/com/android/server/am/CachedAppOptimizer.java index 2d7b0dc7b536d..7edaeaa791b99 100644 --- a/services/core/java/com/android/server/am/CachedAppOptimizer.java +++ b/services/core/java/com/android/server/am/CachedAppOptimizer.java @@ -37,6 +37,7 @@ import android.provider.DeviceConfig.Properties; import android.provider.Settings; import android.text.TextUtils; import android.util.EventLog; +import android.util.IntArray; import android.util.Pair; import android.util.Slog; import android.util.SparseArray; @@ -2110,15 +2111,28 @@ public final class CachedAppOptimizer { @GuardedBy({"mAm"}) @Override - public void onBlockingFileLock(int pid) { + public void onBlockingFileLock(IntArray pids) { if (DEBUG_FREEZER) { - Slog.d(TAG_AM, "Process (pid=" + pid + ") holds blocking file lock"); + Slog.d(TAG_AM, "Blocking file lock found: " + pids); } synchronized (mProcLock) { + int pid = pids.get(0); ProcessRecord app = mFrozenProcesses.get(pid); + ProcessRecord pr; if (app != null) { - Slog.i(TAG_AM, app.processName + " (" + pid + ") holds blocking file lock"); - unfreezeAppLSP(app, OomAdjuster.OOM_ADJ_REASON_NONE); + for (int i = 1; i < pids.size(); i++) { + int blocked = pids.get(i); + synchronized (mAm.mPidsSelfLocked) { + pr = mAm.mPidsSelfLocked.get(blocked); + } + if (pr != null && pr.mState.getCurAdj() < ProcessList.CACHED_APP_MIN_ADJ) { + Slog.d(TAG_AM, app.processName + " (" + pid + ") blocks " + + pr.processName + " (" + blocked + ")"); + // Found at least one blocked non-cached process + unfreezeAppLSP(app, OomAdjuster.OOM_ADJ_REASON_NONE); + break; + } + } } } }