From 41cadf537bbcaded332a82d24fbd3ba809c25aa3 Mon Sep 17 00:00:00 2001 From: Arnon Yaari Date: Tue, 6 Feb 2018 21:56:44 +0200 Subject: [PATCH] Ignore negative deltas in cpu times when calculating percentages (#1210) --- psutil/__init__.py | 61 +++++++++++++++++++------------------- psutil/tests/test_linux.py | 46 ++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 31 deletions(-) diff --git a/psutil/__init__.py b/psutil/__init__.py index 5e29a7fc5b..db918c329a 100644 --- a/psutil/__init__.py +++ b/psutil/__init__.py @@ -1656,6 +1656,26 @@ def _cpu_busy_time(times): return busy +def _cpu_times_deltas(t1, t2): + field_deltas = [] + for field in _psplatform.scputimes._fields: + field_delta = getattr(t2, field) - getattr(t1, field) + # CPU times are always supposed to increase over time + # or at least remain the same and that's because time + # cannot go backwards. + # Surprisingly sometimes this might not be the case (at + # least on Windows and Linux), see: + # https://github.com/giampaolo/psutil/issues/392 + # https://github.com/giampaolo/psutil/issues/645 + # https://github.com/giampaolo/psutil/issues/1210 + # Trim negative deltas to zero to ignore decreasing fields. + # top does the same. Reference: + # https://gitlab.com/procps-ng/procps/blob/v3.3.12/top/top.c#L5063 + field_delta = max(0, field_delta) + field_deltas.append(field_delta) + return _psplatform.scputimes(*field_deltas) + + def cpu_percent(interval=None, percpu=False): """Return a float representing the current system-wide CPU utilization as a percentage. @@ -1698,18 +1718,11 @@ def cpu_percent(interval=None, percpu=False): raise ValueError("interval is not positive (got %r)" % interval) def calculate(t1, t2): - t1_all = _cpu_tot_time(t1) - t1_busy = _cpu_busy_time(t1) + times_delta = _cpu_times_deltas(t1, t2) - t2_all = _cpu_tot_time(t2) - t2_busy = _cpu_busy_time(t2) + all_delta = _cpu_tot_time(times_delta) + busy_delta = _cpu_busy_time(times_delta) - # this usually indicates a float precision issue - if t2_busy <= t1_busy: - return 0.0 - - busy_delta = t2_busy - t1_busy - all_delta = t2_all - t1_all try: busy_perc = (busy_delta / all_delta) * 100 except ZeroDivisionError: @@ -1778,28 +1791,14 @@ def cpu_times_percent(interval=None, percpu=False): def calculate(t1, t2): nums = [] - all_delta = _cpu_tot_time(t2) - _cpu_tot_time(t1) - for field in t1._fields: - field_delta = getattr(t2, field) - getattr(t1, field) - try: - field_perc = (100 * field_delta) / all_delta - except ZeroDivisionError: - field_perc = 0.0 + times_delta = _cpu_times_deltas(t1, t2) + all_delta = _cpu_tot_time(times_delta) + scale = 100.0 / max(1, all_delta) + for field_delta in times_delta: + field_perc = field_delta * scale field_perc = round(field_perc, 1) - # CPU times are always supposed to increase over time - # or at least remain the same and that's because time - # cannot go backwards. - # Surprisingly sometimes this might not be the case (at - # least on Windows and Linux), see: - # https://github.com/giampaolo/psutil/issues/392 - # https://github.com/giampaolo/psutil/issues/645 - # I really don't know what to do about that except - # forcing the value to 0 or 100. - if field_perc > 100.0: - field_perc = 100.0 - # `<=` because `-0.0 == 0.0` evaluates to True - elif field_perc <= 0.0: - field_perc = 0.0 + # make sure we don't return negative values or values over 100% + field_perc = min(max(0.0, field_perc), 100.0) nums.append(field_perc) return _psplatform.scputimes(*nums) diff --git a/psutil/tests/test_linux.py b/psutil/tests/test_linux.py index 6ba17b2543..5307a29f3b 100755 --- a/psutil/tests/test_linux.py +++ b/psutil/tests/test_linux.py @@ -1090,6 +1090,52 @@ def open_mock(name, *args, **kwargs): self.assertEqual(psutil.PROCFS_PATH, '/proc') + def test_cpu_steal_decrease(self): + # test cumulative cpu stats decrease. We should ignore this. + # see issue #1210. + my_procfs = tempfile.mkdtemp() + + # start with "steal" = 1 and everything else = 0 + with open(os.path.join(my_procfs, 'stat'), 'w') as f: + f.write('cpu 0 0 0 0 0 0 0 1 0 0\n') + f.write('cpu0 0 0 0 0 0 0 0 1 0 0\n') + f.write('cpu1 0 0 0 0 0 0 0 1 0 0\n') + + try: + psutil.PROCFS_PATH = my_procfs + + # first call to "percent" functions should read the new stat file + # and compare to the "real" file read at import time - so the + # values are meaningless + psutil.cpu_percent() + psutil.cpu_percent(percpu=True) + psutil.cpu_times_percent() + psutil.cpu_times_percent(percpu=True) + + # increase "user" while steal goes "backwards" to zero + with open(os.path.join(my_procfs, 'stat'), 'w') as f: + f.write('cpu 1 0 0 0 0 0 0 0 0 0\n') + f.write('cpu0 1 0 0 0 0 0 0 0 0 0\n') + f.write('cpu1 1 0 0 0 0 0 0 0 0 0\n') + + cpu_percent = psutil.cpu_percent() + cpu_percent_percpu = psutil.cpu_percent(percpu=True) + cpu_times_percent = psutil.cpu_times_percent() + cpu_times_percent_percpu = psutil.cpu_times_percent(percpu=True) + self.assertNotEqual(cpu_percent, 0) + self.assertNotEqual(sum(cpu_percent_percpu), 0) + self.assertNotEqual(sum(cpu_times_percent), 0) + self.assertNotEqual(sum(cpu_times_percent), 100.0) + self.assertNotEqual(sum(map(sum, cpu_times_percent_percpu)), 0) + self.assertNotEqual(sum(map(sum, cpu_times_percent_percpu)), 100.0) + self.assertEqual(cpu_times_percent.steal, 0) + self.assertNotEqual(cpu_times_percent.user, 0) + finally: + shutil.rmtree(my_procfs) + reload_module(psutil) + + self.assertEqual(psutil.PROCFS_PATH, '/proc') + def test_boot_time_mocked(self): with mock.patch('psutil._pslinux.open', create=True) as m: self.assertRaises(