Thread (4 messages) 4 messages, 2 authors, 2024-09-30

Re: [PATCH] tools/timerlat: Enhance Error Handling and Readability for timerlat_load.py

From: furkanonder <hidden>
Date: 2024-09-29 23:21:18

I’m sorry; I included the patch as an attachment in the email without noticing.
--- tools/tracing/rtla/sample/timerlat_load.py.orig	2024-09-22 02:37:14.767832391 +0300
+++ tools/tracing/rtla/sample/timerlat_load.py	2024-09-22 02:37:05.721215367 +0300
@@ -25,50 +25,54 @@ import sys
 import os
 
 parser = argparse.ArgumentParser(description='user-space timerlat thread in Python')
-parser.add_argument("cpu", help='CPU to run timerlat thread')
-parser.add_argument("-p", "--prio", help='FIFO priority')
-
+parser.add_argument("cpu", type=int, help='CPU to run timerlat thread')
+parser.add_argument("-p", "--prio", type=int, help='FIFO priority')
 args = parser.parse_args()
 
 try:
-    affinity_mask = { int(args.cpu) }
-except:
-    print("Invalid cpu: " + args.cpu)
-    exit(1)
-
-try:
-    os.sched_setaffinity(0, affinity_mask);
-except:
-    print("Error setting affinity")
-    exit(1)
+    affinity_mask = {args.cpu}
+    os.sched_setaffinity(0, affinity_mask)
+except Exception as e:
+    print(f"Error setting affinity: {e}")
+    sys.exit(1)
 
-if (args.prio):
+if args.prio:
     try:
-        param = os.sched_param(int(args.prio))
+        param = os.sched_param(args.prio)
         os.sched_setscheduler(0, os.SCHED_FIFO, param)
-    except:
-        print("Error setting priority")
-        exit(1)
+    except Exception as e:
+        print(f"Error setting priority: {e}")
+        sys.exit(1)
 
 try:
-    timerlat_path = "/sys/kernel/tracing/osnoise/per_cpu/cpu" + args.cpu + "/timerlat_fd"
+    timerlat_path = f"/sys/kernel/tracing/osnoise/per_cpu/cpu{args.cpu}/timerlat_fd"
     timerlat_fd = open(timerlat_path, 'r')
-except:
+except PermissionError:
+    print("Permission denied. Please check your access rights.")
+    sys.exit(1)
+except OSError:
     print("Error opening timerlat fd, did you run timerlat -U?")
-    exit(1)
+    sys.exit(1)
 
 try:
-    data_fd = open("/dev/full", 'r');
-except:
-    print("Error opening data fd")
+    data_fd = open("/dev/full", 'r')
+except Exception as e:
+    print(f"Error opening data fd: {e}")
+    sys.exit(1)
 
 while True:
     try:
         timerlat_fd.read(1)
-        data_fd.read(20*1024*1024)
-    except:
+        data_fd.read(20 * 1024 * 1024)
+    except KeyboardInterrupt:
         print("Leaving")
         break
+    except IOError as e:
+        print(f"I/O error occurred: {e}")
+        break
+    except Exception as e:
+        print(f"Unexpected error: {e}")
+        break
 
 timerlat_fd.close()
 data_fd.close()

Sent with Proton Mail secure email.

On Thursday, September 26th, 2024 at 7:38 AM, Steven Rostedt [off-list ref] wrote:
Hi,

Thank you for your patch. First, Daniel is unfortunately no longer the
maintainer of this code:

https://lwn.net/Articles/979912/

This patch is not formatted correctly, so it can not even be reviewed.
Please take a look at Submitting Patches:

https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html

-- Steve


On Sat, 21 Sep 2024 23:43:02 +0000
furkanonder furkanonder@protonmail.com wrote:
quoted
The enhancements made to timerlat_load.py focus on improving error handling, readability, and overall user experience. These changes aim to make the script more robust and easier to maintain while providing clearer feedback to users.
Key modifications include:

Type Declaration in Argument Parsing:
Added type declarations for command-line arguments in the argument parser. This removes the need for manual type checks later in the code, improving clarity and safety.
Before:
parser.add_argument("cpu", help='CPU to run timerlat thread')
parser.add_argument("-p", "--prio", help='FIFO priority')
After:
parser.add_argument("cpu", type=int, help='CPU to run timerlat thread')
parser.add_argument("-p", "--prio", type=int, help='FIFO priority')

String Formatting:
Replaced string concatenation with an f-string to enhancing readability and conciseness.
Before:
timerlat_path = "/sys/kernel/tracing/osnoise/per_cpu/cpu" + args.cpu + "/timerlat_fd"
After:
timerlat_path = f"/sys/kernel/tracing/osnoise/per_cpu/cpu{args.cpu}/timerlat_fd"

Enhanced Exception Handling and Consistent Error Reporting:
Specific exceptions are now caught and printed with clearer messages, providing context for errors when opening file descriptors. Added exception
handling for the data file descriptor opening to ensure uniformity across the script.

Before:
$ sudo python timerlat_load.py 122
Error setting affinity
After:
$ sudo python timerlat_load.py 122
Error setting affinity: [Errno 22] Invalid argument

Before:
$ sudo python timerlat_load.py 1 -p 950
Error setting priority
After:
$ sudo python timerlat_load.py 1 -p 950
Error setting priority: [Errno 22] Invalid argument

Before:
$ python timerlat_load.py 1
Error opening timerlat fd, did you run timerlat -U?
After:
$ python timerlat_load.py 1
Permission denied. Please check your access rights.

Changes for the read Infinite Loop:
The original generic exception clause has been replaced with more specific exception types to provide clearer feedback on errors.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help