Incorporate review comments v1

doc/api.rst

@@ -119,10 +119,3 @@ drgn_tools.cmdline
 
 .. automodule:: drgn_tools.cmdline
    :members:
-
-
-drgn_tools.lock
------------------------
-
-.. automodule:: drgn_tools.lock
-   :members:

drgn_tools/lock.py

@@ -14,17 +14,17 @@
 int mutex_lock_interruptible(struct mutex *lock);
 int atomic_dec_and_mutex_lock(atomic_t *cnt, struct mutex *lock);
 
-TRY variats of mutex are ignored as they will not block.
+Try variants of mutex are ignored as they will not block.
 The common function used in all these api's is:
 __mutex_lock() and is sufficant to trap all block by mutexes.
 
 For semaphores,
-There is no woners, and waiters generally have the common functions
+There is no owners, and waiters generally have the common functions
 as : "__down_common" and "__down" depending upon releases. So trapping
-these two function is sufficent to check the semaphore waiters.
+these two function is sufficient to check the semaphore waiters.
 """
 import argparse
-from typing import List
+from typing import Set
 
 import drgn
 from drgn import Program
@@ -34,117 +34,112 @@
 from drgn_tools.corelens import CorelensModule
 from drgn_tools.locking import for_each_mutex_waiter
 from drgn_tools.locking import for_each_rwsem_waiter
-from drgn_tools.locking import mtx_owner
+from drgn_tools.locking import mutex_owner
 from drgn_tools.locking import show_lock_waiter
 
 
-def scan_mutex_lock(prog: Program, stk: bool) -> None:
+def scan_mutex_lock(prog: Program, stack: bool) -> None:
     """Scan for mutex and show deitals"""
 
     frame_list = bt_has(prog, "__mutex_lock")
-    lock_detected = bool(frame_list)
-
-    arr_lock: List = []
-
-    if not lock_detected:
+    if not frame_list:
         return
 
+    seen_mutexes: Set[int] = set()
+
+    warned_absent = False
     for task, frame in frame_list:
-        # Debug...
-        # pid = task.pid.value_()
-        # comm = task.comm.string_().decode("utf-8")
-        # print("%-15s %-15s" %(pid,comm))
-
-        mtx = frame["lock"]
-        struct_owner = mtx_owner(prog, mtx)
-
-        duplock = 1
-        if not arr_lock:
-            arr_lock.append(mtx)
-        else:
-            for locks in arr_lock:
-                if locks == mtx:
-                    duplock = 0
-
-        if duplock == 1:
-            arr_lock.append(mtx)
-            index = 1
-            print("Mutex:", hex(mtx.owner.counter.address_of_().value_()))
-            print("Mutex OWNER:", struct_owner.comm.string_().decode("utf-8"))
-            print("")
-            if stk:
-                bt(struct_owner.pid)
-                print("")
-            print(
-                "Mutex WAITERS (Index, cpu, comm, pid, state, wait time (d hr:min:sec:ms)):"
-            )
-            for waiter in for_each_mutex_waiter(prog, mtx):
-                show_lock_waiter(prog, waiter, index, stacktrace=stk)
-                index = index + 1
-            print("")
+        try:
+            mutex = frame["lock"]
+            mutex_addr = mutex.value_()
+        except drgn.ObjectAbsentError:
+            if not warned_absent:
+                print(
+                    "warning: failed to get mutex from stack frame"
+                    "- information is incomplete"
+                )
+                warned_absent = True
+            continue
 
+        struct_owner = mutex_owner(prog, mutex)
 
-sem_lock: List = []
+        if mutex_addr in seen_mutexes:
+            continue
+        seen_mutexes.add(mutex_addr)
+
+        index = 1
+        print(f"Mutex: 0x{mutex_addr:x}")
+        print("Mutex OWNER:", struct_owner.comm.string_().decode("utf-8"))
+        print("")
+        if stack:
+            bt(struct_owner.pid)
+            print("")
+        print(
+            "Mutex WAITERS (Index, cpu, comm, pid, state, wait time (d hr:min:sec:ms)):"
+        )
+        for waiter in for_each_mutex_waiter(prog, mutex):
+            show_lock_waiter(prog, waiter, index, stacktrace=stack)
+            index = index + 1
+        print("")
 
 
-def show_sem_lock(prog: Program, frame_list, stk: bool) -> None:
+def show_sem_lock(prog: Program, frame_list, seen_sems, stack: bool) -> None:
     """Show semaphore details"""
+    warned_absent = False
     for task, frame in frame_list:
         try:
             sem = frame["sem"]
             semaddr = sem.value_()
         except drgn.ObjectAbsentError:
+            if not warned_absent:
+                print(
+                    "warning: failed to get semaphore from stack frame"
+                    "- information is incomplete"
+                )
+                warned_absent = True
             continue
 
-        duplock = 1
-
-        if not sem_lock:
-            sem_lock.append(semaddr)
-        else:
-            for locks in sem_lock:
-                if locks == semaddr:
-                    duplock = 0
-
-        if duplock == 1:
-            sem_lock.append(semaddr)
-            index = 1
-            print("Semaphore:", hex(semaddr))
-            print(
-                "Semaphore WAITERS (Index, cpu, comm, pid, state, wait time (d hr:min:sec:ms)):"
-            )
-            for waiter in for_each_rwsem_waiter(prog, sem):
-                show_lock_waiter(prog, waiter, index, stacktrace=stk)
-                index = index + 1
+        if semaddr in seen_sems:
+            continue
+        seen_sems.add(semaddr)
 
-            print("")
+        index = 1
+        print(f"Semaphore: 0x{semaddr:x}")
+        print(
+            "Semaphore WAITERS (Index, cpu, comm, pid, state, wait time (d hr:min:sec:ms)):"
+        )
+        for waiter in for_each_rwsem_waiter(prog, sem):
+            show_lock_waiter(prog, waiter, index, stacktrace=stack)
+            index = index + 1
+
+        print("")
 
 
-def scan_sem_lock(prog: Program, stk: bool) -> None:
+def scan_sem_lock(prog: Program, stack: bool) -> None:
     """Scan for semphores"""
+    seen_sems: Set[int] = set()
     frame_list = bt_has(prog, "__down")
-    lock_detected = bool(frame_list)
-    if lock_detected:
-        show_sem_lock(prog, frame_list, stk)
+    if frame_list:
+        show_sem_lock(prog, frame_list, seen_sems, stack)
 
     frame_list = bt_has(prog, "__down_common")
-    lock_detected = bool(frame_list)
-    if lock_detected:
-        show_sem_lock(prog, frame_list, stk)
+    if frame_list:
+        show_sem_lock(prog, frame_list, seen_sems, stack)
 
 
-def scan_task(prog: Program, stk: bool) -> None:
+def scan_lock(prog: Program, stack: bool) -> None:
     """Scan tasks for Mutex and Semaphore"""
     print("Scanning Mutexes ...")
     print("")
-    scan_mutex_lock(prog, stk)
+    scan_mutex_lock(prog, stack)
 
     print("Scanning Semaphores...")
     print("")
-    scan_sem_lock(prog, stk)
+    scan_sem_lock(prog, stack)
 
 
 class Locking(CorelensModule):
-    """Display active mutex and semaphoes and their waiters"""
+    """Display active mutex and semaphores and their waiters"""
 
     name = "lock"
     need_dwarf = True
@@ -155,4 +150,4 @@ def add_args(self, parser: argparse.ArgumentParser) -> None:
         )
 
     def run(self, prog: Program, args: argparse.Namespace) -> None:
-        scan_task(prog, args.stack)
+        scan_lock(prog, args.stack)

drgn_tools/locking.py

@@ -17,8 +17,8 @@
 MUTEX_FLAGS = 0x7
 
 
-def mtx_owner(prog: Program, mtx: drgn.Object) -> drgn.Object:
-    owner = mtx.owner.counter.value_()
+def mutex_owner(prog: Program, mutex: drgn.Object) -> drgn.Object:
+    owner = mutex.owner.counter.value_()
     if owner < 0:
         owner = 2**64 + owner - 1
     Owner = owner & (~MUTEX_FLAGS)

tests/test_lock.py

@@ -4,4 +4,4 @@
 
 
 def test_locks(prog):
-    lock.scan_task(prog, 1)
+    lock.scan_task(prog, stack=True)