From e979bafb50fe4309502b05ba79322d8d256525c4 Mon Sep 17 00:00:00 2001
From: Douglas Raillard <douglas.raillard@arm.com>
Date: Fri, 20 Aug 2021 10:59:40 +0100
Subject: [PATCH] target: Speedup Target.write_value()

Avoid an execute() by doing the check in the same command. This also
allows to return early if the write is fast, and to extend for longer if
the write is slow. The speed at which you can observe a write in sysfs
depends on the backing kernel handlers, so there is a wide variety of
situations.

Also, make a more fine grained error detection by allowing the write
itself to fail, which can happen when writing invalid values to sysfs.
---
 devlib/target.py | 42 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 37 insertions(+), 5 deletions(-)

diff --git a/devlib/target.py b/devlib/target.py
index f07f326..b78ca6d 100644
--- a/devlib/target.py
+++ b/devlib/target.py
@@ -50,7 +50,7 @@ from devlib.platform import Platform
 from devlib.exception import (DevlibTransientError, TargetStableError,
                               TargetNotRespondingError, TimeoutError,
                               TargetTransientError, KernelConfigKeyError,
-                              TargetError, HostError) # pylint: disable=redefined-builtin
+                              TargetError, HostError, TargetCalledProcessError) # pylint: disable=redefined-builtin
 from devlib.utils.ssh import SshConnection
 from devlib.utils.android import AdbConnection, AndroidProperties, LogcatMonitor, adb_command, adb_disconnect, INTENT_FLAGS
 from devlib.utils.misc import memoized, isiterable, convert_new_lines, groupby_value
@@ -848,12 +848,44 @@ class Target(object):
 
     def write_value(self, path, value, verify=True):
         value = str(value)
-        self.execute('echo {} > {}'.format(quote(value), quote(path)), check_exit_code=False, as_root=True)
+
         if verify:
-            output = self.read_value(path)
-            if not output == value:
-                message = 'Could not set the value of {} to "{}" (read "{}")'.format(path, value, output)
+            # Check in a loop for a while since updates to sysfs files can take
+            # some time to be observed, typically when a write triggers a
+            # lengthy kernel-side request, and the read is based on some piece
+            # of state that may take some time to be updated by the write
+            # request, such as hotplugging a CPU.
+            cmd = '''
+orig=$(cat {path} 2>/dev/null || printf "")
+printf "%s" {value} > {path} || exit 10
+if [ {value} != "$orig" ]; then
+   trials=0
+   while [ "$(cat {path} 2>/dev/null)" != {value} ]; do
+       if [ $trials -ge 10 ]; then
+           cat {path}
+           exit 11
+       fi
+       sleep 0.01
+       trials=$((trials + 1))
+   done
+fi
+'''
+        else:
+            cmd = '{busybox} printf "%s" {value} > {path}'
+        cmd = cmd.format(busybox=quote(self.busybox), path=quote(path), value=quote(value))
+
+        try:
+            self.execute(cmd, check_exit_code=True, as_root=True)
+        except TargetCalledProcessError as e:
+            if e.returncode == 10:
+                raise TargetStableError('Could not write "{value}" to {path}: {e.output}'.format(
+                    value=value, path=path, e=e))
+            elif verify and e.returncode == 11:
+                out = e.output
+                message = 'Could not set the value of {} to "{}" (read "{}")'.format(path, value, out)
                 raise TargetStableError(message)
+            else:
+                raise
 
     def reset(self):
         try: