From d74eeff14852c2a5d9f8b0e8da7c35a2cac57b19 Mon Sep 17 00:00:00 2001 From: ooonea <35407790+ooonea@users.noreply.github.com> Date: Sun, 19 Apr 2026 21:28:18 +0200 Subject: [PATCH] Fix correctness bugs in throttled.py and mmio.py - get_msr_list(): sort numerically and skip non-digit entries (like /dev/cpu/microcode). os.listdir() ordering is arbitrary, so the previous code returned readmsr() results indexed by enumeration order rather than CPU number, breaking the cpu= and flatten=True paths on systems where listdir didn't happen to return CPUs in order; it would also crash with ValueError when /dev/cpu contained any non-CPU entry. - readmsr/writemsr: close MSR file descriptors via try/finally so a failed read/write doesn't leak the fd. - is_on_battery(): replace bare `raise` (no active exception) and catch-all `except:` with specific exceptions; restore the intended fall-through to the upower fallback. - test_msr_rw_capabilities(): declare TESTMSR global. The previous `TESTMSR = True` only created a local, so the in-flight detection could not actually catch MSR errors and would call fatal() instead. - power_thread(): always sleep at end of loop. Previously the exit_event.wait was only reached in the else-branch of the HWP check, so when HWP fired the thread spun without sleeping. - set_disable_bdprochot(): use `read_value == 0` for the debug match check; `~read_value` was always truthy. - main(): initialize cpuid = None so --force does not raise UnboundLocalError when starting the power thread. - mmio.py: map /dev/mem with PROT_READ | PROT_WRITE so read32 is defined and portable; close fd if mmap.mmap raises; fix __str__ which referenced nonexistent self.base/self.size; drop dead self._fd assignment. Signed-off-by: ooonea <35407790+ooonea@users.noreply.github.com> --- mmio.py | 12 ++++++---- throttled.py | 65 +++++++++++++++++++++++++++++----------------------- 2 files changed, 44 insertions(+), 33 deletions(-) diff --git a/mmio.py b/mmio.py index 4568af9..27c343b 100644 --- a/mmio.py +++ b/mmio.py @@ -61,8 +61,14 @@ class MMIO(object): try: self.mapping = mmap.mmap( - fd, self._aligned_size, flags=mmap.MAP_SHARED, prot=mmap.PROT_WRITE, offset=self._aligned_physaddr) + fd, + self._aligned_size, + flags=mmap.MAP_SHARED, + prot=mmap.PROT_READ | mmap.PROT_WRITE, + offset=self._aligned_physaddr, + ) except OSError as e: + os.close(fd) raise MMIOError(e.errno, "Mapping /dev/mem: " + e.strerror) try: @@ -126,9 +132,7 @@ class MMIO(object): self.mapping.close() self.mapping = None - self._fd = None - # String representation def __str__(self): - return "MMIO 0x%08x (size=%d)" % (self.base, self.size) + return "MMIO 0x%08x (size=%d)" % (self._physaddr, self._size) diff --git a/throttled.py b/throttled.py index dcd474d..9396d5a 100755 --- a/throttled.py +++ b/throttled.py @@ -223,7 +223,8 @@ def warning(msg, oneshot=True, end='\n'): def get_msr_list(): - return ['/dev/cpu/{:d}/msr'.format(int(x)) for x in os.listdir("/dev/cpu")] + cpus = sorted(int(x) for x in os.listdir('/dev/cpu') if x.isdigit()) + return ['/dev/cpu/{:d}/msr'.format(cpu) for cpu in cpus] def writemsr(msr, val): msr_list = get_msr_list() @@ -235,9 +236,11 @@ def writemsr(msr, val): try: for addr in msr_list: f = os.open(addr, os.O_WRONLY) - os.lseek(f, MSR_DICT[msr], os.SEEK_SET) - os.write(f, struct.pack('Q', val)) - os.close(f) + try: + os.lseek(f, MSR_DICT[msr], os.SEEK_SET) + os.write(f, struct.pack('Q', val)) + finally: + os.close(f) except (IOError, OSError) as e: if TESTMSR: raise e @@ -267,9 +270,11 @@ def readmsr(msr, from_bit=0, to_bit=63, cpu=None, flatten=False): output = [] for addr in msr_list: f = os.open(addr, os.O_RDONLY) - os.lseek(f, MSR_DICT[msr], os.SEEK_SET) - val = struct.unpack('Q', os.read(f, 8))[0] - os.close(f) + try: + os.lseek(f, MSR_DICT[msr], os.SEEK_SET) + val = struct.unpack('Q', os.read(f, 8))[0] + finally: + os.close(f) output.append(get_value_for_bits(val, from_bit, to_bit)) if flatten: if len(set(output)) > 1: @@ -312,15 +317,16 @@ def is_on_battery(config): for path in glob.glob(config.get('GENERAL', 'Sysfs_Power_Path', fallback=DEFAULT_SYSFS_POWER_PATH)): with open(path) as f: return not bool(int(f.read())) - raise - except: + except (IOError, OSError, ValueError) as e: + warning('Sysfs_Power_Path read failed ({}). Trying upower method.'.format(e)) + else: warning('No valid Sysfs_Power_Path found! Trying upower method') try: bus = dbus.SystemBus() proxy = bus.get_object('org.freedesktop.UPower', '/org/freedesktop/UPower') iface = dbus.Interface(proxy, 'org.freedesktop.DBus.Properties') - return iface.Get('org.freedesktop.UPower', 'OnBattery') - except: + return bool(iface.Get('org.freedesktop.UPower', 'OnBattery')) + except dbus.DBusException: pass warning('No valid power detection methods found. Assuming that the system is running on battery power.') @@ -660,7 +666,7 @@ def set_disable_bdprochot(): writemsr('MSR_POWER_CTL', new_val) if args.debug: read_value = readmsr('MSR_POWER_CTL', from_bit=0, to_bit=0)[0] - match = OK if ~read_value else ERR + match = OK if read_value == 0 else ERR log('[D] BDPROCHOT - write "{:#02x}" - read "{:#02x}" - match {}'.format(0, read_value, match)) @@ -790,8 +796,7 @@ def power_thread(config, regs, exit_event, cpuid): set_hwp(enable_hwp_mode) next_hwp_write = time() + HWP_INTERVAL - else: - exit_event.wait(wait_t) + exit_event.wait(wait_t) def check_kernel(): @@ -859,24 +864,25 @@ def check_cpu(): def test_msr_rw_capabilities(): + global TESTMSR TESTMSR = True - try: - log('[I] Testing if undervolt is supported...') - get_undervolt() - except: - warning('Undervolt seems not to be supported by your system, disabling.') - UNSUPPORTED_FEATURES.append('UNDERVOLT') + try: + log('[I] Testing if undervolt is supported...') + get_undervolt() + except (IOError, OSError): + warning('Undervolt seems not to be supported by your system, disabling.') + UNSUPPORTED_FEATURES.append('UNDERVOLT') - try: - log('[I] Testing if HWP is supported...') - cur_val = readmsr('IA32_HWP_REQUEST', cpu=0) - writemsr('IA32_HWP_REQUEST', cur_val) - except: - warning('HWP seems not to be supported by your system, disabling.') - UNSUPPORTED_FEATURES.append('HWP') - - TESTMSR = False + try: + log('[I] Testing if HWP is supported...') + cur_val = readmsr('IA32_HWP_REQUEST', cpu=0) + writemsr('IA32_HWP_REQUEST', cur_val) + except (IOError, OSError): + warning('HWP seems not to be supported by your system, disabling.') + UNSUPPORTED_FEATURES.append('HWP') + finally: + TESTMSR = False def monitor(exit_event, wait): @@ -958,6 +964,7 @@ def main(): args.log = None fatal('Unable to write to the log file!') + cpuid = None if not args.force: check_kernel() cpuid = check_cpu()