Merge pull request #888 from giampaolo/887-linux-free-mem-standardization

887 linux free mem standardization

5 files changed, 144 insertions, 76 deletions

diff --git a/HISTORY.rst b/HISTORY.rst
index b5775bfd..34a34165 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -1,7 +1,7 @@
 Bug tracker at https://github.com/giampaolo/psutil/issues
 
 
-4.3.2 - XXXX-XX-XX
+4.4.0 - XXXX-XX-XX
 ==================
 
 **Bug fixes**
@@ -10,6 +10,9 @@ Bug tracker at https://github.com/giampaolo/psutil/issues
 - #880: [Windows] Handle race condition inside psutil_net_connections.
 - #885: ValueError is raised if a negative integer is passed to cpu_percent()
   functions.
+- #887: [Linux] free, available and used fields are more precise and match
+  "free" cmdline utility. It also takes into account LCX containers preventing
+  "avail" to overflow "total".
 
 
 4.3.1 - 2016-09-01
diff --git a/docs/index.rst b/docs/index.rst
index fa83aefa..d64ab43a 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -178,8 +178,8 @@ Memory
   - **available**: the actual amount of available memory that can be given
     instantly to processes that request more memory in bytes; this is
     calculated by summing different memory values depending on the platform
-    (e.g. ``(free + buffers + cached)`` on Linux) and it is supposed to be used
-    to monitor actual memory usage in a cross platform fashion.
+    and it is supposed to be used to monitor actual memory usage in a cross
+    platform fashion.
   - **percent**: the percentage usage calculated as
     ``(total - available) / total * 100``.
   - **used**: memory used, calculated differently depending on the platform and
@@ -221,6 +221,9 @@ Memory
 
   .. versionchanged:: 4.2.0 added *shared* metrics on Linux.
 
+  .. versionchanged:: 4.4.0 on Linux, *free*, *available* and *used* fields
+    are more precise and match "free" cmdline utility.
+
 .. function:: swap_memory()
 
   Return system swap memory statistics as a namedtuple including the following
diff --git a/psutil/__init__.py b/psutil/__init__.py
index 0a6f3ec6..020a0bdd 100644
--- a/psutil/__init__.py
+++ b/psutil/__init__.py
@@ -187,7 +187,7 @@ __all__ = [
 ]
 __all__.extend(_psplatform.__extra__all__)
 __author__ = "Giampaolo Rodola'"
-__version__ = "4.3.2"
+__version__ = "4.4.0"
 version_info = tuple([int(num) for num in __version__.split('.')])
 AF_LINK = _psplatform.AF_LINK
 _TOTAL_PHYMEM = None
diff --git a/psutil/_pslinux.py b/psutil/_pslinux.py
index 7f6e0405..1a0b57bc 100644
--- a/psutil/_pslinux.py
+++ b/psutil/_pslinux.py
@@ -289,61 +289,109 @@ except Exception:
 
 
 def virtual_memory():
-    total, free, buffers, shared, _, _, unit_multiplier = cext.linux_sysinfo()
-    total *= unit_multiplier
-    free *= unit_multiplier
-    buffers *= unit_multiplier
-    # Note: this (on my Ubuntu 14.04, kernel 3.13 at least) may be 0.
-    # If so, it will be determined from /proc/meminfo.
-    shared *= unit_multiplier or None
-    if shared == 0:
-        shared = None
-
-    cached = active = inactive = None
+    """Report memory stats trying to match "free" and "vmstat -s" cmdline
+    utility values as much as possible.
+
+    This implementation uses procps-ng-3.3.12 as a reference (2016-09-18):
+    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.
+    """
+    missing_fields = []
+    mems = {}
     with open_binary('%s/meminfo' % get_procfs_path()) as f:
         for line in f:
