From c343b5ceff3a1153db772a4c0ffb24c6870c3db0 Mon Sep 17 00:00:00 2001 From: mukesh agrawal Date: Fri, 5 May 2017 17:14:21 -0700 Subject: WifiDiagnostics: disable ring-buffers on |user| builds We've seen ring-buffer events cause a number of problems due to overload or deadlock. While the ring-buffers events aren't necessarily the root cause of these problems, the ring-buffer events _are_ a significant factor. Mitigate risk for end-users, by disabling ring-buffers on |user| builds. Bug: 37756562 Test: tests/wifitests/runtests.sh Test: manual test (see below) Manual test: (on bullhead) 1. install |user| build 2. connect to GoogleGuest 3. adb shell pgrep system_server 4. adb shell dumpsys wifi | grep "ring-buffer =" -> expect no matching lines 5. adb shell pgrep system_server 6. PIDs from steps 3 and 5 should match 7. install |userdebug| build 8. connect to GoogleGuest 9. adb shell pgrep system_server 10. adb shell dumpsys wifi | grep "ring-buffer =" -> expect at least one match 11. adb shell pgrep system_server 12. PIDs from steps 9 and 11 should match Change-Id: Ib7cf1f9e9442ed3c3191d2b20b0812afbbf62930 --- .../com/android/server/wifi/WifiDiagnostics.java | 5 +++ .../android/server/wifi/WifiDiagnosticsTest.java | 38 ++++++++++++++++++---- 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/service/java/com/android/server/wifi/WifiDiagnostics.java b/service/java/com/android/server/wifi/WifiDiagnostics.java index 9803b6dc0..b2acb2abf 100644 --- a/service/java/com/android/server/wifi/WifiDiagnostics.java +++ b/service/java/com/android/server/wifi/WifiDiagnostics.java @@ -413,6 +413,11 @@ class WifiDiagnostics extends BaseWifiDiagnostics { } private boolean fetchRingBuffers() { + if (mBuildProperties.isUserBuild()) { + mRingBuffers = null; + return false; + } + if (mRingBuffers != null) return true; mRingBuffers = mWifiNative.getRingBufferStatus(); diff --git a/tests/wifitests/src/com/android/server/wifi/WifiDiagnosticsTest.java b/tests/wifitests/src/com/android/server/wifi/WifiDiagnosticsTest.java index 41453d6a7..f4b710e91 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiDiagnosticsTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiDiagnosticsTest.java @@ -168,6 +168,7 @@ public class WifiDiagnosticsTest { @Test public void startLoggingStopsAndRestartsRingBufferLogging() throws Exception { final boolean verbosityToggle = false; + setBuildPropertiesToEnableRingBuffers(); mWifiDiagnostics.startLogging(verbosityToggle); verify(mWifiNative).startLoggingRingBuffer( eq(WifiDiagnostics.VERBOSE_NO_LOG), anyInt(), anyInt(), anyInt(), @@ -177,6 +178,14 @@ public class WifiDiagnosticsTest { eq(FAKE_RING_BUFFER_NAME)); } + @Test + public void startLoggingDoesNotStartRingBuffersOnUserBuilds() throws Exception { + final boolean verbosityToggle = true; + mWifiDiagnostics.startLogging(verbosityToggle); + verify(mWifiNative, never()).startLoggingRingBuffer( + anyInt(), anyInt(), anyInt(), anyInt(), anyString()); + } + /** Verifies that, if a log handler was registered, then stopLogging() resets it. */ @Test public void stopLoggingResetsLogHandlerIfHandlerWasRegistered() throws Exception { @@ -222,6 +231,7 @@ public class WifiDiagnosticsTest { @Test public void canCaptureAndStoreRingBufferData() throws Exception { final boolean verbosityToggle = false; + setBuildPropertiesToEnableRingBuffers(); mWifiDiagnostics.startLogging(verbosityToggle); final byte[] data = new byte[SMALL_RING_BUFFER_SIZE_KB * BYTES_PER_KBYTE]; @@ -239,6 +249,7 @@ public class WifiDiagnosticsTest { @Test public void loggerDiscardsExtraneousData() throws Exception { final boolean verbosityToggle = false; + setBuildPropertiesToEnableRingBuffers(); mWifiDiagnostics.startLogging(verbosityToggle); final byte[] data1 = new byte[SMALL_RING_BUFFER_SIZE_KB * BYTES_PER_KBYTE]; @@ -593,15 +604,16 @@ public class WifiDiagnosticsTest { assertFalse(fateDumpString.contains("Frame bytes")); } - /** Verifies that the default size of our ring buffers is small. */ @Test - public void ringBufferSizeIsSmallByDefault() throws Exception { - final boolean verbosityToggle = false; + public void dumpSucceedsEvenIfRingBuffersAreDisabled() { + final boolean verbosityToggle = true; mWifiDiagnostics.startLogging(verbosityToggle); - mWifiDiagnostics.onRingBufferData( - mFakeRbs, new byte[SMALL_RING_BUFFER_SIZE_KB * BYTES_PER_KBYTE + 1]); - mWifiDiagnostics.captureBugReportData(WifiDiagnostics.REPORT_REASON_NONE); - assertEquals(0, getLoggerRingBufferData().length); + verify(mWifiNative, never()).startLoggingRingBuffer( + anyInt(), anyInt(), anyInt(), anyInt(), anyString()); + + StringWriter sw = new StringWriter(); + PrintWriter pw = new PrintWriter(sw); + mWifiDiagnostics.dump(new FileDescriptor(), pw, new String[]{"bogus", "args"}); } /** Verifies that we use small ring buffers by default, on userdebug builds. */ @@ -636,6 +648,8 @@ public class WifiDiagnosticsTest { @Test public void ringBufferSizeIsLargeInVerboseMode() throws Exception { final boolean verbosityToggle = true; + setBuildPropertiesToEnableRingBuffers(); + mWifiDiagnostics.startLogging(verbosityToggle); mWifiDiagnostics.onRingBufferData( mFakeRbs, new byte[LARGE_RING_BUFFER_SIZE_KB * BYTES_PER_KBYTE]); @@ -646,6 +660,8 @@ public class WifiDiagnosticsTest { /** Verifies that we use large ring buffers when switched from normal to verbose mode. */ @Test public void startLoggingGrowsRingBuffersIfNeeded() throws Exception { + setBuildPropertiesToEnableRingBuffers(); + mWifiDiagnostics.startLogging(false /* verbose disabled */); mWifiDiagnostics.startLogging(true /* verbose enabled */); mWifiDiagnostics.onRingBufferData( @@ -657,6 +673,8 @@ public class WifiDiagnosticsTest { /** Verifies that we use small ring buffers when switched from verbose to normal mode. */ @Test public void startLoggingShrinksRingBuffersIfNeeded() throws Exception { + setBuildPropertiesToEnableRingBuffers(); + mWifiDiagnostics.startLogging(true /* verbose enabled */); mWifiDiagnostics.onRingBufferData( mFakeRbs, new byte[SMALL_RING_BUFFER_SIZE_KB * BYTES_PER_KBYTE + 1]); @@ -786,4 +804,10 @@ public class WifiDiagnosticsTest { new FileDescriptor(), new PrintWriter(new StringWriter()), new String[]{}); verify(mLastMileLogger).dump(anyObject()); } + + private void setBuildPropertiesToEnableRingBuffers() { + when(mBuildProperties.isEngBuild()).thenReturn(false); + when(mBuildProperties.isUserdebugBuild()).thenReturn(true); + when(mBuildProperties.isUserBuild()).thenReturn(false); + } } -- cgit v1.2.3