Merge branch 'master' of github.com:giampaolo/psutil

3 files changed, 107 insertions, 68 deletions

diff --git a/HISTORY.rst b/HISTORY.rst
index 1e259cbb..a86f56c4 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -12,6 +12,10 @@
 
 **Bug fixes**
 
+- 1915_, [Linux]: on certain kernels, ``"MemAvailable"`` field from
+  ``/proc/meminfo`` returns ``0`` (possibly a kernel bug), in which case we
+  calculate an approximation for ``available`` memory which matches "free"
+  CLI utility.
 - 2164_, [Linux]: compilation fails on kernels < 2.6.27 (e.g. CentOS 5).
 - 2186_, [FreeBSD]: compilation fails with Clang 15.  (patch by Po-Chuan Hsieh)
 - 2191_, [Linux]: `disk_partitions()`_: do not unnecessarily read
diff --git a/psutil/_pslinux.py b/psutil/_pslinux.py
index 11e62944..1bdeabfe 100644
--- a/psutil/_pslinux.py
+++ b/psutil/_pslinux.py
@@ -344,34 +344,43 @@ if prlimit is not None:
 
 def calculate_avail_vmem(mems):
     """Fallback for kernels < 3.14 where /proc/meminfo does not provide
-    "MemAvailable:" column, see:
+    "MemAvailable", see:
     https://blog.famzah.net/2014/09/24/
+
     This code reimplements the algorithm outlined here:
     https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/
         commit/?id=34e431b0ae398fc54ea69ff85ec700722c9da773
 
-    XXX: on recent kernels this calculation differs by ~1.5% than
-    "MemAvailable:" as it's calculated slightly differently, see:
-    https://gitlab.com/procps-ng/procps/issues/42
-    https://github.com/famzah/linux-memavailable-procfs/issues/2
+    We use this function also when "MemAvailable" returns 0 (possibly a
+    kernel bug, see: https://github.com/giampaolo/psutil/issues/1915).
+    In that case this routine matches "free" CLI tool result ("available"
+    column).
+
+    XXX: on recent kernels this calculation may differ by ~1.5% compared
+    to "MemAvailable:", as it's calculated slightly differently.
     It is still way more realistic than doing (free + cached) though.
+    See:
+    * https://gitlab.com/procps-ng/procps/issues/42
+    * https://github.com/famzah/linux-memavailable-procfs/issues/2
     """
-    # Fallback for very old distros. According to
+    # Note about "fallback" value. According to:
     # https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/
     #     commit/?id=34e431b0ae398fc54ea69ff85ec700722c9da773
-    # ...long ago "avail" was calculated as (free + cached).
-    # We might fallback in such cases:
-    # "Active(file)" not available: 2.6.28 / Dec 2008
-    # "Inactive(file)" not available: 2.6.28 / Dec 2008
-    # "SReclaimable:" not available: 2.6.19 / Nov 2006
-    # /proc/zoneinfo not available: 2.6.13 / Aug 2005
+    # ...long ago "available" memory was calculated as (free + cached),
+    # We use fallback when one of these is missing from /proc/meminfo:
+    # "Active(file)": introduced in 2.6.28 / Dec 2008
+    # "Inactive(file)": introduced in 2.6.28 / Dec 2008
+    # "SReclaimable": introduced in 2.6.19 / Nov 2006
+    # /proc/zoneinfo: introduced in 2.6.13 / Aug 2005
     free = mems[b'MemFree:']
     fallback = free + mems.get(b"Cached:", 0)
     try:
         lru_active_file = mems[b'Active(file):']
         lru_inactive_file = mems[b'Inactive(file):']
         slab_reclaimable = mems[b'SReclaimable:']
-    except KeyError:
+    except KeyError as err:
+        debug("%r is missing from /proc/meminfo; using an approximation "
+              "for calculating available memory" % err.args[0])
         return fallback
     try:
         f = open_binary('%s/zoneinfo' % get_procfs_path())