-            if cached is None and line.startswith(b"Cached:"):
-                cached = int(line.split()[1]) * 1024
-            elif active is None and line.startswith(b"Active:"):
-                active = int(line.split()[1]) * 1024
-            elif inactive is None and line.startswith(b"Inactive:"):
-                inactive = int(line.split()[1]) * 1024
-            # From "man free":
-            # The shared memory column represents either the MemShared
-            # value (2.4 kernels) or the Shmem value (2.6+ kernels) taken
-            # from the /proc/meminfo file. The value is zero if none of
-            # the entries is exported by the kernel.
-            elif shared is None and \
-                    line.startswith(b"MemShared:") or \
-                    line.startswith(b"Shmem:"):
-                shared = int(line.split()[1]) * 1024
-
-    missing = []
-    if cached is None:
-        missing.append('cached')
-        cached = 0
-    if active is None:
-        missing.append('active')
+            fields = line.split()
+            mems[fields[0]] = int(fields[1]) * 1024
+
+    # Note: these info are available also as cext.linux_sysinfo().
+    total = mems[b'MemTotal:']
+    free = mems[b'MemFree:']
+    buffers = mems[b'Buffers:']
+    cached = mems[b"Cached:"]
+    # "free" cmdline utility sums cached + reclamaible (available
+    # since kernel 2.6.19):
+    # https://gitlab.com/procps-ng/procps/
+    #     blob/195565746136d09333ded280cf3ba93853e855b8/proc/sysinfo.c#L761
+    # Older versions of procps added slab memory instead.
+    # This got changed in:
+    # https://gitlab.com/procps-ng/procps/commit/
+    #     05d751c4f076a2f0118b914c5e51cfbb4762ad8e
+    cached += mems.get(b"SReclaimable:", 0)
+
+    try:
+        shared = mems[b'Shmem:']  # since kernel 2.6.32
+    except KeyError:
+        try:
+            shared = mems[b'MemShared:']  # kernels 2.4
+        except KeyError:
+            shared = 0
+            missing_fields.append('shared')
+
+    try:
+        active = mems[b"Active:"]
+    except KeyError:
         active = 0
-    if inactive is None:
-        missing.append('inactive')
-        inactive = 0
-    if shared is None:
-        missing.append('shared')
-        shared = 0
-    if missing:
+        missing_fields.append('active')
+
+    try:
+        inactive = mems[b"Inactive:"]
+    except KeyError:
+        # https://gitlab.com/procps-ng/procps/blob/
+        #   195565746136d09333ded280cf3ba93853e855b8/proc/sysinfo.c#L758
+        try:
+            inactive = \
+                mems[b"Inact_dirty:"] + \
+                mems[b"Inact_clean:"] + \
+                mems[b"Inact_laundry:"]
+        except KeyError:
+            inactive = 0
+            missing_fields.append('inactive')
+
+    # https://gitlab.com/procps-ng/procps/blob/
+    #     24fd2605c51fccc375ab0287cec33aa767f06718/proc/sysinfo.c#L769
+    used = total - free - cached - buffers
+    if used < 0:
+        # May be symptomatic of running within a LCX container where such
+        # values will be dramatically distorted over those of the host.
+        used = total - free
+
+    # Note: starting from 4.4.0 we match "free" "available" column.
+    # Before 4.4.0 we calculated it as:
+    # >>> avail = free + buffers + cached
+    # ...which matched htop.
+    # free and htop available memory differs as per:
+    # http://askubuntu.com/a/369589
+    # http://unix.stackexchange.com/a/65852/168884
+    try:
+        avail = mems[b'MemAvailable:']
+    except KeyError:
+        # Column is not there; it's likely this is an older kernel.
+        # In this case "free" won't show an "available" column.
+        # Also, procps does some hacky things:
+        # https://gitlab.com/procps-ng/procps/blob/
+        #     /24fd2605c51fccc375ab0287cec33aa767f06718/proc/sysinfo.c#L774
+        # We won't. Like this we'll match "htop".
+        avail = free + buffers + cached
+    # 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:
+        avail = free
+
+    percent = usage_percent((total - avail), total, _round=1)
+
+    # Warn about missing metrics which are set to 0.
+    if missing_fields:
         msg = "%s memory stats couldn't be determined and %s set to 0" % (
-            ", ".join(missing),
-            "was" if len(missing) == 1 else "were")
+            ", ".join(missing_fields),
+            "was" if len(missing_fields) == 1 else "were")
         warnings.warn(msg, RuntimeWarning)
 
-    # Note: this value matches "htop" perfectly.
-    avail = free + buffers + cached
-    # Note: this value matches "free", but not all the time, see:
-    # https://github.com/giampaolo/psutil/issues/685#issuecomment-202914057
-    used = total - free
-    # Note: this value matches "htop" perfectly.
-    percent = usage_percent((total - avail), total, _round=1)
     return svmem(total, avail, percent, used, free,
                  active, inactive, buffers, cached, shared)
 
