diff options
| author | 2017-03-13 14:53:11 -0600 | |
|---|---|---|
| committer | 2017-03-13 15:38:25 -0600 | |
| commit | 5f3e93451e87d72c513e75c5d5459a4bd2cc41b2 (patch) | |
| tree | 1afb1dc9913cf233b8bd0e9f885c63f776733b9e | |
| parent | a5af24cd02f9dadbea7d23c59ce5f04ce045080d (diff) | |
Lower-overhead version of LockGuard.
Instead of building up a giant set of all locks inside the system
server, there are only a handful that we're interested in watching:
specifically those below the ActivityManagerService lock.
This change adds a index-based variant of lock registering and
checking, which has much lower overhead that doesn't bring a device
to its knees. It's disabled by default, but can be enabled on local
development builds.
Currently uses the boostPriorityForLockedSection() hook in AMS to
check for lock inversions when enabled.
Test: builds, boots, no AMS lock inversions detected
Bug: 35410906
Change-Id: I473d634d23c08538029412a1254bf4f92b96fb84
8 files changed, 77 insertions, 4 deletions
diff --git a/services/core/java/com/android/server/AppOpsService.java b/services/core/java/com/android/server/AppOpsService.java index de11f36499c4..a5e357ca91f8 100644 --- a/services/core/java/com/android/server/AppOpsService.java +++ b/services/core/java/com/android/server/AppOpsService.java @@ -235,6 +235,7 @@ public class AppOpsService extends IAppOpsService.Stub { } public AppOpsService(File storagePath, Handler handler) { + LockGuard.installLock(this, LockGuard.INDEX_APP_OPS); mFile = new AtomicFile(storagePath); mHandler = handler; readState(); diff --git a/services/core/java/com/android/server/LockGuard.java b/services/core/java/com/android/server/LockGuard.java index 3a381ae8bf7c..b7449173be07 100644 --- a/services/core/java/com/android/server/LockGuard.java +++ b/services/core/java/com/android/server/LockGuard.java @@ -53,10 +53,29 @@ import java.io.PrintWriter; * <li>A guarded synchronized block takes 50ns when disabled. * <li>A guarded synchronized block takes 460ns per lock checked when enabled. * </ul> + * <p> + * This class also supports a second simpler mode of operation where well-known + * locks are explicitly registered and checked via indexes. */ public class LockGuard { private static final String TAG = "LockGuard"; + public static final boolean ENABLED = false; + + /** + * Well-known locks ordered by fixed index. Locks with a specific index + * should never be acquired while holding a lock of a lower index. + */ + public static final int INDEX_APP_OPS = 0; + public static final int INDEX_POWER = 1; + public static final int INDEX_USER = 2; + public static final int INDEX_PACKAGES = 3; + public static final int INDEX_STORAGE = 4; + public static final int INDEX_WINDOW = 5; + public static final int INDEX_ACTIVITY = 6; + + private static Object[] sKnownFixed = new Object[INDEX_ACTIVITY + 1]; + private static ArrayMap<Object, LockInfo> sKnown = new ArrayMap<>(0, true); private static class LockInfo { @@ -119,11 +138,41 @@ public class LockGuard { } /** + * Yell if any lower-level locks are being held by the calling thread that + * is about to acquire the given lock. + */ + public static void guard(int index) { + for (int i = 0; i < index; i++) { + final Object lock = sKnownFixed[i]; + if (lock != null && Thread.holdsLock(lock)) { + Slog.w(TAG, "Calling thread " + Thread.currentThread().getName() + " is holding " + + lockToString(i) + " while trying to acquire " + + lockToString(index), new Throwable()); + } + } + } + + /** * Report the given lock with a well-known label. */ - public static void installLock(Object lock, String label) { + public static Object installLock(Object lock, String label) { final LockInfo info = findOrCreateLockInfo(lock); info.label = label; + return lock; + } + + /** + * Report the given lock with a well-known index. + */ + public static Object installLock(Object lock, int index) { + sKnownFixed[index] = lock; + return lock; + } + + public static Object installNewLock(int index) { + final Object lock = new Object(); + installLock(lock, index); + return lock; } private static String lockToString(Object lock) { @@ -135,6 +184,19 @@ public class LockGuard { } } + private static String lockToString(int index) { + switch (index) { + case INDEX_APP_OPS: return "APP_OPS"; + case INDEX_POWER: return "POWER"; + case INDEX_USER: return "USER"; + case INDEX_PACKAGES: return "PACKAGES"; + case INDEX_STORAGE: return "STORAGE"; + case INDEX_WINDOW: return "WINDOW"; + case INDEX_ACTIVITY: return "ACTIVITY"; + default: return Integer.toString(index); + } + } + public static void dump(FileDescriptor fd, PrintWriter pw, String[] args) { for (int i = 0; i < sKnown.size(); i++) { final Object lock = sKnown.keyAt(i); diff --git a/services/core/java/com/android/server/StorageManagerService.java b/services/core/java/com/android/server/StorageManagerService.java index 32136bbf6a95..85a1a1c98cfe 100644 --- a/services/core/java/com/android/server/StorageManagerService.java +++ b/services/core/java/com/android/server/StorageManagerService.java @@ -308,7 +308,7 @@ class StorageManagerService extends IStorageManager.Stub * <em>Never</em> hold the lock while performing downcalls into vold, since * unsolicited events can suddenly appear to update data structures. */ - private final Object mLock = new Object(); + private final Object mLock = LockGuard.installNewLock(LockGuard.INDEX_STORAGE); /** Set of users that we know are unlocked. */ @GuardedBy("mLock") diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index fef7b513b7bc..9079b76cd4f2 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -691,6 +691,9 @@ public class ActivityManagerService extends IActivityManager.Stub Process.setThreadPriority(tid, -2); } state.regionCounter++; + if (LockGuard.ENABLED) { + LockGuard.guard(LockGuard.INDEX_ACTIVITY); + } } static void resetPriorityAfterLockedSection() { @@ -2657,6 +2660,7 @@ public class ActivityManagerService extends IActivityManager.Stub // Note: This method is invoked on the main thread but may need to attach various // handlers to other threads. So take care to be explicit about the looper. public ActivityManagerService(Context systemContext) { + LockGuard.installLock(this, LockGuard.INDEX_ACTIVITY); mContext = systemContext; mFactoryTest = FactoryTest.getMode(); mSystemThread = ActivityThread.currentActivityThread(); diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index e8af3108011b..c27806d80ac9 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -266,6 +266,7 @@ import com.android.server.EventLogTags; import com.android.server.FgThread; import com.android.server.IntentResolver; import com.android.server.LocalServices; +import com.android.server.LockGuard; import com.android.server.ServiceThread; import com.android.server.SystemConfig; import com.android.server.SystemServerInitThreadPool; @@ -2212,6 +2213,7 @@ public class PackageManagerService extends IPackageManager.Stub { public PackageManagerService(Context context, Installer installer, boolean factoryTest, boolean onlyCore) { + LockGuard.installLock(mPackages, LockGuard.INDEX_PACKAGES); Trace.traceBegin(TRACE_TAG_PACKAGE_MANAGER, "create package manager"); EventLog.writeEvent(EventLogTags.BOOT_PROGRESS_PMS_START, SystemClock.uptimeMillis()); diff --git a/services/core/java/com/android/server/pm/UserManagerService.java b/services/core/java/com/android/server/pm/UserManagerService.java index 627fa545bf58..b9fcf4e39203 100644 --- a/services/core/java/com/android/server/pm/UserManagerService.java +++ b/services/core/java/com/android/server/pm/UserManagerService.java @@ -94,6 +94,7 @@ import com.android.internal.util.Preconditions; import com.android.internal.util.XmlUtils; import com.android.internal.widget.LockPatternUtils; import com.android.server.LocalServices; +import com.android.server.LockGuard; import com.android.server.SystemService; import com.android.server.am.UserState; import com.android.server.storage.DeviceStorageMonitorInternal; @@ -227,7 +228,7 @@ public class UserManagerService extends IUserManager.Stub { private final Object mPackagesLock; private final UserDataPreparer mUserDataPreparer; // Short-term lock for internal state, when interaction/sync with PM is not required - private final Object mUsersLock = new Object(); + private final Object mUsersLock = LockGuard.installNewLock(LockGuard.INDEX_USER); private final Object mRestrictionsLock = new Object(); private final Handler mHandler; diff --git a/services/core/java/com/android/server/power/PowerManagerService.java b/services/core/java/com/android/server/power/PowerManagerService.java index 24f6f896325b..4f67e8ce2d15 100644 --- a/services/core/java/com/android/server/power/PowerManagerService.java +++ b/services/core/java/com/android/server/power/PowerManagerService.java @@ -76,6 +76,7 @@ import com.android.internal.hardware.AmbientDisplayConfiguration; import com.android.internal.os.BackgroundThread; import com.android.internal.util.ArrayUtils; import com.android.server.EventLogTags; +import com.android.server.LockGuard; import com.android.server.RescueParty; import com.android.server.ServiceThread; import com.android.server.SystemService; @@ -210,7 +211,7 @@ public final class PowerManagerService extends SystemService private DreamManagerInternal mDreamManager; private Light mAttentionLight; - private final Object mLock = new Object(); + private final Object mLock = LockGuard.installNewLock(LockGuard.INDEX_POWER); // A bitfield that indicates what parts of the power state have // changed and need to be recalculated. diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java index c0cc923ad831..9527e6b1b6eb 100644 --- a/services/core/java/com/android/server/wm/WindowManagerService.java +++ b/services/core/java/com/android/server/wm/WindowManagerService.java @@ -222,6 +222,7 @@ import com.android.server.DisplayThread; import com.android.server.EventLogTags; import com.android.server.FgThread; import com.android.server.LocalServices; +import com.android.server.LockGuard; import com.android.server.UiThread; import com.android.server.Watchdog; import com.android.server.input.InputManagerService; @@ -952,6 +953,7 @@ public class WindowManagerService extends IWindowManager.Stub private WindowManagerService(Context context, InputManagerService inputManager, boolean haveInputMethods, boolean showBootMsgs, boolean onlyCore, WindowManagerPolicy policy) { + LockGuard.installLock(this, LockGuard.INDEX_WINDOW); mRoot = new RootWindowContainer(this); mContext = context; mHaveInputMethods = haveInputMethods; |