@@ -396,19 +405,11 @@ def calculate_avail_vmem(mems):
 
 def virtual_memory():
     """Report virtual memory stats.
-    This implementation matches "free" and "vmstat -s" cmdline
-    utility values and procps-ng-3.3.12 source was used as a reference
-    (2016-09-18):
+    This implementation mimicks procps-ng-3.3.12, aka "free" CLI tool:
     https://gitlab.com/procps-ng/procps/blob/
-        24fd2605c51fccc375ab0287cec33aa767f06718/proc/sysinfo.c
-    For reference, procps-ng-3.3.10 is the version available on Ubuntu
-    16.04.
-
-    Note about "available" memory: up until psutil 4.3 it was
-    calculated as "avail = (free + buffers + cached)". Now
-    "MemAvailable:" column (kernel 3.14) from /proc/meminfo is used as
-    it's more accurate.
-    That matches "available" column in newer versions of "free".
+        24fd2605c51fccc375ab0287cec33aa767f06718/proc/sysinfo.c#L778-791
+    The returned values are supposed to match both "free" and "vmstat -s"
+    CLI tools.
     """
     missing_fields = []
     mems = {}
@@ -490,17 +491,23 @@ def virtual_memory():
         avail = mems[b'MemAvailable:']
     except KeyError:
         avail = calculate_avail_vmem(mems)
+    else:
+        if avail == 0:
+            # Yes, it can happen (probably a kernel bug):
+            # https://github.com/giampaolo/psutil/issues/1915
+            # In this case "free" CLI tool makes an estimate. We do the same,
+            # and it matches "free" CLI tool.
+            avail = calculate_avail_vmem(mems)
 
     if avail < 0:
         avail = 0
         missing_fields.append('available')
-
-    # If avail is greater than total or our calculation overflows,
-    # that's symptomatic of running within a LCX container where such
-    # values will be dramatically distorted over those of the host.
-    # https://gitlab.com/procps-ng/procps/blob/
-    #     24fd2605c51fccc375ab0287cec33aa767f06718/proc/sysinfo.c#L764
-    if avail > total:
+    elif avail > total:
+        # If avail is greater than total or our calculation overflows,
+        # that's symptomatic of running within a LCX container where such
+        # values will be dramatically distorted over those of the host.
+        # https://gitlab.com/procps-ng/procps/blob/
+        #     24fd2605c51fccc375ab0287cec33aa767f06718/proc/sysinfo.c#L764
         avail = free
 
     percent = usage_percent((total - avail), total, round_=1)
diff --git a/psutil/tests/test_linux.py b/psutil/tests/test_linux.py
index b8be6ca3..964416b3 100755
--- a/psutil/tests/test_linux.py
+++ b/psutil/tests/test_linux.py
@@ -250,12 +250,64 @@ def mock_open_exception(for_path, exc):
 
 
 @unittest.skipIf(not LINUX, "LINUX only")
-class TestSystemVirtualMemory(PsutilTestCase):
+class TestSystemVirtualMemoryAgainstFree(PsutilTestCase):
+
+    def test_total(self):
+        cli_value = free_physmem().total
+        psutil_value = psutil.virtual_memory().total
+        self.assertEqual(cli_value, psutil_value)
+
+    @retry_on_failure()
+    def test_used(self):
+        # Older versions of procps used slab memory to calculate used memory.
+        # This got changed in:
+        # https://gitlab.com/procps-ng/procps/commit/
+        #     05d751c4f076a2f0118b914c5e51cfbb4762ad8e
+        if get_free_version_info() < (3, 3, 12):
+            raise self.skipTest("old free version")
+        cli_value = free_physmem().used
+        psutil_value = psutil.virtual_memory().used
+        self.assertAlmostEqual(cli_value, psutil_value,
+                               delta=TOLERANCE_SYS_MEM)
+
+    @retry_on_failure()
+    def test_free(self):
+        cli_value = free_physmem().free
+        psutil_value = psutil.virtual_memory().free
+        self.assertAlmostEqual(cli_value, psutil_value,
+                               delta=TOLERANCE_SYS_MEM)
+
+    @retry_on_failure()
+    def test_shared(self):
+        free = free_physmem()
+        free_value = free.shared
+        if free_value == 0:
+            raise unittest.SkipTest("free does not support 'shared' column")
+        psutil_value = psutil.virtual_memory().shared
+        self.assertAlmostEqual(
+            free_value, psutil_value, delta=TOLERANCE_SYS_MEM,
+            msg='%s %s \n%s' % (free_value, psutil_value, free.output))
+
+    @retry_on_failure()
+    def test_available(self):
+        # "free" output format has changed at some point:
+        # https://github.com/giampaolo/psutil/issues/538#issuecomment-147192098
+        out = sh(["free", "-b"])
+        lines = out.split('\n')
+        if 'available' not in lines[0]:
+            raise unittest.SkipTest("free does not support 'available' column")
+        else:
+            free_value = int(lines[1].split()[-1])
+            psutil_value = psutil.virtual_memory().available
+            self.assertAlmostEqual(
+                free_value, psutil_value, delta=TOLERANCE_SYS_MEM,
+                msg='%s %s \n%s' % (free_value, psutil_value, out))
+
+
+@unittest.skipIf(not LINUX, "LINUX only")
+class TestSystemVirtualMemoryAgainstVmstat(PsutilTestCase):
 
     def test_total(self):
