From bfb472171560f7bbdad0bfbafb6da7afeaedacb1 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Tue, 3 Oct 2017 16:47:11 +0100 Subject: [PATCH 1/6] Target: ensure shutils is always set Make sure shutils is always set, even if setup() has not been called by initializing it on first access if necessary. --- devlib/target.py | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/devlib/target.py b/devlib/target.py index 4b2da42..f84c6a1 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -149,6 +149,12 @@ class Target(object): else: return None + @property + def shutils(self): + if self._shutils is None: + self._setup_shutils() + return self._shutils + def __init__(self, connection_settings=None, platform=None, @@ -189,6 +195,7 @@ class Target(object): self._installed_modules = {} self._cache = {} self._connections = {} + self._shutils = None self.busybox = None if load_default_modules: @@ -229,20 +236,7 @@ class Target(object): self.execute('mkdir -p {}'.format(self.executables_directory)) self.busybox = self.install(os.path.join(PACKAGE_BIN_DIRECTORY, self.abi, 'busybox')) - # Setup shutils script for the target - shutils_ifile = os.path.join(PACKAGE_BIN_DIRECTORY, 'scripts', 'shutils.in') - shutils_ofile = os.path.join(PACKAGE_BIN_DIRECTORY, 'scripts', 'shutils') - shell_path = '/bin/sh' - if self.os == 'android': - shell_path = '/system/bin/sh' - with open(shutils_ifile) as fh: - lines = fh.readlines() - with open(shutils_ofile, 'w') as ofile: - for line in lines: - line = line.replace("__DEVLIB_SHELL__", shell_path) - line = line.replace("__DEVLIB_BUSYBOX__", self.busybox) - ofile.write(line) - self.shutils = self.install(os.path.join(PACKAGE_BIN_DIRECTORY, 'scripts', 'shutils')) + self._setup_shutils() for host_exe in (executables or []): # pylint: disable=superfluous-parens self.install(host_exe) @@ -622,6 +616,21 @@ class Target(object): # internal methods + def _setup_shutils(self): + shutils_ifile = os.path.join(PACKAGE_BIN_DIRECTORY, 'scripts', 'shutils.in') + shutils_ofile = os.path.join(PACKAGE_BIN_DIRECTORY, 'scripts', 'shutils') + shell_path = '/bin/sh' + if self.os == 'android': + shell_path = '/system/bin/sh' + with open(shutils_ifile) as fh: + lines = fh.readlines() + with open(shutils_ofile, 'w') as ofile: + for line in lines: + line = line.replace("__DEVLIB_SHELL__", shell_path) + line = line.replace("__DEVLIB_BUSYBOX__", self.busybox) + ofile.write(line) + self._shutils = self.install(os.path.join(PACKAGE_BIN_DIRECTORY, 'scripts', 'shutils')) + def _execute_util(self, command, timeout=None, check_exit_code=True, as_root=False): command = '{} {}'.format(self.shutils, command) return self.conn.execute(command, timeout, check_exit_code, as_root) From 92fb54d57b1c0d221101fae4c6aeb56a5948b3e2 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Tue, 3 Oct 2017 16:50:23 +0100 Subject: [PATCH 2/6] module: nicer logger name Use Module.name rather than Module.__name__ to name the logger for slightly more readable log output. --- devlib/module/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devlib/module/__init__.py b/devlib/module/__init__.py index 5cf0a43..38a2315 100644 --- a/devlib/module/__init__.py +++ b/devlib/module/__init__.py @@ -56,7 +56,7 @@ class Module(object): def __init__(self, target): self.target = target - self.logger = logging.getLogger(self.__class__.__name__) + self.logger = logging.getLogger(self.name) class HardRestModule(Module): # pylint: disable=R0921 From 181bc180c4f9f13208e4e19e3f2ed2d1013b7db1 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Tue, 3 Oct 2017 16:28:09 +0100 Subject: [PATCH 3/6] Target: add read_tree_values and read_tree_values_flat Add two new methods to target that allow querying values of all sysfs nodes in a sub-directory structure all at once. The difference is that read_tree_values_flat returns a flat dict of path->value mappings; read_tree_values returns a nested dict structure that mimics the scanned sub-directories tree. --- devlib/bin/scripts/shutils.in | 19 ++++++++++++++++ devlib/target.py | 43 +++++++++++++++++++++++++++++++++++ doc/target.rst | 26 +++++++++++++++++++++ 3 files changed, 88 insertions(+) diff --git a/devlib/bin/scripts/shutils.in b/devlib/bin/scripts/shutils.in index 6d78be7..004030d 100755 --- a/devlib/bin/scripts/shutils.in +++ b/devlib/bin/scripts/shutils.in @@ -195,6 +195,22 @@ cgroups_freezer_set_state() { exit 1 } +################################################################################ +# Misc +################################################################################ + +read_tree_values() { + PATH=$1 + MAXDEPTH=$2 + + PATHS=$($BUSYBOX find $PATH -follow -maxdepth $MAXDEPTH) + if [ ${#PATHS[@]} -eq 0 ]; then + echo "ERROR: '$1' does not exist" + else + $BUSYBOX grep -s '' $PATHS + fi +} + ################################################################################ # Main Function Dispatcher ################################################################################ @@ -236,6 +252,9 @@ cgroups_freezer_set_state) ftrace_get_function_stats) ftrace_get_function_stats ;; +read_tree_values) + read_tree_values $* + ;; *) echo "Command [$CMD] not supported" exit -1 diff --git a/devlib/target.py b/devlib/target.py index f84c6a1..8609fec 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -614,6 +614,20 @@ class Target(object): timeout = duration + 10 self.execute('sleep {}'.format(duration), timeout=timeout) + def read_tree_values_flat(self, path, depth=1, check_exit_code=True): + command = 'read_tree_values {} {}'.format(path, depth) + output = self._execute_util(command, as_root=self.is_rooted, + check_exit_code=check_exit_code) + result = {} + for entry in output.strip().split('\n'): + path, value = entry.strip().split(':', 1) + result[path] = value + return result + + def read_tree_values(self, path, depth=1, dictcls=dict, check_exit_code=True): + value_map = self.read_tree_values_flat(path, depth, check_exit_code) + return _build_path_tree(value_map, path, self.path.sep, dictcls) + # internal methods def _setup_shutils(self): @@ -1558,3 +1572,32 @@ def _get_part_name(section): if name is None: name = '{}/{}/{}'.format(implementer, part, variant) return name + + +def _build_path_tree(path_map, basepath, sep=os.path.sep, dictcls=dict): + """ + Convert a flat mapping of paths to values into a nested structure of + dict-line object (``dict``'s by default), mirroring the directory hierarchy + represented by the paths relative to ``basepath``. + + """ + def process_node(node, path, value): + parts = path.split(sep, 1) + if len(parts) == 1: # leaf + node[parts[0]] = value + else: # branch + if parts[0] not in node: + node[parts[0]] = dictcls() + process_node(node[parts[0]], parts[1], value) + + relpath_map = {os.path.relpath(p, basepath): v + for p, v in path_map.iteritems()} + + if len(relpath_map) == 1 and relpath_map.keys()[0] == '.': + result = relpath_map.values()[0] + else: + result = dictcls() + for path, value in relpath_map.iteritems(): + process_node(result, path, value) + + return result diff --git a/doc/target.rst b/doc/target.rst index 08472e2..ae6ddb0 100644 --- a/doc/target.rst +++ b/doc/target.rst @@ -327,6 +327,32 @@ Target some sysfs entries silently failing to set the written value without returning an error code. +.. method:: Target.read_tree_values(path, depth=1, dictcls=dict): + + Read values of all sysfs (or similar) file nodes under ``path``, traversing + up to the maximum depth ``depth``. + + Returns a nested structure of dict-like objects (``dict``\ s by default) that + follows the structure of the scanned sub-directory tree. The top-level entry + has a single item who's key is ``path``. If ``path`` points to a single file, + the value of the entry is the value ready from that file node. Otherwise, the + value is a dict-line object with a key for every entry under ``path`` + mapping onto its value or further dict-like objects as appropriate. + + :param path: sysfs path to scan + :param depth: maximum depth to descend + :param dictcls: a dict-like type to be used for each level of the hierarchy. + +.. method:: Target.read_tree_values_flat(path, depth=1): + + Read values of all sysfs (or similar) file nodes under ``path``, traversing + up to the maximum depth ``depth``. + + Returns a dict mapping paths of file nodes to corresponding values. + + :param path: sysfs path to scan + :param depth: maximum depth to descend + .. method:: Target.reset() Soft reset the target. Typically, this means executing ``reboot`` on the From 5a599f91db437d3a95f3ae15e1113704418fa9c1 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Tue, 3 Oct 2017 16:52:40 +0100 Subject: [PATCH 4/6] module/hwmon: optimize initialization. Optimize hwmon module initialization by using the new Target.grep_values call to get information about all HWMON devices in a single call to the target. --- devlib/module/hwmon.py | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/devlib/module/hwmon.py b/devlib/module/hwmon.py index 06fa550..d04bce7 100644 --- a/devlib/module/hwmon.py +++ b/devlib/module/hwmon.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. # +import os import re from collections import defaultdict @@ -78,16 +79,15 @@ class HwmonDevice(object): all_sensors.extend(sensors_of_kind.values()) return all_sensors - def __init__(self, target, path): + def __init__(self, target, path, name, fields): self.target = target self.path = path - self.name = self.target.read_value(self.target.path.join(self.path, 'name')) + self.name = name self._sensors = defaultdict(dict) path = self.path if not path.endswith(self.target.path.sep): path += self.target.path.sep - for entry in self.target.list_directory(path, - as_root=self.target.is_rooted): + for entry in fields: match = HWMON_FILE_REGEX.search(entry) if match: kind = match.group('kind') @@ -117,14 +117,11 @@ class HwmonModule(Module): @staticmethod def probe(target): - if not target.file_exists(HWMON_ROOT): - return False try: target.list_directory(HWMON_ROOT, as_root=target.is_rooted) except TargetError: - # Probably no permissions + # Doesn't exist or no permissions return False - return True @property @@ -141,11 +138,13 @@ class HwmonModule(Module): self.scan() def scan(self): - for entry in self.target.list_directory(self.root, - as_root=self.target.is_rooted): - if entry.startswith('hwmon'): - entry_path = self.target.path.join(self.root, entry) - if self.target.file_exists(self.target.path.join(entry_path, 'name')): - device = HwmonDevice(self.target, entry_path) - self.devices.append(device) + values_tree = self.target.read_tree_values(self.root, depth=3) + for entry_id, fields in values_tree.iteritems(): + path = self.target.path.join(self.root, entry_id) + name = fields.pop('name', None) + if name is None: + continue + self.logger.debug('Adding device {}'.format(name)) + device = HwmonDevice(self.target, path, name, fields) + self.devices.append(device) From d7ca39e4d13ea1bebb12cd27ae09835f683ed327 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Wed, 4 Oct 2017 09:28:09 +0100 Subject: [PATCH 5/6] module/cpuidle: optimize initialization. Optimize cpuidle module initialization by using the new Target.grep_values call to get information about all idle states in a single call to the target during module intialization, rather lazily fetching them from the target afterwards. --- devlib/module/cpuidle.py | 98 +++++++++++++++++++--------------------- 1 file changed, 47 insertions(+), 51 deletions(-) diff --git a/devlib/module/cpuidle.py b/devlib/module/cpuidle.py index fd986c0..b17f1f5 100644 --- a/devlib/module/cpuidle.py +++ b/devlib/module/cpuidle.py @@ -41,51 +41,17 @@ class CpuidleState(object): raise ValueError('invalid idle state name: "{}"'.format(self.id)) return int(self.id[i:]) - def __init__(self, target, index, path): + def __init__(self, target, index, path, name, desc, power, latency, residency): self.target = target self.index = index self.path = path + self.name = name + self.desc = desc + self.power = power + self.latency = latency self.id = self.target.path.basename(self.path) self.cpu = self.target.path.basename(self.target.path.dirname(path)) - @property - @memoized - def desc(self): - return self.get('desc') - - @property - @memoized - def name(self): - return self.get('name') - - @property - @memoized - def latency(self): - """Exit latency in uS""" - return self.get('latency') - - @property - @memoized - def power(self): - """Power usage in mW - - ..note:: - - This value is not always populated by the kernel and may be garbage. - """ - return self.get('power') - - @property - @memoized - def target_residency(self): - """Target residency in uS - - This is the amount of time in the state required to 'break even' on - power - the system should avoid entering the state for less time than - this. - """ - return self.get('residency') - def enable(self): self.set('disable', 0) @@ -126,23 +92,47 @@ class Cpuidle(Module): def probe(target): return target.file_exists(Cpuidle.root_path) - def get_driver(self): - return self.target.read_value(self.target.path.join(self.root_path, 'current_driver')) + def __init__(self, target): + super(Cpuidle, self).__init__(target) + self._states = {} - def get_governor(self): - return self.target.read_value(self.target.path.join(self.root_path, 'current_governor_ro')) + basepath = '/sys/devices/system/cpu/' + values_tree = self.target.read_tree_values(basepath, depth=4, check_exit_code=False) + i = 0 + cpu_id = 'cpu{}'.format(i) + while cpu_id in values_tree: + cpu_node = values_tree[cpu_id] + + if 'cpuidle' in cpu_node: + idle_node = cpu_node['cpuidle'] + self._states[cpu_id] = [] + j = 0 + state_id = 'state{}'.format(j) + while state_id in idle_node: + state_node = idle_node[state_id] + state = CpuidleState( + self.target, + index=j, + path=self.target.path.join(basepath, cpu_id, 'cpuidle', state_id), + name=state_node['name'], + desc=state_node['desc'], + power=int(state_node['power']), + latency=int(state_node['latency']), + residency=int(state_node['residency']) if 'residency' in state_node else None, + ) + msg = 'Adding {} state {}: {} {}' + self.logger.debug(msg.format(cpu_id, j, state.name, state.desc)) + self._states[cpu_id].append(state) + j += 1 + state_id = 'state{}'.format(j) + + i += 1 + cpu_id = 'cpu{}'.format(i) - @memoized def get_states(self, cpu=0): if isinstance(cpu, int): cpu = 'cpu{}'.format(cpu) - states_dir = self.target.path.join(self.target.path.dirname(self.root_path), cpu, 'cpuidle') - idle_states = [] - for state in self.target.list_directory(states_dir): - if state.startswith('state'): - index = int(state[5:]) - idle_states.append(CpuidleState(self.target, index, self.target.path.join(states_dir, state))) - return idle_states + return self._states.get(cpu) def get_state(self, state, cpu=0): if isinstance(state, int): @@ -176,3 +166,9 @@ class Cpuidle(Module): """ output = self.target._execute_util('cpuidle_wake_all_cpus') print(output) + + def get_driver(self): + return self.target.read_value(self.target.path.join(self.root_path, 'current_driver')) + + def get_governor(self): + return self.target.read_value(self.target.path.join(self.root_path, 'current_governor_ro')) From f0426467922bb8cb195de0f73c10ddf9bcf5d835 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Wed, 4 Oct 2017 13:19:26 +0100 Subject: [PATCH 6/6] module/hotplug: optimize online_all Optimize online_all by offloading it to a shutils function which only requres a single call to the target. --- devlib/bin/scripts/shutils.in | 16 ++++++++++++++++ devlib/module/hotplug.py | 3 ++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/devlib/bin/scripts/shutils.in b/devlib/bin/scripts/shutils.in index 004030d..a678a65 100755 --- a/devlib/bin/scripts/shutils.in +++ b/devlib/bin/scripts/shutils.in @@ -195,6 +195,19 @@ cgroups_freezer_set_state() { exit 1 } +################################################################################ +# Hotplug +################################################################################ + +hotplug_online_all() { + PATHS=(/sys/devices/system/cpu/cpu[0-9]*) + for path in "${PATHS[@]}"; do + if [ $(cat $path/online) -eq 0 ]; then + echo 1 > $path/online + fi + done +} + ################################################################################ # Misc ################################################################################ @@ -252,6 +265,9 @@ cgroups_freezer_set_state) ftrace_get_function_stats) ftrace_get_function_stats ;; +hotplug_online_all) + hotplug_online_all + ;; read_tree_values) read_tree_values $* ;; diff --git a/devlib/module/hotplug.py b/devlib/module/hotplug.py index 8ae238e..cfce2e5 100644 --- a/devlib/module/hotplug.py +++ b/devlib/module/hotplug.py @@ -21,7 +21,8 @@ class HotplugModule(Module): return target.path.join(cls.base_path, cpu, 'online') def online_all(self): - self.online(*range(self.target.number_of_cpus)) + self.target._execute_util('hotplug_online_all', + as_root=self.target.is_rooted) def online(self, *args): for cpu in args: