New issue
Advanced search Search tips

Issue 877909 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 4
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

servod uses 100% CPU after servo_micro is connected

Project Member Reported by drinkcat@chromium.org, Aug 27

Issue description

What steps will reproduce the problem?
(1) Plug servo_micro
(2) Start servod: sudo servod -b kukui
(3) Unplug servo_micro
 
What is the expected result?

servod exits.

What happens instead?

servod stays up, and uses ~100% CPU.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/hdctools/+/8a8878deaef67aef38a470dc11e579acfda62947

commit 8a8878deaef67aef38a470dc11e579acfda62947
Author: Ruben Rodriguez Buchillon <coconutruben@chromium.org>
Date: Sun Sep 09 17:33:45 2018

servod: add device watchdog

This adds a watchdog thread that every second ensures all servo
devices can still be found. If it cannot find one device, leverages the
new clean shutdown path to tear down the servod invocation.

BUG= chromium:877909 , b:74022289
TEST=manual testing
test matrix
           	unplug		ctrl-c
v4 + ccd	 (Y)		 (Y)
v4 + micro	 (Y)		 (Y)
micro		 (Y)		 (Y)
v2		 (Y)		 (Y)
suzyq		 N/A		 (Y)

Change-Id: I960a4f1d959f541695504ffcb4204f1e604d7b65
Signed-off-by: Ruben Rodriguez Buchillon <coconutruben@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1195270

[modify] https://crrev.com/8a8878deaef67aef38a470dc11e579acfda62947/servo/servod.py

Project Member

Comment 2 by bugdroid1@chromium.org, Sep 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/hdctools/+/a5d8b924187eeb3327569485c712994504485b88

commit a5d8b924187eeb3327569485c712994504485b88
Author: Ruben Rodriguez Buchillon <coconutruben@chromium.org>
Date: Sun Sep 09 17:33:45 2018

servod: Extend watchdog to have reinit capabilities.

This CL replaces the WATCHDOG_EXEMPTION with REINIT_CAPABLE i.e. instead
of giving CCD a free-pass, it will attempt to find it again for ~10s,
and attempt to reinitialize it, before giving up.

This works well when plugging/unplugging ccd & when using
power_state:ccd_reset

I have my concerns as I haven't stress-tested enough if it also reliably
works when just rebooting cr50 straight from its console, but I haven't
encountered issues yes.

BUG= chromium:877909 
TEST=manual testing
$ sudo servod -b soraka &
<unplug ccd>
WARNING - Device - vid: 0x18d1 pid: 0x5014 serial: u'17017055-90ECC673' not
found when polling. Device is maked as reinit capable. xx more reinit attempts
before giving up.
<plug back in>
$ dut-control cr50_version
cr50_version:0.4.4/cr50_v1.9308_B.334-78f874b

$ dut-control "cr50_uart_cmd:ccd open"
$ dut-control "cr50_uart_cmd:reboot"
WARNING - Device - vid: 0x18d1 pid: 0x5014 serial: u'17017055-90ECC673' not
found when polling. Device is maked as reinit capable. xx more reinit attempts
before giving up.
$ dut-control cr50_version
cr50_version:0.4.4/cr50_v1.9308_B.334-78f874b

$ dut-control power_state:cr50_reset
WARNING - Device - vid: 0x18d1 pid: 0x5014 serial: u'17017055-90ECC673' not
found when polling. Device is maked as reinit capable. xx more reinit attempts
before giving up.
$ dut-control cr50_version
cr50_version:0.4.4/cr50_v1.9308_B.334-78f874b

Change-Id: Id577aec97a29eb9bdb8ae79104f175c80a7138ec
Signed-off-by: Ruben Rodriguez Buchillon <coconutruben@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1201922
Reviewed-by: Mary Ruthven <mruthven@chromium.org>

[modify] https://crrev.com/a5d8b924187eeb3327569485c712994504485b88/servo/drv/power_state.py
[modify] https://crrev.com/a5d8b924187eeb3327569485c712994504485b88/servo/servod.py
[modify] https://crrev.com/a5d8b924187eeb3327569485c712994504485b88/servo/servo_server.py

Project Member

Comment 3 by bugdroid1@chromium.org, Sep 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/hdctools/+/0ec4834a884fd4e8a50c3f8c11464a8e3e165229

commit 0ec4834a884fd4e8a50c3f8c11464a8e3e165229
Author: Ruben Rodriguez Buchillon <coconutruben@chromium.org>
Date: Fri Sep 21 07:51:16 2018

servod: refactor watchdog to use sysfs paths

This changes the watchdog to instead of polling pyusb for vid, pid,
serial, to use those to find the device's path under /sys/bus/usb, and
poll for that paths' existence. This alleviates some issues with
receiving a pipe error when trying to read a usb.core.Device's
iSerialNumber descriptor.
We might still want to investigate what caused/causes those.

Also fixes an issue where the reinit logic was (vid, pid) based, and not
(vid, pid, serialname), thus making all ccd devices share the same
reinit tokens.

BUG= chromium:877909 
TEST=manual testing
> usb disconnect still triggers a shutdown
> pulling out the ccd cable & plugging it back in still reinitializes
the interface
> dut-control cr50_uart_cmd:reboot still manages to reinitialize

Change-Id: I233a37b18cc9d9dcee5f19c56af01b111d168746
Signed-off-by: Ruben Rodriguez Buchillon <coconutruben@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1233133
Tested-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-by: Nick Sanders <nsanders@chromium.org>
Reviewed-by: Todd Broch <tbroch@chromium.org>

[modify] https://crrev.com/0ec4834a884fd4e8a50c3f8c11464a8e3e165229/servo/servod.py
[modify] https://crrev.com/0ec4834a884fd4e8a50c3f8c11464a8e3e165229/servo/servodutil.py

Status: Fixed (was: Assigned)
Closing this out as the watchdog takes care of this now.
If this surfaces again, please file a bug again/reopen.

Sign in to add a comment