diff --git a/psutil/tests/test_linux.py b/psutil/tests/test_linux.py
index 9f7c25fb..a6b5d3f3 100644
--- a/psutil/tests/test_linux.py
+++ b/psutil/tests/test_linux.py
@@ -117,8 +117,9 @@ def free_physmem():
         if line.startswith('Mem'):
             total, used, free, shared = \
                 [int(x) for x in line.split()[1:5]]
-            nt = collections.namedtuple('free', 'total used free shared')
-            return nt(total, used, free, shared)
+            nt = collections.namedtuple(
+                'free', 'total used free shared output')
+            return nt(total, used, free, shared, out)
     raise ValueError(
         "can't find 'Mem' in 'free' output:\n%s" % '\n'.join(lines))
 
@@ -132,6 +133,11 @@ def vmstat(stat):
     raise ValueError("can't find %r in 'vmstat' output" % stat)
 
 
+def get_free_version_info():
+    out = sh("free -V").strip()
+    return tuple(map(int, out.split()[-1].split('.')))
+
+
 # =====================================================================
 # system virtual memory
 # =====================================================================
@@ -148,12 +154,20 @@ class TestSystemVirtualMemory(unittest.TestCase):
         psutil_value = psutil.virtual_memory().total
         self.assertAlmostEqual(vmstat_value, psutil_value)
 
+    # Older versions of procps used slab memory to calculate used memory.
+    # This got changed in:
+    # https://gitlab.com/procps-ng/procps/commit/
+    #     05d751c4f076a2f0118b914c5e51cfbb4762ad8e
+    @unittest.skipUnless(
+        LINUX and get_free_version_info() >= (3, 3, 12), "old free version")
     @retry_before_failing()
     def test_used(self):
-        free_value = free_physmem().used
+        free = free_physmem()
+        free_value = free.used
         psutil_value = psutil.virtual_memory().used
         self.assertAlmostEqual(
-            free_value, psutil_value, delta=MEMORY_TOLERANCE)
+            free_value, psutil_value, delta=MEMORY_TOLERANCE,
+            msg='%s %s \n%s' % (free_value, psutil_value, free.output))
 
     @retry_before_failing()
     def test_free(self):
@@ -188,31 +202,30 @@ class TestSystemVirtualMemory(unittest.TestCase):
             vmstat_value, psutil_value, delta=MEMORY_TOLERANCE)
 
     @retry_before_failing()
-    @unittest.skipIf(TRAVIS, "fails on travis")
     def test_shared(self):
-        free_value = free_physmem().shared
+        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=MEMORY_TOLERANCE)
-
-    # --- mocked tests
+            free_value, psutil_value, delta=MEMORY_TOLERANCE,
+            msg='%s %s \n%s' % (free_value, psutil_value, free.output))
 
-    def test_warnings_mocked(self):
-        with mock.patch('psutil._pslinux.open', create=True) as m:
-            with warnings.catch_warnings(record=True) as ws:
-                warnings.simplefilter("always")
-                ret = psutil._pslinux.virtual_memory()
-                assert m.called
-                self.assertEqual(len(ws), 1)
-                w = ws[0]
-                self.assertTrue(w.filename.endswith('psutil/_pslinux.py'))
-                self.assertIn(
-                    "memory stats couldn't be determined", str(w.message))
-                self.assertEqual(ret.cached, 0)
-                self.assertEqual(ret.active, 0)
-                self.assertEqual(ret.inactive, 0)
+    @retry_before_failing()
+    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=MEMORY_TOLERANCE,
+                msg='%s %s \n%s' % (free_value, psutil_value, out))
 
 
 # =====================================================================
@@ -370,6 +383,7 @@ class TestSystemCPU(unittest.TestCase):
 @unittest.skipUnless(LINUX, "not a Linux system")
 class TestSystemCPUStats(unittest.TestCase):
 
+    @unittest.skipIf(TRAVIS, "fails on Travis")
     def test_ctx_switches(self):
         vmstat_value = vmstat("context switches")
         psutil_value = psutil.cpu_stats().ctx_switches