Re: [PATCH] delayacct: add timestamp of delay max

[fan.yu9]

> bash-4.4# ./getdelays -d -t 200
> print delayacct stats ON
> TGID    200
> 
> CPU         count     real total  virtual total    delay total  delay average      delay max      delay min       max timestamp
>                45      176000000      181535534        1429077          0.032ms     0.418387ms     0.124835ms 2026-01-13 12:38:39
The output shows "max timestamp" which could be confusing. Consider 
renaming to "delay max ts" or "max delay timestamp" for clarity.


> @@ -87,6 +88,7 @@ struct task_delay_info;
>  struct task_group;
>  struct task_struct;
>  struct user_event_mm;
> +struct timespec64;
In include/linux/sched.h, you added `struct timespec64;` declaration
which breaks the alphabetical ordering rule mentioned in the comment.
/* task_struct member predeclarations (sorted alphabetically): */


> @@ -435,6 +437,9 @@ struct sched_info {
>       /* When were we last queued to run? */
>       unsigned long long              last_queued;
> 
> +     /* Timestamp of max time spent waiting on a runqueue: */
> +     struct timespec64               max_run_delay_ts;
> +
>  #endif /* CONFIG_SCHED_INFO */
>  };
Adding struct timespec64 fields to sched_info changes its size and 
alignment, which affects task_struct layout. While this is acceptable for
internal structures, please ensure there are no padding issues.


>  void __delayacct_wpcopy_start(void)
> @@ -286,7 +296,8 @@ void __delayacct_wpcopy_end(void)
>                     &current->delays->wpcopy_delay,
>                     &current->delays->wpcopy_count,
>                     &current->delays->wpcopy_delay_max,
> -                   &current->delays->wpcopy_delay_min);
> +                   &current->delays->wpcopy_delay_min,
> +                       &current->delays->wpcopy_delay_max_ts);
>  }
> 
>  void __delayacct_irq(struct task_struct *task, u32 delta)
You added timestamp fields for IRQ delays but doesn't update 
`irq_delay_max_ts` in `__delayacct_irq()` function.


> +static const char *format_timespec64(struct timespec64 *ts)
> +{
> +     static char buffer[32];
> +     struct tm *tm_info;
> +     time_t time_sec;
> +
> +     /* Check if timestamp is zero (not set) */
> +     if (ts->tv_sec == 0 && ts->tv_nsec == 0)
> +             return "N/A";
> +
> +     time_sec = (time_t)ts->tv_sec;
> +     tm_info = localtime(&time_sec);
The `format_timespec64()` function use the non-thread-safe `localtime()`
and a static buffer. Please use `localtime_r()` instead and consider
passing a buffer as parameter.


Best regards,
Fan Yu