Re: [PATCH 1/3] tools/rtla: Code Cleanup and Readability Improvements for timerlat

[Tomas Glozar <tglozar@redhat.com>]

ne 20. 10. 2024 v 22:01 odesílatel furkanonder
<furkanonder@protonmail.com> napsal:
>
> The enhancements made to timerlat_load.py are intended to improve the
> script's robustness and readability.
>
> Summary of the changes:
> - Unnecessary semicolons at the end of lines have been removed.
> - Parentheses surrounding the if statement checking args.prio have been
> eliminated.
> - String concatenation for constructing timerlat_path has been replaced
> with an f-string.
> - Spacing in a multiplication expression has been adjusted for improved
> clarity.
>
> Signed-off-by: Furkan Onder <furkanonder@protonmail.com>
> ---
>  tools/tracing/rtla/sample/timerlat_load.py | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/tools/tracing/rtla/sample/timerlat_load.py b/tools/tracing/rtla/sample/timerlat_load.py
> index 8cc5eb2d2e69..785e9a83539a 100644
> --- a/tools/tracing/rtla/sample/timerlat_load.py
> +++ b/tools/tracing/rtla/sample/timerlat_load.py
> @@ -37,12 +37,12 @@ except:
>      exit(1)
>
>  try:
> -    os.sched_setaffinity(0, affinity_mask);
> +    os.sched_setaffinity(0, affinity_mask)
>  except:
>      print("Error setting affinity")
>      exit(1)
>
> -if (args.prio):
> +if args.prio:
>      try:
>          param = os.sched_param(int(args.prio))
>          os.sched_setscheduler(0, os.SCHED_FIFO, param)
> @@ -51,21 +51,21 @@ if (args.prio):
>          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:
>      print("Error opening timerlat fd, did you run timerlat -U?")
>      exit(1)
>
>  try:
> -    data_fd = open("/dev/full", 'r');
> +    data_fd = open("/dev/full", 'r')
>  except:
>      print("Error opening data fd")
>
>  while True:
>      try:
>          timerlat_fd.read(1)
> -        data_fd.read(20*1024*1024)
> +        data_fd.read(20 * 1024 * 1024)
>      except:
>          print("Leaving")
>          break
> --
> 2.46.2
>

The code part looks good now, thank you! Just one comment: the commit
titles should include more clearly which part of timerlat it is
changing: timerlat_load.py instead of just timerlat - the latter
sounds like you are changing the code of rtla-timerlat itself (in C)
rather than the example Python script. It is also recommended [1] to
use imperative mood in the commit messages, like this: "tools/rtla:
Improve code readability in timerlat_load.py" (and similarly for the
other patches).

[1] See the for submitting patches at
https://www.kernel.org/doc/html/latest/process/submitting-patches.html,
section Describe your changes.


Tomas