2022년 2월 25일 (금) 오후 6:47, lijiang <lijiang@xxxxxxxxxx>님이 작성: > > Thank you for the patch, Austin. > > On Fri, Feb 25, 2022 at 4:52 PM <crash-utility-request@xxxxxxxxxx> wrote: >> >> Date: Fri, 25 Feb 2022 07:19:32 +0000 >> From: Austin Kim <austindh.kim@xxxxxxxxx> >> To: k-hagio-ab@xxxxxxx, crash-utility@xxxxxxxxxx >> Cc: kernel-team@xxxxxxx, mikeseohyungjin@xxxxxxxxx >> Subject: [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? > Relevant commit I think of is below. f6db83479932 ("sched/stat: Simplify the sched_info accounting") Let me add above commit ID to the commit description with v2 patch. > 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? Thanks for good suggetion. I guess one patch is enough with "ps -l|-m" commit title. Let me send v2 patch which includes 1 commit. > >> Signed-off-by: Austin Kim <austindh.kim@xxxxxxxxx> >> --- >> 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 for the information. Let me make sure to append these to the end of the offset_table. I will resend patch v2 version soon. Thanks for detailed review and tips. BR, Austin Kim > 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 Crash-utility@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/crash-utility