Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5f3e9345 authored by Jeff Sharkey's avatar Jeff Sharkey
Browse files

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
parent a5af24cd

services/core/java/com/android/server/AppOpsService.java

+1 −0
Original line number Diff line number Diff line
@@ -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();

services/core/java/com/android/server/LockGuard.java

+63 −1
Original line number Diff line number Diff line
@@ -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 {
@@ -118,12 +137,42 @@ public class LockGuard { 
        return lock;

    }



    /**

     * 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);

services/core/java/com/android/server/StorageManagerService.java

+1 −1
Original line number Diff line number Diff line
@@ -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")

services/core/java/com/android/server/am/ActivityManagerService.java

+4 −0
Original line number Diff line number Diff line
@@ -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();

































services/core/java/com/android/server/pm/PackageManagerService.java



+2
−0




























Original line number
Diff line number
Diff line








@@ -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());