Thank you for the patch, Austin.

On Fri, Feb 25, 2022 at 4:52 PM <[email protected]> wrote:

> Date: Fri, 25 Feb 2022 07:19:32 +0000
> From: Austin Kim <[email protected]>
> To: [email protected], [email protected]
> Cc: [email protected], [email protected]
> Subject: [Crash-utility] [PATCH 1/2] ps: Add support to "ps -l" to
>         properly display process list
> Message-ID: <20220225071932.GA1097@raspberrypi>
> Content-Type: text/plain; charset=us-ascii
>
> Sometimes kernel image is generated without CONFIG_SCHED_STAT or
> CONFIG_SCHED_INFO.
>
> Running crash-utility with above kernel image,
> "ps -l" options cannot display all processes sorted with most recently-run
> process
>
> crash> ps -l
> ps: last-run timestamps do not exist in this kernel
> Usage:
>   ps [-k|-u|-G] [-s] [-p|-c|-t|-[l|m][-C cpu]|-a|-g|-r|-S]
>      [pid | task | command] ...
> Enter "help ps" for details.
>
> This is because output of 'ps -l' depends on
> task_struct.sched_info.last_arrival.
> Without CONFIG_SCHED_STAT or CONFIG_SCHED_INFO, 'sched_info' field is not
> included
> in task_struct.
>
> So we make 'ps -e' option to access 'exec_start' field of sched_entity.
> where 'exec_start' is task_struct.se.exec_start.
>
> With this patch, "ps -l" option works well without CONFIG_SCHED_STAT or
> CONFIG_SCHED_INFO.
>
> The history of CONFIG_SCHED_INFO and CONFIG_SCHED_STAT is as below;
>
>   - CONFIG_SCHED_INFO: KERNEL_VERSION >= LINUX(4,2,0)
>   - CONFIG_SCHED_STAT: KERNEL_VERSION < LINUX(4,2,0)
>
>
Could you please add the kernel commit ID for the relevant changes?

In addition, I would suggest to fold these two patches as one patch and
change the subject as:
"ps: Add support to "ps -l|-m" to properly display process list", what do
you think?

Signed-off-by: Austin Kim <[email protected]>
> ---
>  defs.h    |  2 ++
>  symbols.c |  2 ++
>  task.c    | 17 ++++++++++++++---
>  3 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/defs.h b/defs.h
> index 7d386d2..ed2f5ca 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -1768,6 +1768,8 @@ struct offset_table {                    /* stash of
> commonly-used offsets */
>         long vcpu_struct_rq;
>         long task_struct_sched_info;
>         long sched_info_last_arrival;
> +       long task_struct_sched_entity;
> +       long se_exec_start;
>

This can be only appended to the end of the offset_table.
For more details, refer to the section "writing patches" in wiki:
https://github.com/crash-utility/crash/wiki

Thanks.
Lianbo

        long page_objects;
>         long kmem_cache_oo;
>         long char_device_struct_cdev;
> diff --git a/symbols.c b/symbols.c
> index 97fb778..5e2032a 100644
> --- a/symbols.c
> +++ b/symbols.c
> @@ -8892,6 +8892,8 @@ dump_offset_table(char *spec, ulong makestruct)
>                  OFFSET(sched_rt_entity_run_list));
>         fprintf(fp, "       sched_info_last_arrival: %ld\n",
>                  OFFSET(sched_info_last_arrival));
> +               fprintf(fp, "       se_exec_start: %ld\n",
> +                               OFFSET(se_exec_start));
>          fprintf(fp, "       task_struct_thread_info: %ld\n",
>                  OFFSET(task_struct_thread_info));
>          fprintf(fp, "             task_struct_stack: %ld\n",
> diff --git a/task.c b/task.c
> index 864c838..e6fde74 100644
> --- a/task.c
> +++ b/task.c
> @@ -334,9 +334,15 @@ task_init(void)
>         if (VALID_MEMBER(task_struct_sched_info))
>                 MEMBER_OFFSET_INIT(sched_info_last_arrival,
>                         "sched_info", "last_arrival");
> +       MEMBER_OFFSET_INIT(task_struct_sched_entity, "task_struct", "se");
> +       if (VALID_MEMBER(task_struct_sched_entity)) {
> +               STRUCT_SIZE_INIT(sched_entity, "sched_entity");
> +               MEMBER_OFFSET_INIT(se_exec_start, "sched_entity",
> "exec_start");
> +       }
>         if (VALID_MEMBER(task_struct_last_run) ||
>             VALID_MEMBER(task_struct_timestamp) ||
> -           VALID_MEMBER(sched_info_last_arrival)) {
> +           VALID_MEMBER(sched_info_last_arrival) ||
> +           VALID_MEMBER(se_exec_start)) {
>                 char buf[BUFSIZE];
>                 strcpy(buf, "alias last ps -l");
>                 alias_init(buf);
> @@ -3574,7 +3580,8 @@ cmd_ps(void)
>                 case 'l':
>                         if (INVALID_MEMBER(task_struct_last_run) &&
>                             INVALID_MEMBER(task_struct_timestamp) &&
> -                           INVALID_MEMBER(sched_info_last_arrival)) {
> +                           INVALID_MEMBER(sched_info_last_arrival) &&
> +                           INVALID_MEMBER(se_exec_start)) {
>                                 error(INFO,
>                              "last-run timestamps do not exist in this
> kernel\n");
>                                 argerrs++;
> @@ -6020,7 +6027,11 @@ task_last_run(ulong task)
>                 timestamp = tt->last_task_read ?
> ULONGLONG(tt->task_struct +
>                         OFFSET(task_struct_sched_info) +
>                         OFFSET(sched_info_last_arrival)) : 0;
> -
> +       else if (VALID_MEMBER(se_exec_start))
> +                       timestamp = tt->last_task_read ?
> ULONGLONG(tt->task_struct +
> +                       OFFSET(task_struct_sched_entity) +
> +                       OFFSET(se_exec_start)) : 0;
> +
>          return timestamp;
>  }
>
> --
> 2.20.1
>
--
Crash-utility mailing list
[email protected]
https://listman.redhat.com/mailman/listinfo/crash-utility

Reply via email to