From b302c54f11d5468100c566fba8e70d8614490e1a Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Fri, 15 Sep 2017 12:57:59 -0600 Subject: [PATCH] Move long-running calls to async with listeners. Now that we're using Binder, we can have callers provide explicit listeners for every request instead of trying to squeeze them all into unsolicited socket events. Move benchmarking to be async to avoid blocking other commands for up to several minutes. Remove post-trim benchmarking flag, since benchmarking now requires a separate callback. Will bring back in a future CL. Test: cts-tradefed run commandAndExit cts-dev -m CtsAppSecurityHostTestCases -t android.appsecurity.cts.AdoptableHostTest Test: adb shell sm fstrim Bug: 62201209, 13758960 Change-Id: I26f76c66734ac2fd0f64713b8ab9828430499019 --- Android.mk | 1 + cmds/sm/src/com/android/commands/sm/Sm.java | 14 +- .../android/os/storage/StorageManager.java | 2 - services/core/Android.mk | 1 + .../android/server/StorageManagerService.java | 151 +++++++++--------- 5 files changed, 93 insertions(+), 76 deletions(-) diff --git a/Android.mk b/Android.mk index cedb60b00c027..05559fdf9ded4 100644 --- a/Android.mk +++ b/Android.mk @@ -569,6 +569,7 @@ LOCAL_SRC_FILES += \ ../../system/netd/server/binder/android/net/INetd.aidl \ ../../system/vold/binder/android/os/IVold.aidl \ ../../system/vold/binder/android/os/IVoldListener.aidl \ + ../../system/vold/binder/android/os/IVoldTaskListener.aidl \ ../native/cmds/installd/binder/android/os/IInstalld.aidl \ LOCAL_AIDL_INCLUDES += system/update_engine/binder_bindings diff --git a/cmds/sm/src/com/android/commands/sm/Sm.java b/cmds/sm/src/com/android/commands/sm/Sm.java index 658d662de5e12..a9a4118a8e986 100644 --- a/cmds/sm/src/com/android/commands/sm/Sm.java +++ b/cmds/sm/src/com/android/commands/sm/Sm.java @@ -16,6 +16,10 @@ package com.android.commands.sm; +import static android.os.storage.StorageManager.PROP_ADOPTABLE_FBE; +import static android.os.storage.StorageManager.PROP_HAS_ADOPTABLE; +import static android.os.storage.StorageManager.PROP_VIRTUAL_DISK; + import android.os.RemoteException; import android.os.ServiceManager; import android.os.SystemProperties; @@ -134,7 +138,15 @@ public final class Sm { } public void runHasAdoptable() { - System.out.println(SystemProperties.getBoolean(StorageManager.PROP_HAS_ADOPTABLE, false)); + final boolean hasHardware = SystemProperties.getBoolean(PROP_HAS_ADOPTABLE, false) + || SystemProperties.getBoolean(PROP_VIRTUAL_DISK, false); + final boolean hasSoftware; + if (StorageManager.isFileEncryptedNativeOnly()) { + hasSoftware = SystemProperties.getBoolean(PROP_ADOPTABLE_FBE, false); + } else { + hasSoftware = true; + } + System.out.println(hasHardware && hasSoftware); } public void runGetPrimaryStorageUuid() throws RemoteException { diff --git a/core/java/android/os/storage/StorageManager.java b/core/java/android/os/storage/StorageManager.java index 70363468c0971..6594cd070d248 100644 --- a/core/java/android/os/storage/StorageManager.java +++ b/core/java/android/os/storage/StorageManager.java @@ -221,8 +221,6 @@ public class StorageManager { /** {@hide} */ public static final int FSTRIM_FLAG_DEEP = IVold.FSTRIM_FLAG_DEEP_TRIM; - /** {@hide} */ - public static final int FSTRIM_FLAG_BENCHMARK = IVold.FSTRIM_FLAG_BENCHMARK_AFTER; /** @hide The volume is not encrypted. */ public static final int ENCRYPTION_STATE_NONE = diff --git a/services/core/Android.mk b/services/core/Android.mk index 7776346ff55bc..599485ffe5c1f 100644 --- a/services/core/Android.mk +++ b/services/core/Android.mk @@ -17,6 +17,7 @@ LOCAL_SRC_FILES += \ ../../../../system/netd/server/binder/android/net/metrics/INetdEventListener.aidl \ ../../../../system/vold/binder/android/os/IVold.aidl \ ../../../../system/vold/binder/android/os/IVoldListener.aidl \ + ../../../../system/vold/binder/android/os/IVoldTaskListener.aidl \ ../../../native/cmds/installd/binder/android/os/IInstalld.aidl \ LOCAL_AIDL_INCLUDES += \ diff --git a/services/core/java/com/android/server/StorageManagerService.java b/services/core/java/com/android/server/StorageManagerService.java index e2c0e326365f6..c0fcfd07a75f0 100644 --- a/services/core/java/com/android/server/StorageManagerService.java +++ b/services/core/java/com/android/server/StorageManagerService.java @@ -58,10 +58,12 @@ import android.os.HandlerThread; import android.os.IBinder; import android.os.IVold; import android.os.IVoldListener; +import android.os.IVoldTaskListener; import android.os.Looper; import android.os.Message; import android.os.ParcelFileDescriptor; import android.os.ParcelableException; +import android.os.PersistableBundle; import android.os.PowerManager; import android.os.Process; import android.os.RemoteCallbackList; @@ -144,6 +146,7 @@ import java.util.Locale; import java.util.Map; import java.util.Map.Entry; import java.util.Objects; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -287,10 +290,6 @@ class StorageManagerService extends IStorageManager.Stub public static final int VOLUME_PATH_CHANGED = 655; public static final int VOLUME_INTERNAL_PATH_CHANGED = 656; public static final int VOLUME_DESTROYED = 659; - - public static final int MOVE_STATUS = 660; - public static final int BENCHMARK_RESULT = 661; - public static final int TRIM_RESULT = 662; } private static final int VERSION_INIT = 1; @@ -673,8 +672,8 @@ class StorageManagerService extends IStorageManager.Stub Slog.e(TAG, "Unable to record last fstrim!"); } - final int flags = shouldBenchmark() ? StorageManager.FSTRIM_FLAG_BENCHMARK : 0; - fstrim(flags); + // TODO: Reintroduce shouldBenchmark() test + fstrim(0); // invoke the completion callback, if any // TODO: fstrim is non-blocking, so remove this useless callback @@ -1249,52 +1248,6 @@ class StorageManagerService extends IStorageManager.Stub mListener.onVolumeDestroyed(volId); break; } - - case VoldResponseCode.MOVE_STATUS: { - final int status = Integer.parseInt(cooked[1]); - onMoveStatusLocked(status); - break; - } - case VoldResponseCode.BENCHMARK_RESULT: { - if (cooked.length != 7) break; - final String path = cooked[1]; - final String ident = cooked[2]; - final long create = Long.parseLong(cooked[3]); - final long drop = Long.parseLong(cooked[4]); - final long run = Long.parseLong(cooked[5]); - final long destroy = Long.parseLong(cooked[6]); - - final DropBoxManager dropBox = mContext.getSystemService(DropBoxManager.class); - dropBox.addText(TAG_STORAGE_BENCHMARK, scrubPath(path) - + " " + ident + " " + create + " " + run + " " + destroy); - - final VolumeRecord rec = findRecordForPath(path); - if (rec != null) { - rec.lastBenchMillis = System.currentTimeMillis(); - writeSettingsLocked(); - } - - break; - } - case VoldResponseCode.TRIM_RESULT: { - if (cooked.length != 4) break; - final String path = cooked[1]; - final long bytes = Long.parseLong(cooked[2]); - final long time = Long.parseLong(cooked[3]); - - final DropBoxManager dropBox = mContext.getSystemService(DropBoxManager.class); - dropBox.addText(TAG_STORAGE_TRIM, scrubPath(path) - + " " + bytes + " " + time); - - final VolumeRecord rec = findRecordForPath(path); - if (rec != null) { - rec.lastTrimMillis = System.currentTimeMillis(); - writeSettingsLocked(); - } - - break; - } - default: { Slog.d(TAG, "Unhandled vold event " + code); } @@ -2027,15 +1980,39 @@ class StorageManagerService extends IStorageManager.Stub enforcePermission(android.Manifest.permission.MOUNT_FORMAT_FILESYSTEMS); waitForReady(); + // TODO: refactor for callers to provide a listener try { - // TODO: make benchmark async so we don't block other commands - if (ENABLE_BINDER) { - return mVold.benchmark(volId); - } else { - final NativeDaemonEvent res = mConnector.execute(3 * DateUtils.MINUTE_IN_MILLIS, - "volume", "benchmark", volId); - return Long.parseLong(res.getMessage()); - } + final CompletableFuture result = new CompletableFuture<>(); + mVold.benchmark(volId, new IVoldTaskListener.Stub() { + @Override + public void onStatus(int status, PersistableBundle extras) { + // Not currently used + } + + @Override + public void onFinished(int status, PersistableBundle extras) { + result.complete(extras); + + final String path = extras.getString("path"); + final String ident = extras.getString("ident"); + final long create = extras.getLong("create"); + final long run = extras.getLong("run"); + final long destroy = extras.getLong("destroy"); + + final DropBoxManager dropBox = mContext.getSystemService(DropBoxManager.class); + dropBox.addText(TAG_STORAGE_BENCHMARK, scrubPath(path) + + " " + ident + " " + create + " " + run + " " + destroy); + + synchronized (mLock) { + final VolumeRecord rec = findRecordForPath(path); + if (rec != null) { + rec.lastBenchMillis = System.currentTimeMillis(); + writeSettingsLocked(); + } + } + } + }); + return result.get(3, TimeUnit.MINUTES).getLong("run", Long.MAX_VALUE); } catch (Exception e) { Slog.wtf(TAG, e); return Long.MAX_VALUE; @@ -2199,16 +2176,36 @@ class StorageManagerService extends IStorageManager.Stub } else { cmd = "dotrim"; } - if ((flags & StorageManager.FSTRIM_FLAG_BENCHMARK) != 0) { - cmd += "bench"; - } try { - if (ENABLE_BINDER) { - mVold.fstrim(flags); - } else { - mConnector.execute("fstrim", cmd); - } + mVold.fstrim(flags, new IVoldTaskListener.Stub() { + @Override + public void onStatus(int status, PersistableBundle extras) { + // Ignore trim failures + if (status != 0) return; + + final String path = extras.getString("path"); + final long bytes = extras.getLong("bytes"); + final long time = extras.getLong("time"); + + final DropBoxManager dropBox = mContext.getSystemService(DropBoxManager.class); + dropBox.addText(TAG_STORAGE_TRIM, scrubPath(path) + " " + bytes + " " + time); + + synchronized (mLock) { + final VolumeRecord rec = findRecordForPath(path); + if (rec != null) { + rec.lastTrimMillis = System.currentTimeMillis(); + writeSettingsLocked(); + } + } + } + + @Override + public void onFinished(int status, PersistableBundle extras) { + // Not currently used + // TODO: benchmark when desired + } + }); } catch (Exception e) { Slog.wtf(TAG, e); } @@ -2388,11 +2385,19 @@ class StorageManagerService extends IStorageManager.Stub } try { - if (ENABLE_BINDER) { - mVold.moveStorage(from.id, to.id); - } else { - mConnector.execute("volume", "move_storage", from.id, to.id); - } + mVold.moveStorage(from.id, to.id, new IVoldTaskListener.Stub() { + @Override + public void onStatus(int status, PersistableBundle extras) { + synchronized (mLock) { + onMoveStatusLocked(status); + } + } + + @Override + public void onFinished(int status, PersistableBundle extras) { + // Not currently used + } + }); } catch (Exception e) { Slog.wtf(TAG, e); }