Re: [PATCH v2] ps: Add support to "ps -l|-m" to properly display process list

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello, Kazu

2022년 3월 1일 (화) 오후 4:42, HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@xxxxxxx>님이 작성:
>
> Hi Austin,
>
> thank you for the patch.
>
> -----Original Message-----
> > Sometimes kernel image is generated without CONFIG_SCHEDSTATS or CONFIG_SCHED_INFO.
> > Where relevant commit id is f6db83479932 ("sched/stat: Simplify the sched_info accounting")
> >
> >   - CONFIG_SCHED_INFO: KERNEL_VERSION >= LINUX(4,2,0)
> >   - CONFIG_SCHEDSTATS: KERNEL_VERSION < LINUX(4,2,0)
> >
> > Running crash-utility with above kernel image,
> > "ps -l" option cannot display all processes sorted with most recently-run process.
> > Also "ps -m" option cannot display all processes with timestamp.
> >
> > crash> ps -l or crash> ps -m
> > 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|-m" depends on task_struct.sched_info.last_arrival.
> >
> > Without CONFIG_SCHEDSTATS or CONFIG_SCHED_INFO, 'sched_info' field is not included
> > in task_struct.
> >
> > So we make "ps -l|-m" option to access 'exec_start' field of sched_entity
> > where 'exec_start' is task_struct.se.exec_start.
>
> Could you describe what the exec_start means?  When is it updated?
>

The 'task_struct.se.exec_start' contains the most recently-executed
timestamp when
process is running in the below cases;

 - enqueued to runqueue
 - dequeued from ruqueue
 - scheduler tick is invoked
 - etc

So I guess 'task_struct.se.exec_start' could be one of statistics
which indicates
the most recently run timestamp of process activity.

>From CFS scheduler's point of view, 'task_struct.se.exec_start' is
updated within update_curr()
where its call path is various as below.

 - enqueue_task_fair, -dequeue_task_fair, task_tick_fair,
check_preempt_wakeup, ...

> Checking a vmcore on hand, those values are different each other.
>
> crash> task | grep -e exec_start -e last_arrival
>     exec_start = 737120183773,
>     last_arrival = 737112829272,
> crash> task 1 | grep -e exec_start -e last_arrival
>     exec_start = 736519632741,
>     last_arrival = 736515663193,
>
> The current last_arrival looks to be updated only by this path and
> I think it's the time when a task gets a CPU:
>   context_switch
>     prepare_task_switch
>       sched_info_switch
>         sched_info_arrive
>
> So if the meaning of the exec_start is different from last_arrival,
> it would be better to have a description about it in the help text.
>

With the next v3 patch, description about exec_start will be included
in help text and
commit message. Thanks for feedback.

> >
> > With this patch, "ps -l|-m" option works well without CONFIG_SCHEDSTATS or
> > CONFIG_SCHED_INFO.
> >
> > Signed-off-by: Austin Kim <austindh.kim@xxxxxxxxx>
> > ---
> >  defs.h    |  2 ++
> >  symbols.c |  2 ++
> >  task.c    | 20 ++++++++++++++++----
> >  3 files changed, 20 insertions(+), 4 deletions(-)
> >
> > diff --git a/defs.h b/defs.h
> > index bf2c59b..5dda176 100644
> > --- a/defs.h
> > +++ b/defs.h
> > @@ -2168,6 +2168,8 @@ struct offset_table {                    /* stash of commonly-used offsets */
> >       long sbitmap_queue_min_shallow_depth;
> >       long sbq_wait_state_wait_cnt;
> >       long sbq_wait_state_wait;
> > +     long task_struct_sched_entity;
> > +     long se_exec_start;
>
> Normally names in this table are "struct_name_member_name", so
> these should be "task_struct_se" and "sched_entity_exec_start"
> respectively.
>
> And "task_struct_se" already exists, so please fix those.
>

Sure, let me resend the v3 patch after renaming member.

>         long cfs_rq_tasks_timeline;
>         long task_struct_se;
>         long sched_entity_run_node;
>
>
> >  };
> >
> >  struct size_table {         /* stash of commonly-used sizes */
> > diff --git a/symbols.c b/symbols.c
> > index ba5e274..e5abe87 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));
>
> There are already some sched_entity members, please move this here.
>

Thanks for feedback.
it will be moved as you recommended in the next v3 patch.

>         fprintf(fp, "                task_struct_se: %ld\n",
>                 OFFSET(task_struct_se));
>         fprintf(fp, "         sched_entity_run_node: %ld\n",
>                 OFFSET(sched_entity_run_node));
>         fprintf(fp, "           sched_entity_cfs_rq: %ld\n",
>                 OFFSET(sched_entity_cfs_rq));
>         fprintf(fp, "             sched_entity_my_q: %ld\n",
>                 OFFSET(sched_entity_my_q));
>         fprintf(fp, "            sched_entity_on_rq: %ld\n",
>                 OFFSET(sched_entity_on_rq));
>
>
> Thanks,
> Kazu
>
> >          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..55e2312 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);
> > @@ -3559,7 +3565,8 @@ cmd_ps(void)
> >               case 'm':
> >                       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++;
> > @@ -3574,7 +3581,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 +6028,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
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki




[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux