From 5ea3163325927188a342f581f119654fa3ed0deb Mon Sep 17 00:00:00 2001 From: Ruchir Rastogi Date: Mon, 13 Jan 2020 16:42:12 -0800 Subject: [PATCH] Migrate DeviceCalculcatedPower pullers to new API As part of migrating the pullers to the new API, we modify permission checks within BatteryStatsService. Previously, a Binder thread within StatsCompanionService (with statsd's calling identity) called BatteryStatsService functions, which was why statsd was assigned the BATTERY_STATS permission. Now, that call is being made from the system process Background thread. Because enforceCallingPermission outside of Binder threads, we switched to enforceCallingOrSelfPermission. Test: m -j Test: adb shell cmd stats pull-source 10039 Test: adb shell cmd stats pull-source 10040 Test: adb shell cmd stats pull-source 10041 Test: atest CtsStatsdHostTestCases:UidAtomTests#testDeviceCalculatedPowerUse Test: atest CtsStatsdHostTestCases:UidAtomTests#testDeviceCalculatedPowerBlameUid Test: atest CtsStatsdHostTestCases:BatteryStatsValidationTests#testPowerUse Bug: 145565211 Change-Id: Ie009e6eead3e48ecee6b40d9a38c9d571d4d4117 --- .../server/stats/StatsCompanionService.java | 87 -------------- .../src/external/StatsPullerManager.cpp | 14 --- data/etc/platform.xml | 1 - .../server/am/BatteryStatsService.java | 2 +- .../server/stats/StatsPullAtomService.java | 111 ++++++++++++++++-- 5 files changed, 102 insertions(+), 113 deletions(-) diff --git a/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java b/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java index 6444b4e7c9ad7..51c6f91c1507a 100644 --- a/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java +++ b/apex/statsd/service/java/com/android/server/stats/StatsCompanionService.java @@ -1404,78 +1404,6 @@ public class StatsCompanionService extends IStatsCompanionService.Stub { } } - private BatteryStatsHelper getBatteryStatsHelper() { - if (mBatteryStatsHelper == null) { - final long callingToken = Binder.clearCallingIdentity(); - try { - // clearCallingIdentity required for BatteryStatsHelper.checkWifiOnly(). - mBatteryStatsHelper = new BatteryStatsHelper(mContext, false); - } finally { - Binder.restoreCallingIdentity(callingToken); - } - mBatteryStatsHelper.create((Bundle) null); - } - long currentTime = SystemClock.elapsedRealtime(); - if (currentTime - mBatteryStatsHelperTimestampMs >= MAX_BATTERY_STATS_HELPER_FREQUENCY_MS) { - // Load BatteryStats and do all the calculations. - mBatteryStatsHelper.refreshStats(BatteryStats.STATS_SINCE_CHARGED, UserHandle.USER_ALL); - // Calculations are done so we don't need to save the raw BatteryStats data in RAM. - mBatteryStatsHelper.clearStats(); - mBatteryStatsHelperTimestampMs = currentTime; - } - return mBatteryStatsHelper; - } - - private long milliAmpHrsToNanoAmpSecs(double mAh) { - final long MILLI_AMP_HR_TO_NANO_AMP_SECS = 1_000_000L * 3600L; - return (long) (mAh * MILLI_AMP_HR_TO_NANO_AMP_SECS + 0.5); - } - - private void pullDeviceCalculatedPowerUse(int tagId, - long elapsedNanos, final long wallClockNanos, List pulledData) { - BatteryStatsHelper bsHelper = getBatteryStatsHelper(); - StatsLogEventWrapper e = new StatsLogEventWrapper(tagId, elapsedNanos, wallClockNanos); - e.writeLong(milliAmpHrsToNanoAmpSecs(bsHelper.getComputedPower())); - pulledData.add(e); - } - - private void pullDeviceCalculatedPowerBlameUid(int tagId, - long elapsedNanos, final long wallClockNanos, List pulledData) { - final List sippers = getBatteryStatsHelper().getUsageList(); - if (sippers == null) { - return; - } - for (BatterySipper bs : sippers) { - if (bs.drainType != bs.drainType.APP) { - continue; - } - StatsLogEventWrapper e = new StatsLogEventWrapper(tagId, elapsedNanos, wallClockNanos); - e.writeInt(bs.uidObj.getUid()); - e.writeLong(milliAmpHrsToNanoAmpSecs(bs.totalPowerMah)); - pulledData.add(e); - } - } - - private void pullDeviceCalculatedPowerBlameOther(int tagId, - long elapsedNanos, final long wallClockNanos, List pulledData) { - final List sippers = getBatteryStatsHelper().getUsageList(); - if (sippers == null) { - return; - } - for (BatterySipper bs : sippers) { - if (bs.drainType == bs.drainType.APP) { - continue; // This is a separate atom; see pullDeviceCalculatedPowerBlameUid(). - } - if (bs.drainType == bs.drainType.USER) { - continue; // This is not supported. We purposefully calculate over USER_ALL. - } - StatsLogEventWrapper e = new StatsLogEventWrapper(tagId, elapsedNanos, wallClockNanos); - e.writeInt(bs.drainType.ordinal()); - e.writeLong(milliAmpHrsToNanoAmpSecs(bs.totalPowerMah)); - pulledData.add(e); - } - } - private void pullDiskIo(int tagId, long elapsedNanos, final long wallClockNanos, List pulledData) { mStoragedUidIoStatsReader.readAbsolute((uid, fgCharsRead, fgCharsWrite, fgBytesRead, @@ -2107,21 +2035,6 @@ public class StatsCompanionService extends IStatsCompanionService.Stub { break; } - case StatsLog.DEVICE_CALCULATED_POWER_USE: { - pullDeviceCalculatedPowerUse(tagId, elapsedNanos, wallClockNanos, ret); - break; - } - - case StatsLog.DEVICE_CALCULATED_POWER_BLAME_UID: { - pullDeviceCalculatedPowerBlameUid(tagId, elapsedNanos, wallClockNanos, ret); - break; - } - - case StatsLog.DEVICE_CALCULATED_POWER_BLAME_OTHER: { - pullDeviceCalculatedPowerBlameOther(tagId, elapsedNanos, wallClockNanos, ret); - break; - } - case StatsLog.TEMPERATURE: { pullTemperature(tagId, elapsedNanos, wallClockNanos, ret); break; diff --git a/cmds/statsd/src/external/StatsPullerManager.cpp b/cmds/statsd/src/external/StatsPullerManager.cpp index b5920cb6057ab..37ee8b7be8f40 100644 --- a/cmds/statsd/src/external/StatsPullerManager.cpp +++ b/cmds/statsd/src/external/StatsPullerManager.cpp @@ -193,20 +193,6 @@ std::map StatsPullerManager::kAllPullAtomInfo = { {.additiveFields = {7, 9, 11, 13, 15, 17, 19, 21}, .puller = new StatsCompanionServicePuller(android::util::CPU_TIME_PER_THREAD_FREQ)}}, - // DeviceCalculatedPowerUse. - {{.atomTag = android::util::DEVICE_CALCULATED_POWER_USE}, - {.puller = new StatsCompanionServicePuller(android::util::DEVICE_CALCULATED_POWER_USE)}}, - - // DeviceCalculatedPowerBlameUid. - {{.atomTag = android::util::DEVICE_CALCULATED_POWER_BLAME_UID}, - {.puller = new StatsCompanionServicePuller( - android::util::DEVICE_CALCULATED_POWER_BLAME_UID)}}, - - // DeviceCalculatedPowerBlameOther. - {{.atomTag = android::util::DEVICE_CALCULATED_POWER_BLAME_OTHER}, - {.puller = new StatsCompanionServicePuller( - android::util::DEVICE_CALCULATED_POWER_BLAME_OTHER)}}, - // DebugElapsedClock. {{.atomTag = android::util::DEBUG_ELAPSED_CLOCK}, {.additiveFields = {1, 2, 3, 4}, diff --git a/data/etc/platform.xml b/data/etc/platform.xml index 0574775712a6a..877ef26873498 100644 --- a/data/etc/platform.xml +++ b/data/etc/platform.xml @@ -180,7 +180,6 @@ - diff --git a/services/core/java/com/android/server/am/BatteryStatsService.java b/services/core/java/com/android/server/am/BatteryStatsService.java index 37026fd42f238..a98b83b09a6a9 100644 --- a/services/core/java/com/android/server/am/BatteryStatsService.java +++ b/services/core/java/com/android/server/am/BatteryStatsService.java @@ -402,7 +402,7 @@ public final class BatteryStatsService extends IBatteryStats.Stub } public ParcelFileDescriptor getStatisticsStream() { - mContext.enforceCallingPermission( + mContext.enforceCallingOrSelfPermission( android.Manifest.permission.BATTERY_STATS, null); //Slog.i("foo", "SENDING BATTERY INFO:"); //mStats.dumpLocked(new LogPrinter(Log.INFO, "foo", Log.LOG_ID_SYSTEM)); diff --git a/services/core/java/com/android/server/stats/StatsPullAtomService.java b/services/core/java/com/android/server/stats/StatsPullAtomService.java index 1f7ed8ad80727..372b9cae6b80b 100644 --- a/services/core/java/com/android/server/stats/StatsPullAtomService.java +++ b/services/core/java/com/android/server/stats/StatsPullAtomService.java @@ -1113,28 +1113,119 @@ public class StatsPullAtomService extends SystemService { // No op. } - private void registerDeviceCalculatedPowerUse() { - // No op. + // TODO: move to top of file when all migrations are complete + private BatteryStatsHelper mBatteryStatsHelper = null; + private static final int MAX_BATTERY_STATS_HELPER_FREQUENCY_MS = 1000; + private long mBatteryStatsHelperTimestampMs = -MAX_BATTERY_STATS_HELPER_FREQUENCY_MS; + private static final long MILLI_AMP_HR_TO_NANO_AMP_SECS = 1_000_000L * 3600L; + + private BatteryStatsHelper getBatteryStatsHelper() { + if (mBatteryStatsHelper == null) { + final long callingToken = Binder.clearCallingIdentity(); + try { + // clearCallingIdentity required for BatteryStatsHelper.checkWifiOnly(). + mBatteryStatsHelper = new BatteryStatsHelper(mContext, false); + } finally { + Binder.restoreCallingIdentity(callingToken); + } + mBatteryStatsHelper.create((Bundle) null); + } + long currentTime = SystemClock.elapsedRealtime(); + if (currentTime - mBatteryStatsHelperTimestampMs >= MAX_BATTERY_STATS_HELPER_FREQUENCY_MS) { + // Load BatteryStats and do all the calculations. + mBatteryStatsHelper.refreshStats(BatteryStats.STATS_SINCE_CHARGED, UserHandle.USER_ALL); + // Calculations are done so we don't need to save the raw BatteryStats data in RAM. + mBatteryStatsHelper.clearStats(); + mBatteryStatsHelperTimestampMs = currentTime; + } + return mBatteryStatsHelper; } - private void pullDeviceCalculatedPowerUse() { - // No op. + private long milliAmpHrsToNanoAmpSecs(double mAh) { + return (long) (mAh * MILLI_AMP_HR_TO_NANO_AMP_SECS + 0.5); + } + + private void registerDeviceCalculatedPowerUse() { + int tagId = StatsLog.DEVICE_CALCULATED_POWER_USE; + mStatsManager.registerPullAtomCallback( + tagId, + null, // use default PullAtomMetadata values + (atomTag, data) -> pullDeviceCalculatedPowerUse(atomTag, data), + BackgroundThread.getExecutor() + ); + } + + private int pullDeviceCalculatedPowerUse(int atomTag, List pulledData) { + BatteryStatsHelper bsHelper = getBatteryStatsHelper(); + StatsEvent e = StatsEvent.newBuilder() + .setAtomId(atomTag) + .writeLong(milliAmpHrsToNanoAmpSecs(bsHelper.getComputedPower())) + .build(); + pulledData.add(e); + return StatsManager.PULL_SUCCESS; } private void registerDeviceCalculatedPowerBlameUid() { - // No op. + int tagId = StatsLog.DEVICE_CALCULATED_POWER_BLAME_UID; + mStatsManager.registerPullAtomCallback( + tagId, + null, // use default PullAtomMetadata values + (atomTag, data) -> pullDeviceCalculatedPowerBlameUid(atomTag, data), + BackgroundThread.getExecutor() + ); } - private void pullDeviceCalculatedPowerBlameUid() { - // No op. + private int pullDeviceCalculatedPowerBlameUid(int atomTag, List pulledData) { + final List sippers = getBatteryStatsHelper().getUsageList(); + if (sippers == null) { + return StatsManager.PULL_SKIP; + } + + for (BatterySipper bs : sippers) { + if (bs.drainType != bs.drainType.APP) { + continue; + } + StatsEvent e = StatsEvent.newBuilder() + .setAtomId(atomTag) + .writeInt(bs.uidObj.getUid()) + .writeLong(milliAmpHrsToNanoAmpSecs(bs.totalPowerMah)) + .build(); + pulledData.add(e); + } + return StatsManager.PULL_SUCCESS; } private void registerDeviceCalculatedPowerBlameOther() { - // No op. + int tagId = StatsLog.DEVICE_CALCULATED_POWER_BLAME_OTHER; + mStatsManager.registerPullAtomCallback( + tagId, + null, // use default PullAtomMetadata values + (atomTag, data) -> pullDeviceCalculatedPowerBlameOther(atomTag, data), + BackgroundThread.getExecutor() + ); } - private void pullDeviceCalculatedPowerBlameOther() { - // No op. + private int pullDeviceCalculatedPowerBlameOther(int atomTag, List pulledData) { + final List sippers = getBatteryStatsHelper().getUsageList(); + if (sippers == null) { + return StatsManager.PULL_SKIP; + } + + for (BatterySipper bs : sippers) { + if (bs.drainType == bs.drainType.APP) { + continue; // This is a separate atom; see pullDeviceCalculatedPowerBlameUid(). + } + if (bs.drainType == bs.drainType.USER) { + continue; // This is not supported. We purposefully calculate over USER_ALL. + } + StatsEvent e = StatsEvent.newBuilder() + .setAtomId(atomTag) + .writeInt(bs.drainType.ordinal()) + .writeLong(milliAmpHrsToNanoAmpSecs(bs.totalPowerMah)) + .build(); + pulledData.add(e); + } + return StatsManager.PULL_SUCCESS; } private void registerDebugElapsedClock() {