New issue
Advanced search Search tips

Issue 751386 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

flash_ec with no running servod stops/starts a bunch of (incorrect) processes

Project Member Reported by drinkcat@google.com, Aug 2 2017

Issue description

No servod running, or pass an incorrect parameter to flash_ec --port:

# ./util/flash_ec --board=hammer --port=1234
Connection refused
INFO: Using .
INFO: Using ec image : /mnt/host/source/src/platform/ec/build/hammer/ec.bin
Connection refused
Connection refused
ERROR: Cannot communicate with servo. is servod running ?
INFO: ec UART pty : 
INFO: Flashing chip stm32.
INFO: Using serial flasher : /mnt/host/source/src/platform/ec/build/hammer/util/stm32mon
Connection refused
WARNING: hdctools cannot disconnect the EC-3PO interpreter from the UART.
INFO: Sending SIGSTOP to process 32362!
INFO: Sending SIGSTOP to process 40854!
INFO: Sending SIGSTOP to process 32362!
...
INFO: Sending SIGSTOP to process 33799!
INFO: Sending SIGSTOP to process 40109!
INFO: Sending SIGSTOP to process 2!
./util/flash_ec: line 493: kill: (2) - Operation not permitted
INFO: Sending SIGCONT to process 32362!
INFO: Sending SIGCONT to process 40854!
INFO: Sending SIGCONT to process 32362!
...
INFO: Sending SIGCONT to process 40232!
INFO: Sending SIGCONT to process 33799!
INFO: Sending SIGCONT to process 40109!
INFO: Sending SIGCONT to process 2!
./util/flash_ec: line 367: kill: (2) - Operation not permitted

IIRC from previous investigation this is due to claim_pty being called without parameters.
 
This works around the issue:

diff --git a/util/flash_ec b/util/flash_ec
index 166321418..57b45d546 100755
--- a/util/flash_ec
+++ b/util/flash_ec
@@ -468,6 +468,11 @@ function servo_restore() {
 }
 
 function claim_pty() {
+       if [ -z "$1" ]; then
+               warn "no parameter passed to claim pty"
+               return
+       fi
+
        if grep -q cros_sdk /proc/1/cmdline; then
                die "You must run this tool in a chroot that was entered with" \
                    "'cros_sdk --no-ns-pid' (see  crbug.com/444931  for details)"

Status: Started (was: Assigned)
What caught my eye is why is execution continuing after the ERROR message. I'm working on a patch to fix that and incorporate the claim_pty fix to prevent freezing random processes in the future.
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/9051e6f999673f635f649b97106833b2be9f9727

commit 9051e6f999673f635f649b97106833b2be9f9727
Author: Aseda Aboagye <aaboagye@google.com>
Date: Fri Aug 04 02:23:17 2017

flash_ec: Make sure die works in ec_uart_pty().

If flash_ec was run without `servod` running and servod is needed, an
error message is printed out.

ERROR: Cannot communicate with servo. is servod running ?

However, in the case of flashing an stm32 without servod running,
execution would continue and would lead to claim_pty freezing and
thawing a bunch of unrelated processes.  I believe the reason is that
the "die" was run in a subshell and therefore execution continued.

This commit now changes the way that servo_ec_uart_pty() works.  If no
PTY is found, then flash_ec exits printing out the error message.
Additonally, claim_pty() will now warn if no arguments are passed to it
instead of finding random victims.

BUG= chromium:751386 
BRANCH=maybe some fw branches.
TEST=Without servod running, try `./util/flash_ec --board hammer`;
Verify that flash_ec exists after the call to die.

Change-Id: I50784e0c43bbf0e32d408261cb83029377b576a0
Signed-off-by: Aseda Aboagye <aaboagye@google.com>
Reviewed-on: https://chromium-review.googlesource.com/598506
Commit-Ready: Aseda Aboagye <aaboagye@chromium.org>
Tested-by: Aseda Aboagye <aaboagye@chromium.org>
Reviewed-by: Nicolas Boichat <drinkcat@chromium.org>

[modify] https://crrev.com/9051e6f999673f635f649b97106833b2be9f9727/util/flash_ec

Status: Verified (was: Started)
Verified locally in CL.

Sign in to add a comment