Ability to choose either suspend counter or /sys/power/wake_lock.

Suspend blocker that are kept track of by SystemSuspend can now be
backed by either a ref-counter OR /sys/power/wake_lock interface.

We need /sys/power/wake_[un]lock to export debugging info via
/sys/kernel/debug/wakeup_sources. We do this to preserve the workflow
with existing toolchains.

In the future, we want to keep all native wake lock debugging info in
SystemSuspend. That info will then be exposed directly to our tools.

Debug info in /d/wakeup_sources might be misaligned if there wake locks
with colliding names on the device. There are no such cases in Android
platform or pixel devices.

Bug: 128923994
Test: SystemSuspendV1_0UnitTest
Test: device suspends if left alone
Test: /d/wakeup_sources keeps track of all native wake locks
Change-Id: I7d15fca42efca945122ec0671f424e75e186f5d8

diff --git a/suspend/1.0/default/SystemSuspend.cpp b/suspend/1.0/default/SystemSuspend.cpp
index 1cbce94..a87ce84 100644 (file)
--- a/suspend/1.0/default/SystemSuspend.cpp
+++ b/suspend/1.0/default/SystemSuspend.cpp
@@ -42,6 +42,10 @@ namespace suspend {
 namespace V1_0 {
 
 static const char kSleepState[] = "mem";
+// TODO(b/128923994): we only need /sys/power/wake_[un]lock to export debugging info via
+// /sys/kernel/debug/wakeup_sources.
+static constexpr char kSysPowerWakeLock[] = "/sys/power/wake_lock";
+static constexpr char kSysPowerWakeUnlock[] = "/sys/power/wake_unlock";
 
 // This function assumes that data in fd is small enough that it can be read in one go.
 // We use this function instead of the ones available in libbase because it doesn't block
@@ -67,9 +71,9 @@ TimestampType getEpochTimeNow() {
     return std::chrono::duration_cast<std::chrono::microseconds>(timeSinceEpoch).count();
 }
 
-WakeLock::WakeLock(SystemSuspend* systemSuspend, const WakeLockIdType& id)
-    : mReleased(), mSystemSuspend(systemSuspend), mId(id) {
-    mSystemSuspend->incSuspendCounter();
+WakeLock::WakeLock(SystemSuspend* systemSuspend, const WakeLockIdType& id, const string& name)
+    : mReleased(), mSystemSuspend(systemSuspend), mId(id), mName(name) {
+    mSystemSuspend->incSuspendCounter(mName);
 }
 
 WakeLock::~WakeLock() {
@@ -83,22 +87,37 @@ Return<void> WakeLock::release() {
 
 void WakeLock::releaseOnce() {
     std::call_once(mReleased, [this]() {
-        mSystemSuspend->decSuspendCounter();
+        mSystemSuspend->decSuspendCounter(mName);
         mSystemSuspend->deleteWakeLockStatsEntry(mId);
     });
 }
 
 SystemSuspend::SystemSuspend(unique_fd wakeupCountFd, unique_fd stateFd, size_t maxStatsEntries,
                              std::chrono::milliseconds baseSleepTime,
-                             const sp<SuspendControlService>& controlService)
+                             const sp<SuspendControlService>& controlService,
+                             bool useSuspendCounter)
     : mSuspendCounter(0),
       mWakeupCountFd(std::move(wakeupCountFd)),
       mStateFd(std::move(stateFd)),
       mMaxStatsEntries(maxStatsEntries),
       mBaseSleepTime(baseSleepTime),
       mSleepTime(baseSleepTime),
-      mControlService(controlService) {
+      mControlService(controlService),
+      mUseSuspendCounter(useSuspendCounter),
+      mWakeLockFd(-1),
+      mWakeUnlockFd(-1) {
     mControlService->setSuspendService(this);
+
+    if (!mUseSuspendCounter) {
+        mWakeLockFd.reset(TEMP_FAILURE_RETRY(open(kSysPowerWakeLock, O_CLOEXEC | O_RDWR)));
+        if (mWakeLockFd < 0) {
+            PLOG(ERROR) << "error opening " << kSysPowerWakeLock;
+        }
+        mWakeUnlockFd.reset(TEMP_FAILURE_RETRY(open(kSysPowerWakeUnlock, O_CLOEXEC | O_RDWR)));
+        if (mWakeUnlockFd < 0) {
+            PLOG(ERROR) << "error opening " << kSysPowerWakeUnlock;
+        }
+    }
 }
 
 bool SystemSuspend::enableAutosuspend() {
@@ -117,7 +136,7 @@ Return<sp<IWakeLock>> SystemSuspend::acquireWakeLock(WakeLockType /* type */,
                                                      const hidl_string& name) {
     auto pid = getCallingPid();
     auto wlId = getWakeLockId(pid, name);
-    IWakeLock* wl = new WakeLock{this, wlId};
+    IWakeLock* wl = new WakeLock{this, wlId, name};
     {
         auto l = std::lock_guard(mStatsLock);
 
@@ -158,15 +177,27 @@ Return<void> SystemSuspend::debug(const hidl_handle& handle,
     return Void();
 }
 
-void SystemSuspend::incSuspendCounter() {
+void SystemSuspend::incSuspendCounter(const string& name) {
     auto l = std::lock_guard(mCounterLock);
-    mSuspendCounter++;
+    if (mUseSuspendCounter) {
+        mSuspendCounter++;
+    } else {
+        if (!WriteStringToFd(name, mWakeLockFd)) {
+            PLOG(ERROR) << "error writing " << name << " to " << kSysPowerWakeLock;
+        }
+    }
 }
 
-void SystemSuspend::decSuspendCounter() {
+void SystemSuspend::decSuspendCounter(const string& name) {
     auto l = std::lock_guard(mCounterLock);
-    if (--mSuspendCounter == 0) {
-        mCounterCondVar.notify_one();
+    if (mUseSuspendCounter) {
+        if (--mSuspendCounter == 0) {
+            mCounterCondVar.notify_one();
+        }
+    } else {
+        if (!WriteStringToFd(name, mWakeUnlockFd)) {
+            PLOG(ERROR) << "error writing " << name << " to " << kSysPowerWakeUnlock;
+        }
     }
 }
 

diff --git a/suspend/1.0/default/SystemSuspend.h b/suspend/1.0/default/SystemSuspend.h
index 313b234..7f4c2ff 100644 (file)
--- a/suspend/1.0/default/SystemSuspend.h
+++ b/suspend/1.0/default/SystemSuspend.h
@@ -53,7 +53,7 @@ TimestampType getEpochTimeNow();
 
 class WakeLock : public IWakeLock {
    public:
-    WakeLock(SystemSuspend* systemSuspend, const WakeLockIdType& id);
+    WakeLock(SystemSuspend* systemSuspend, const WakeLockIdType& id, const std::string& name);
     ~WakeLock();
 
     Return<void> release();
@@ -64,17 +64,18 @@ class WakeLock : public IWakeLock {
 
     SystemSuspend* mSystemSuspend;
     WakeLockIdType mId;
+    std::string mName;
 };
 
 class SystemSuspend : public ISystemSuspend {
    public:
     SystemSuspend(unique_fd wakeupCountFd, unique_fd stateFd, size_t maxStatsEntries,
                   std::chrono::milliseconds baseSleepTime,
-                  const sp<SuspendControlService>& controlService);
+                  const sp<SuspendControlService>& controlService, bool useSuspendCounter = true);
     Return<sp<IWakeLock>> acquireWakeLock(WakeLockType type, const hidl_string& name) override;
     Return<void> debug(const hidl_handle& handle, const hidl_vec<hidl_string>& options) override;
-    void incSuspendCounter();
-    void decSuspendCounter();
+    void incSuspendCounter(const std::string& name);
+    void decSuspendCounter(const std::string& name);
     void deleteWakeLockStatsEntry(WakeLockIdType id);
     bool enableAutosuspend();
 
@@ -106,6 +107,13 @@ class SystemSuspend : public ISystemSuspend {
     void updateSleepTime(bool success);
 
     sp<SuspendControlService> mControlService;
+
+    // If true, use mSuspendCounter to keep track of native wake locks. Otherwise, rely on
+    // /sys/power/wake_lock interface to block suspend.
+    // TODO(b/128923994): remove dependency on /sys/power/wake_lock interface.
+    bool mUseSuspendCounter;
+    unique_fd mWakeLockFd;
+    unique_fd mWakeUnlockFd;
 };
 
 }  // namespace V1_0

diff --git a/suspend/1.0/default/android.system.suspend@1.0-service.rc b/suspend/1.0/default/android.system.suspend@1.0-service.rc
index 4a4b46a..c5d3869 100644 (file)
--- a/suspend/1.0/default/android.system.suspend@1.0-service.rc
+++ b/suspend/1.0/default/android.system.suspend@1.0-service.rc
@@ -1,4 +1,5 @@
 service system_suspend /system/bin/hw/android.system.suspend@1.0-service
     class hal
     user system
-    group system
+    group system wakelock
+    capabilities BLOCK_SUSPEND

diff --git a/suspend/1.0/default/main.cpp b/suspend/1.0/default/main.cpp
index e37c576..7f2dead 100644 (file)
--- a/suspend/1.0/default/main.cpp
+++ b/suspend/1.0/default/main.cpp
@@ -81,7 +81,7 @@ int main() {
 
     sp<SystemSuspend> suspend =
         new SystemSuspend(std::move(wakeupCountFd), std::move(stateFd), 100 /* maxStatsEntries */,
-                          100ms /* baseSleepTime */, suspendControl);
+                          100ms /* baseSleepTime */, suspendControl, false /* mUseSuspendCounter*/);
     status_t status = suspend->registerAsService();
     if (android::OK != status) {
         LOG(FATAL) << "Unable to register system-suspend service: " << status;