New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 813390 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Clean AdapterLost metrics pollution

Project Member Reported by sonnysasaka@chromium.org, Feb 18 2018

Issue description

We are recording a lot of AdapterLost metrics when there are actually no adapter lost happening, as evident from looking at minnie's metrics. minnie cannot have USB disconnection as the bluetooth adapter is not even USB interface but UART, but AdapterLost metrics contain a lot of samples.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8fd05494b13d4ebbe5674807b7af368e908740ef

commit 8fd05494b13d4ebbe5674807b7af368e908740ef
Author: Sonny Sasaka <sonnysasaka@chromium.org>
Date: Fri Feb 23 19:39:09 2018

Add UMA metrics for BlueZ

This adds BlueZ.ChipLost. This differs from BlueZ.AdapterLost in that
ChipLost records adapter lost specifically due to hardware
disconnection.

Corresponding CL where metrics is used: http://crosreview.com/924505

BUG= chromium:813390 

Change-Id: I1a1956f6f40508b57af12c49ee4c1265c7309457
Reviewed-on: https://chromium-review.googlesource.com/930482
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
Commit-Queue: Sonny Sasaka <sonnysasaka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538864}
[modify] https://crrev.com/8fd05494b13d4ebbe5674807b7af368e908740ef/tools/metrics/histograms/histograms.xml

Project Member

Comment 2 by bugdroid1@chromium.org, Feb 24 2018

Labels: merge-merged-chromeos-5.44
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/bluez/+/f829a0edb783eac013303dd79d84acec26ddd759

commit f829a0edb783eac013303dd79d84acec26ddd759
Author: Sonny Sasaka <sonnysasaka@chromium.org>
Date: Sat Feb 24 05:06:54 2018

CHROMIUM: Refine adapter lost metrics.

Currently adapter lost events are detected from adapter_free function.
This is a bit risky because we have been assuming that adapter_free is
*only* called when there is USB disconnection, where in fact that
function could be reached from many other unexpected events, e.g.
bluetoothd process killed, bug in adapter object ref-counting, etc.

This CL adds a new metrics which uses adapter lost detection closer to
its source, which is index_removed function which is a callback of the
adapter lost event from kernel directly. Additionally, this metrics is
not recorded if suspend/resume just happened since the adapter lost may
be caused by normal power cut off instead of hardware bug.

The corresponding UMA histogram CL is at http://crosreview.com/930482.

BUG= chromium:813390 
TEST=Simulate adapter lost with 'rmmod btusb' and 'modprobe btusb' and
check from the log that metrics is sent. Simulate system suspend and
check that metrics is not send even though USB disconnection happens.

Change-Id: I92c3829606d301ebb474e6182e4cdf1b7c27cf83
Reviewed-on: https://chromium-review.googlesource.com/924505
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Sonny Sasaka <sonnysasaka@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>

[modify] https://crrev.com/f829a0edb783eac013303dd79d84acec26ddd759/src/metrics.c
[modify] https://crrev.com/f829a0edb783eac013303dd79d84acec26ddd759/src/adapter.c
[modify] https://crrev.com/f829a0edb783eac013303dd79d84acec26ddd759/src/metrics.h

Comment 3 by mcchou@chromium.org, Apr 19 2018

Status: Fixed (was: Started)

Comment 4 by mcchou@chromium.org, Apr 19 2018

Cc: sonnysasaka@chromium.org
 Issue 812013  has been merged into this issue.
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0d0f53fc4126529af8a277418eed2c93225e57d3

commit 0d0f53fc4126529af8a277418eed2c93225e57d3
Author: Sonny Sasaka <sonnysasaka@chromium.org>
Date: Thu Aug 16 00:02:12 2018

Add UMA metrics BlueZ.ChipLost2

This deprecates BlueZ.ChipLost because we need to filter out
suspend/resume events even when adapter is disabled.

Corresponding CL where metrics is used: https://crrev.com/c/1176398.

BUG= chromium:813390 

Change-Id: I8620fdd1c0225a97226d462e234643882f6553a5
Reviewed-on: https://chromium-review.googlesource.com/1176371
Commit-Queue: Sonny Sasaka <sonnysasaka@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583453}
[modify] https://crrev.com/0d0f53fc4126529af8a277418eed2c93225e57d3/tools/metrics/histograms/histograms.xml

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 17

Labels: merge-merged-3440
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/876f380713e4f8339e87c787e9edbad027b1c341

commit 876f380713e4f8339e87c787e9edbad027b1c341
Author: Miao-chen Chou <mcchou@chromium.org>
Date: Fri Aug 17 21:04:52 2018

Add UMA metrics BlueZ.ChipLost2

This deprecates BlueZ.ChipLost because we need to filter out
suspend/resume events even when adapter is disabled.

Corresponding CL where metrics is used: https://crrev.com/c/1176398.

BUG= chromium:813390 
TBR=sonnysasaka@chromium.org

(cherry picked from commit 0d0f53fc4126529af8a277418eed2c93225e57d3)

Change-Id: I8620fdd1c0225a97226d462e234643882f6553a5
Reviewed-on: https://chromium-review.googlesource.com/1176371
Commit-Queue: Sonny Sasaka <sonnysasaka@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#583453}
Reviewed-on: https://chromium-review.googlesource.com/1179254
Cr-Commit-Position: refs/branch-heads/3440@{#807}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/876f380713e4f8339e87c787e9edbad027b1c341/tools/metrics/histograms/histograms.xml

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 17

Labels: merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8ca1577594b284ef0104902ec317205a061f26e7

commit 8ca1577594b284ef0104902ec317205a061f26e7
Author: Miao-chen Chou <mcchou@chromium.org>
Date: Fri Aug 17 21:05:53 2018

Add UMA metrics BlueZ.ChipLost2

This deprecates BlueZ.ChipLost because we need to filter out
suspend/resume events even when adapter is disabled.

Corresponding CL where metrics is used: https://crrev.com/c/1176398.

BUG= chromium:813390 
TBR=sonnysasaka@chromium.org

(cherry picked from commit 0d0f53fc4126529af8a277418eed2c93225e57d3)

Change-Id: I8620fdd1c0225a97226d462e234643882f6553a5
Reviewed-on: https://chromium-review.googlesource.com/1176371
Commit-Queue: Sonny Sasaka <sonnysasaka@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#583453}
Reviewed-on: https://chromium-review.googlesource.com/1179255
Cr-Commit-Position: refs/branch-heads/3497@{#696}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/8ca1577594b284ef0104902ec317205a061f26e7/tools/metrics/histograms/histograms.xml

Status: Verified (was: Fixed)

Sign in to add a comment