-        # free_value = free_physmem().total
-        # psutil_value = psutil.virtual_memory().total
-        # self.assertEqual(free_value, psutil_value)
         vmstat_value = vmstat('total memory') * 1024
         psutil_value = psutil.virtual_memory().total
         self.assertAlmostEqual(
@@ -269,12 +321,10 @@ class TestSystemVirtualMemory(PsutilTestCase):
         #     05d751c4f076a2f0118b914c5e51cfbb4762ad8e
         if get_free_version_info() < (3, 3, 12):
             raise self.skipTest("old free version")
-        free = free_physmem()
-        free_value = free.used
+        vmstat_value = vmstat('used memory') * 1024
         psutil_value = psutil.virtual_memory().used
         self.assertAlmostEqual(
-            free_value, psutil_value, delta=TOLERANCE_SYS_MEM,
-            msg='%s %s \n%s' % (free_value, psutil_value, free.output))
+            vmstat_value, psutil_value, delta=TOLERANCE_SYS_MEM)
 
     @retry_on_failure()
     def test_free(self):
@@ -304,31 +354,9 @@ class TestSystemVirtualMemory(PsutilTestCase):
         self.assertAlmostEqual(
             vmstat_value, psutil_value, delta=TOLERANCE_SYS_MEM)
 
-    @retry_on_failure()
-    def test_shared(self):
-        free = free_physmem()
-        free_value = free.shared
-        if free_value == 0:
-            raise unittest.SkipTest("free does not support 'shared' column")
-        psutil_value = psutil.virtual_memory().shared
-        self.assertAlmostEqual(
-            free_value, psutil_value, delta=TOLERANCE_SYS_MEM,
-            msg='%s %s \n%s' % (free_value, psutil_value, free.output))
 
-    @retry_on_failure()
-    def test_available(self):
-        # "free" output format has changed at some point:
-        # https://github.com/giampaolo/psutil/issues/538#issuecomment-147192098
-        out = sh(["free", "-b"])
-        lines = out.split('\n')
-        if 'available' not in lines[0]:
-            raise unittest.SkipTest("free does not support 'available' column")
-        else:
-            free_value = int(lines[1].split()[-1])
-            psutil_value = psutil.virtual_memory().available
-            self.assertAlmostEqual(
-                free_value, psutil_value, delta=TOLERANCE_SYS_MEM,
-                msg='%s %s \n%s' % (free_value, psutil_value, out))
+@unittest.skipIf(not LINUX, "LINUX only")
+class TestSystemVirtualMemoryMocks(PsutilTestCase):
 
     def test_warnings_on_misses(self):
         # Emulate a case where /proc/meminfo provides few info.
@@ -808,7 +836,7 @@ class TestSystemCPUFrequency(PsutilTestCase):
                     name.startswith("/sys/devices/system/cpu/cpufreq/policy")):
                 return io.BytesIO(b"700000")
             elif name == '/proc/cpuinfo':
-                return io.BytesIO(b"cpu MHz		: 500")
+                return io.BytesIO(b"cpu MHz     : 500")
             else:
                 return orig_open(name, *args, **kwargs)
 
@@ -849,8 +877,8 @@ class TestSystemCPUFrequency(PsutilTestCase):
                     n.startswith("/sys/devices/system/cpu/cpufreq/policy1")):
                 return io.BytesIO(b"600000")
             elif name == '/proc/cpuinfo':
-                return io.BytesIO(b"cpu MHz		: 100\n"
-                                  b"cpu MHz		: 400")
+                return io.BytesIO(b"cpu MHz     : 100\n"
+                                  b"cpu MHz     : 400")
             else:
                 return orig_open(name, *args, **kwargs)
 
@@ -881,7 +909,7 @@ class TestSystemCPUFrequency(PsutilTestCase):
             elif name.endswith('/cpuinfo_cur_freq'):
                 return io.BytesIO(b"200000")
             elif name == '/proc/cpuinfo':
-                return io.BytesIO(b"cpu MHz		: 200")
+                return io.BytesIO(b"cpu MHz     : 200")
             else:
                 return orig_open(name, *args, **kwargs)