Rework handling of GFP flags

OOMAnalyser.html

@@ -427,8 +427,11 @@ window.onerror = function (msg, url, lineNo, columnNo, errorObj) {
         <tr>
             <td>Memory allocation flags<br>(gfp_mask)</td>
             <td class="trigger_proc_gfp_mask text--align-right"></td>
-            <td>These flags are used to control the kernel internal memory allocation<br>
+            <td>These flags are also called GFP (get free pages) flags. They control the kernel-internal memory
-                GFP stands for <code>__get_free_pages()</code>.</td>
+                allocation.<br>
+                Some OOM variants include the individual flags in addition to the hexadecimal representation. The flags
+                are calculated if they are not specified.
+            </td>
         </tr>
         <tr>
             <td>Node mask to show on which CPU Cores this process can run<br>(nodemask)</td>
@@ -936,6 +939,7 @@ window.onerror = function (msg, url, lineNo, columnNo, errorObj) {
     <ol>
         <li>Fix memory calculation in summary section</li>
         <li>Fix calculation of GFP flags</li>
+        <li>Rework handling of GFP flags</li>
         <li>...</li>
     </ol>
 

OOMAnalyser.py

@@ -377,6 +377,18 @@ class BaseKernelConfig:
           (see https://github.com/torvalds/linux/commit/e67d4ca79aaf9d13a00d229b1b1c96b86828e8ba#diff-020720d0699e3ae1afb6fcd815ca8500)
     """
 
+    gfp_reverse_lookup = []
+    """
+    Sorted list of flags used to do a reverse lookup.
+    
+    This list doesn't contain all flags. It contains the "useful flags" (GFP_*) as
+    well as "modifier flags" (__GFP_*). "Plain flags" (___GFP_*) are not part of
+    this list.
+    
+    @type: List(str)
+    @see: _gfp_create_reverse_lookup() 
+    """
+
     pstable_items = [
         "pid",
         "uid",
@@ -445,6 +457,103 @@ class BaseKernelConfig:
         if self.EXTRACT_PATTERN_OVERLAY:
             self.EXTRACT_PATTERN.update(self.EXTRACT_PATTERN_OVERLAY)
 
+        self._gfp_calc_all_values()
+        self.gfp_reverse_lookup = self._gfp_create_reverse_lookup()
+
+    def _gfp_calc_all_values(self):
+        """
+        Calculate decimal values for all GFP flags and store in in GFP_FLAGS[<flag>]["_value"]
+        """
+        # __pragma__ ('jsiter')
+        for flag in self.GFP_FLAGS:
+            value = self._gfp_flag2decimal(flag)
+            self.GFP_FLAGS[flag]["_value"] = value
+        # __pragma__ ('nojsiter')
+
+    def _gfp_flag2decimal(self, flag):
+        """\
+        Convert a single flag into a decimal value.
+
+        The flags can be concatenated with "|" or "~" and negated with "~". The
+        flags will be processed from left to right. Parentheses are not supported.
+        """
+        if flag not in self.GFP_FLAGS:
+            error("No definition for flag {} found".format(flag))
+            return 0
+
+        value = self.GFP_FLAGS[flag]["value"]
+        if isinstance(value, int):
+            return value
+
+        tokenlist = iter(re.split("([|&])", value))
+        operator = "|"  # set to process first flag
+        negate_rvalue = False
+        lvalue = 0
+        while True:
+            try:
+                token = next(tokenlist)
+            except StopIteration:
+                break
+            token = token.strip()
+            if token in ["|", "&"]:
+                operator = token
+                continue
+
+            if token.startswith("~"):
+                token = token[1:]
+                negate_rvalue = True
+
+            if token.isdigit():
+                rvalue = int(token)
+            elif token.startswith("0x") and token[2:].isdigit():
+                rvalue = int(token, 16)
+            else:
+                # it's not a decimal nor a hexadecimal value - reiterate assuming it's a flag string
+                rvalue = self._gfp_flag2decimal(token)
+
+            if negate_rvalue:
+                rvalue = ~rvalue
+
+            if operator == "|":
+                lvalue |= rvalue
+            elif operator == "&":
+                lvalue &= rvalue
+
+            operator = None
+            negate_rvalue = False
+
+        return lvalue
+
+    def _gfp_create_reverse_lookup(self):
+        """
+        Create a sorted list of flags used to do a reverse lookup from value to the flag.
+
+        @rtype: List(str)
+        """
+        # __pragma__ ('jsiter')
+        useful = [
+            key
+            for key in self.GFP_FLAGS
+            if key.startswith("GFP") and self.GFP_FLAGS[key]["_value"] != 0
+        ]
+        useful = sorted(
+            useful, key=lambda key: self.GFP_FLAGS[key]["_value"], reverse=True
+        )
+        modifier = [
+            key
+            for key in self.GFP_FLAGS
+            if key.startswith("__GFP") and self.GFP_FLAGS[key]["_value"] != 0
+        ]
+        modifier = sorted(
+            modifier, key=lambda key: self.GFP_FLAGS[key]["_value"], reverse=True
+        )
+        # __pragma__ ('nojsiter')
+
+        # useful + modifier produces a string with all values concatenated
+        res = useful
+        res.extend(modifier)
+        return res
+
 
 class KernelConfig_4_6(BaseKernelConfig):
     # Support changes:
@@ -1029,9 +1138,8 @@ class OOMAnalyser:
             flags = self.oom_result.details["trigger_proc_gfp_flags"]
             del self.oom_result.details["trigger_proc_gfp_flags"]
         else:
-            flags, unknown = self._hex2flags(
+            flags, unknown = self._gfp_hex2flags(
                 self.oom_result.details["trigger_proc_gfp_mask"],
-                self.oom_result.kconfig.GFP_FLAGS,
             )
             if unknown:
                 flags.append("0x{0:x}".format(unknown))
@@ -1101,77 +1209,26 @@ class OOMAnalyser:
                 self.oom_result.details["_pstable"][pid] = {}
                 self.oom_result.details["_pstable"][pid].update(details)
 
-    def _hex2flags(self, hexvalue, flag_definition):
+    def _gfp_hex2flags(self, hexvalue):
         """\
         Convert the hexadecimal value into flags specified by definition
 
-        @return: list of flags and the decimal sum of all unknown flags
+        @return: Unsorted list of flags and the sum of all unknown flags as integer
+        @rtype: List(str), int
         """
         remaining = int(hexvalue, 16)
         converted_flags = []
 
-        # __pragma__ ('jsiter')
+        for flag in self.oom_result.kconfig.gfp_reverse_lookup:
-        for flag in flag_definition:
+            value = self.oom_result.kconfig.GFP_FLAGS[flag]["_value"]
-            value = self._flag2decimal(flag, flag_definition)
             if (remaining & value) == value:
                 # delete flag by "and" with a reverted mask
                 remaining &= ~value
                 converted_flags.append(flag)
-        # __pragma__ ('nojsiter')
 
+        converted_flags.sort()
         return converted_flags, remaining
 
-    def _flag2decimal(self, flag, flag_definition):
-        """\
-        Convert a single flag into a decimal value
-        """
-        if flag not in flag_definition:
-            error("No definition for flag {} found".format(flag))
-            return 0
-
-        value = flag_definition[flag]["value"]
-        if isinstance(value, int):
-            return value
-
-        tokenlist = iter(re.split("([|&])", value))
-        operator = "|"  # set to process first flag
-        negate_rvalue = False
-        lvalue = 0
-        while True:
-            try:
-                token = next(tokenlist)
-            except StopIteration:
-                break
-            token = token.strip()
-            if token in ["|", "&"]:
-                operator = token
-                continue
-
-            if token.startswith("~"):
-                token = token[1:]
-                negate_rvalue = True
-
-            if token.isdigit():
-                rvalue = int(token)
-            elif token.startswith("0x") and token[2:].isdigit():
-                rvalue = int(token, 16)
-            else:
-                # it's not a decimal nor a hexadecimal value - reiterate assuming it's a flag string
-                rvalue = self._flag2decimal(token, flag_definition)
-
-            if negate_rvalue:
-                rvalue = ~rvalue
-
-            if operator == "|":
-                lvalue |= rvalue
-            elif operator == "&":
-                lvalue &= rvalue
-
-            operator = None
-            negate_rvalue = False
-
-        return lvalue
-
     def _convert_numeric_results_to_integer(self):
         """Convert all *_pages and *_kb to integer"""
         # __pragma__ ('jsiter')

test.py

@@ -189,18 +189,20 @@ class TestInBrowser(TestBase):
         trigger_proc_gfp_mask = self.driver.find_element(
             By.CLASS_NAME, "trigger_proc_gfp_mask"
         )
-        # 0x201da:
+        # 0x201da will split into
-        #  __GFP_HIGHMEM         2       0x02
+        #  GFP_HIGHUSER_MOVABLE   0x200da
-        #  __GFP_MOVABLE         8       0x08
+        #                         (__GFP_WAIT | __GFP_IO | __GFP_FS | __GFP_HARDWALL | __GFP_HIGHMEM | __GFP_MOVABLE)
-        #  __GFP_RECLAIMABLE    16       0x10
+        #    __GFP_WAIT              0x10
-        #  __GFP_IO             64       0x40
+        #    __GFP_IO                0x40
-        #  __GFP_FS            128       0x80
+        #    __GFP_FS                0x80
-        #  __GFP_COLD          256      0x100
+        #    __GFP_HARDWALL       0x20000
-        #  __GFP_HARDWALL   131072    0x20000
+        #    __GFP_HIGHMEM           0x02
-        #                             0x201da
+        #    __GFP_MOVABLE           0x08
+        #  __GFP_COLD               0x100
+        #                    sum: 0x201da
         self.assertEqual(
             trigger_proc_gfp_mask.text,
-            "0x201da (__GFP_HIGHMEM | __GFP_MOVABLE | __GFP_RECLAIMABLE | __GFP_IO | __GFP_FS | __GFP_COLD | __GFP_HARDWALL)",
+            "0x201da (GFP_HIGHUSER_MOVABLE | __GFP_COLD)",
             "Unexpected GFP Mask",
         )
 
@@ -257,6 +259,14 @@ class TestInBrowser(TestBase):
         trigger_proc_gfp_mask = self.driver.find_element(
             By.CLASS_NAME, "trigger_proc_gfp_mask"
         )
+        # 0xcc0 will split into
+        #  GFP_KERNEL             (__GFP_RECLAIM | __GFP_IO | __GFP_FS)
+        #    __GFP_RECLAIM        (___GFP_DIRECT_RECLAIM | ___GFP_KSWAPD_RECLAIM)
+        #        ___GFP_DIRECT_RECLAIM   0x400
+        #        ___GFP_KSWAPD_RECLAIM   0x800
+        #    __GFP_IO                    0x40
+        #    __GFP_FS                    0x80
+        #                  sum:          0xCC0
         self.assertEqual(
             trigger_proc_gfp_mask.text, "0xcc0 (GFP_KERNEL)", "Unexpected GFP Mask"
         )
@@ -653,19 +663,19 @@ Hardware name: HP ProLiant DL385 G7, BIOS A18 12/08/2012
                 "CPU: 4 PID: 29481 Comm: sed Not tainted 4.6.0-514 #1",
             ),
             (
-                OOMAnalyser.KernelConfigRhel7(),
+                OOMAnalyser.KernelConfig_3_10_EL7(),
                 "CPU: 4 PID: 29481 Comm: sed Not tainted 3.10.0-1062.9.1.el7.x86_64 #1",
             ),
             (
-                OOMAnalyser.KernelConfig_5_0(),
+                OOMAnalyser.KernelConfig_5_1(),
                 "CPU: 4 PID: 29481 Comm: sed Not tainted 5.5.1 #1",
             ),
             (
-                OOMAnalyser.KernelConfig_5_8(),
+                OOMAnalyser.KernelConfig_5_18(),
                 "CPU: 4 PID: 29481 Comm: sed Not tainted 5.23.0 #1",
             ),
             (
-                OOMAnalyser.KernelConfig_5_8(),
+                OOMAnalyser.KernelConfig_5_18(),
                 "CPU: 4 PID: 29481 Comm: sed Not tainted 6.12.0 #1",
             ),
             (
@@ -685,6 +695,142 @@ Hardware name: HP ProLiant DL385 G7, BIOS A18 12/08/2012
                 % (type(kcfg), type(result), kversion),
             )
 
+    def test_007_gfp_processing(self):
+        """Test processing GFP flags"""
+        oom = OOMAnalyser.OOMEntity(OOMAnalyser.OOMDisplay.example_rhel7)
+        analyser = OOMAnalyser.OOMAnalyser(oom)
+        success = analyser.analyse()
+        self.assertTrue(success, "OOM analysis failed")
+
+        self.assertEqual(
+            analyser.oom_result.kconfig.release,
+            (3, 10, ".el7."),
+            "Wrong KernelConfig release",
+        )
+
+        for flag, hex_value in [
+            ("__GFP_DMA", 0x01),
+            ("__GFP_WAIT", 0x10),
+            ("__GFP_IO", 0x40),
+            ("__GFP_FS", 0x80),
+            ("GFP_KERNEL", 0xD0),  # __GFP_WAIT | __GFP_IO | __GFP_FS
+            ("__GFP_UNKNOWN", 0x00),  # unknown GFP flag
+        ]:
+            self.assertEqual(
+                analyser.oom_result.kconfig._gfp_flag2decimal(flag),
+                hex_value,
+                "Invalid decimal value for %s" % flag,
+            )
+
+        for hex_value, flags_expected, unknown_expected in [
+            (
+                "0x01",
+                ["__GFP_DMA"],
+                0,
+            ),
+            ("0x05", ["__GFP_DMA", "__GFP_DMA32"], 0),
+            (
+                "0x5000000",  # 0x1000000 + 0x4000000
+                ["__GFP_WRITE"],
+                0x4000000,
+            ),
+            ("0x201da", ["GFP_HIGHUSER_MOVABLE", "__GFP_COLD"], 0),
+        ]:
+            flags_calculated, unknown_calculated = analyser._gfp_hex2flags(hex_value)
+            self.assertEqual(
+                flags_calculated,
+                flags_expected,
+                "Invalid flag(s) for hex value %s" % hex_value,
+            )
+            self.assertEqual(
+                unknown_calculated,
+                unknown_expected,
+                "Invalid remaining / not resolved decimal flag value",
+            )
+
+        oom = OOMAnalyser.OOMEntity(OOMAnalyser.OOMDisplay.example_ubuntu2110)
+        analyser = OOMAnalyser.OOMAnalyser(oom)
+        success = analyser.analyse()
+        self.assertTrue(success, "OOM analysis failed")
+
+        self.assertEqual(
+            analyser.oom_result.kconfig.release,
+            (5, 8, ""),
+            "Wrong KernelConfig release",
+        )
+
+        for flag, hex_value in [
+            ("__GFP_DMA", 0x01),
+            ("__GFP_IO", 0x40),
+            ("__GFP_FS", 0x80),
+            (
+                "GFP_KERNEL",
+                0xCC0,
+            ),  # (__GFP_RECLAIM (___GFP_DIRECT_RECLAIM | ___GFP_KSWAPD_RECLAIM) | __GFP_IO | __GFP_FS)
+            ("__GFP_UNKNOWN", 0x00),  # unknown GFP flag
+        ]:
+            self.assertEqual(
+                analyser.oom_result.kconfig._gfp_flag2decimal(flag),
+                hex_value,
+                "Invalid decimal value for %s" % flag,
+            )
+
+        for hex_value, flags_expected, unknown_expected in [
+            (
+                "0x01",
+                ["__GFP_DMA"],
+                0,
+            ),
+            ("0x05", ["__GFP_DMA", "__GFP_DMA32"], 0),
+            (
+                "0x4001000",  # 0x1000 + 0x4000000
+                ["__GFP_WRITE"],
+                0x4000000,
+            ),
+        ]:
+            flags_calculated, unknown_calculated = analyser._gfp_hex2flags(hex_value)
+            self.assertEqual(
+                flags_calculated,
+                flags_expected,
+                "Invalid flag(s) for hex value %s" % hex_value,
+            )
+            self.assertEqual(
+                unknown_calculated,
+                unknown_expected,
+                "Invalid remaining / not resolved decimal flag value",
+            )
+
+    def test_008_kversion_check(self):
+        """Test check for minimum kernel version"""
+        oom = OOMAnalyser.OOMEntity(OOMAnalyser.OOMDisplay.example_rhel7)
+        analyser = OOMAnalyser.OOMAnalyser(oom)
+
+        for kversion, min_version, expected_result in (
+            ("5.19-rc6", (5, 16, ""), True),
+            ("5.19-rc6", (5, 19, ""), True),
+            ("5.19-rc6", (5, 20, ""), False),
+            ("5.18.6-arch1-1", (5, 18, ""), True),
+            ("5.18.6-arch1-1", (5, 1, ""), True),
+            ("5.18.6-arch1-1", (5, 19, ""), False),
+            ("5.13.0-1028-aws #31~20.04.1-Ubuntu", (5, 14, ""), False),
+            ("5.13.0-1028-aws #31~20.04.1-Ubuntu", (5, 13, ""), True),
+            ("5.13.0-1028-aws #31~20.04.1-Ubuntu", (5, 13, "-aws"), True),
+            ("5.13.0-1028-aws #31~20.04.1-Ubuntu", (5, 13, "not_in_version"), False),
+            ("5.13.0-1028-aws #31~20.04.1-Ubuntu", (5, 12, ""), True),
+            ("4.14.288", (5, 0, ""), False),
+            ("4.14.288", (4, 14, ""), True),
+            ("3.10.0-514.6.1.el7.x86_64 #1", (3, 11, ""), False),
+            ("3.10.0-514.6.1.el7.x86_64 #1", (3, 10, ".el7."), True),
+            ("3.10.0-514.6.1.el7.x86_64 #1", (3, 10, ""), True),
+            ("3.10.0-514.6.1.el7.x86_64 #1", (3, 9, ""), True),
+        ):
+            self.assertEqual(
+                analyser._check_kversion_greater_equal(kversion, min_version),
+                expected_result,
+                'Failed to compare kernel version "%s" with minimum version "%s"'
+                % (kversion, min_version),
+            )
+
 
 if __name__ == "__main__":
     unittest.main(verbosity=2)