Skip to content

Commit fb416b8

Browse files
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.
1 parent bf6edda commit fb416b8

File tree

1 file changed

+36
-5
lines changed

1 file changed

+36
-5
lines changed

devlib/target.py

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@
5050
from devlib.exception import (DevlibTransientError, TargetStableError,
5151
TargetNotRespondingError, TimeoutError,
5252
TargetTransientError, KernelConfigKeyError,
53-
TargetError, HostError) # pylint: disable=redefined-builtin
53+
TargetError, HostError, TargetCalledProcessError) # pylint: disable=redefined-builtin
5454
from devlib.utils.ssh import SshConnection
5555
from devlib.utils.android import AdbConnection, AndroidProperties, LogcatMonitor, adb_command, adb_disconnect, INTENT_FLAGS
5656
from devlib.utils.misc import memoized, isiterable, convert_new_lines
@@ -736,12 +736,43 @@ def batch_revertable_write_value(self, kwargs_list):
736736

737737
def write_value(self, path, value, verify=True):
738738
value = str(value)
739-
self.execute('echo {} > {}'.format(quote(value), quote(path)), check_exit_code=False, as_root=True)
739+
740740
if verify:
741-
output = self.read_value(path)
742-
if not output == value:
743-
message = 'Could not set the value of {} to "{}" (read "{}")'.format(path, value, output)
741+
# Check in a loop for a while since updates to sysfs files can take
742+
# some time to be observed, typically when a write triggers a
743+
# lengthy kernel-side request, and the read is based on some piece
744+
# of state that may take some time to be updated by the write
745+
# request, such as hotplugging a CPU.
746+
cmd = '''
747+
orig=$(cat {path} 2>/dev/null || printf "")
748+
printf "%s" {value} > {path} || exit 10
749+
if [ {value} != "$orig" ]; then
750+
trials=0
751+
while [ "$(cat {path} 2>/dev/null)" != {value} ]; do
752+
if [ $trials -ge 10 ]; then
753+
cat {path}
754+
exit 11
755+
fi
756+
sleep 0.01
757+
trials=$((trials + 1))
758+
done
759+
fi
760+
'''
761+
else:
762+
cmd = 'printf "%s" {value} > {path}'
763+
cmd = cmd.format(path=quote(path), value=quote(value))
764+
765+
try:
766+
self.execute(cmd, check_exit_code=True, as_root=True)
767+
except TargetCalledProcessError as e:
768+
if e.returncode == 10:
769+
raise TargetStableError(f'Could not write "{value}" to {path}: {e.output}')
770+
elif verify and e.returncode == 11:
771+
out = e.output
772+
message = 'Could not set the value of {} to "{}" (read "{}")'.format(path, value, out)
744773
raise TargetStableError(message)
774+
else:
775+
raise
745776

746777
def reset(self):
747778
try:

0 commit comments

Comments
 (0)