Date: Fri, 21 Jan 2022 13:43:09 +0800 From: Tao Liu <ltao(a)redhat.com&gt; To: crash-utility(a)redhat.com Subject: [Crash-utility] [PATCH] Improve the ps performance for vmcores with large number of threads Message-ID: <20220121054309.9782-1-ltao(a)redhat.com&gt; Content-Type: text/plain; charset="US-ASCII" Previously, the ps command will iterate over all threads which have the same tgid, to accumulate their rss value, in order to get a thread/process's final rss value as part of the final output. For non-live systems, the rss accumulation values are identical for threads which have the same tgid, so there is no need to do the iteration and accumulation repeatly, thus a lot of readmem calls are skipped. Otherwise it will be the performance bottleneck if the vmcores have a large number of threads. In this patch, the rss accumulation value will be stored in a cache, next time a thread with the same tgid will take it directly without the iteration. For example, we can monitor the performance issue when a vmcore has ~65k processes, most of which are threads for several specific processes. Without the patch, it will take ~7h for ps command to finish. With the patch, ps command will finish in 1min. Signed-off-by: Tao Liu <ltao(a)redhat.com&gt; --- defs.h | 1 + memory.c | 67 ++++++++++++++++++++++++++++++-------------------------- task.c | 1 + 3 files changed, 38 insertions(+), 31 deletions(-) diff --git a/defs.h b/defs.h index b63741c..55600d5 100644 --- a/defs.h +++ b/defs.h @@ -829,6 +829,7 @@ struct task_context { /* context stored for each task */ struct tgid_context { /* tgid and task stored for each task */ ulong tgid; ulong task; + long rss_cache; }; struct task_table { /* kernel/local task table data */ diff --git a/memory.c b/memory.c index 5af45fd..b930933 100644 --- a/memory.c +++ b/memory.c @@ -4665,7 +4665,7 @@ void get_task_mem_usage(ulong task, struct task_mem_usage *tm) { struct task_context *tc; - long rss = 0; + long rss = 0, rss_cache = 0; BZERO(tm, sizeof(struct task_mem_usage)); @@ -4730,38 +4730,43 @@ get_task_mem_usage(ulong task, struct task_mem_usage *tm) (last->tgid == (last + 1)->tgid)) last++; - while (first <= last) - { - /* count 0 -> filepages */ - if (!readmem(first->task + - OFFSET(task_struct_rss_stat) + - OFFSET(task_rss_stat_count), KVADDR, - &sync_rss, - sizeof(int), - "task_struct rss_stat MM_FILEPAGES", - RETURN_ON_ERROR)) - continue; - - rss += sync_rss; - - /* count 1 -> anonpages */ - if (!readmem(first->task + - OFFSET(task_struct_rss_stat) + - OFFSET(task_rss_stat_count) + - sizeof(int), - KVADDR, &sync_rss, - sizeof(int), - "task_struct rss_stat MM_ANONPAGES", - RETURN_ON_ERROR)) - continue; - - rss += sync_rss; - - if (first == last) - break; - first++; + /* We will use rss_cache only for dumpfile */ + if (ACTIVE() || last->rss_cache == UNINITIALIZED) { + while (first <= last) + { + /* count 0 -> filepages */ + if (!readmem(first->task + + OFFSET(task_struct_rss_stat) + + OFFSET(task_rss_stat_count), KVADDR, + &sync_rss, + sizeof(int), + "task_struct rss_stat MM_FILEPAGES", + RETURN_ON_ERROR)) + continue; + + rss_cache += sync_rss; + + /* count 1 -> anonpages */ + if (!readmem(first->task + + OFFSET(task_struct_rss_stat) + + OFFSET(task_rss_stat_count) + + sizeof(int), + KVADDR, &sync_rss, + sizeof(int), + "task_struct rss_stat MM_ANONPAGES", + RETURN_ON_ERROR)) + continue; + + rss_cache += sync_rss; + + if (first == last) + break; + first++; + } + last->rss_cache = rss_cache; } + rss += last->rss_cache; tt->last_tgid = last; } } diff --git a/task.c b/task.c index 263a834..9868a6e 100644 --- a/task.c +++ b/task.c @@ -2947,6 +2947,7 @@ add_context(ulong task, char *tp) tg = tt->tgid_array + tt->running_tasks; tg->tgid = *tgid_addr; tg->task = task; + tg->rss_cache = UNINITIALIZED; if (do_verify && !verify_task(tc, do_verify)) { error(INFO, "invalid task address: %lx\n", tc->task); -- 2.33.1 ------------------------------ Message: 3 Date: Fri, 21 Jan 2022 07:35:22 +0000 From: HAGIO KAZUHITO(?????) <k-hagio-ab(a)nec.com&gt; To: lijiang <lijiang(a)redhat.com&gt; Cc: &quot;crash-utility(a)redhat.com&quot; <crash-utility(a)redhat.com&gt; Subject: Re: [Crash-utility] [PATCH] Remove ptype command from "ps -t" option to reduce memory and time Message-ID: < TYYPR01MB6777EC6D0A3A1847AA853965DD5B9(a)TYYPR01MB6777.jpnprd01.prod.outlook.com > Content-Type: text/plain; charset="utf-8" -----Original Message----- > On Wed, Jan 19, 2022 at 4:27 PM HAGIO KAZUHITO(?????) < k-hagio-ab(a)nec.com <mailto:k-hagio-ab@nec.com> > > wrote: > > > With some vmlinux e.g. RHEL9 ones, the first execution of the gdb ptype > command heavily consumes memory and time. The "ps -t" option uses it > in start_time_timespec() and it can be replaced with crash macros. > This can reduce about 1.4 GB memory and 6 seconds time comsumption in > the following test: > > $ echo "ps -t" | time crash vmlinux vmcore > > Without the patch: > 11.60user 0.43system 0:11.94elapsed 100%CPU (0avgtext+0avgdata 1837964maxresident)k > 0inputs+400outputs (0major+413636minor)pagefaults 0swaps > > With the patch: > 5.40user 0.16system 0:05.46elapsed 101%CPU (0avgtext+0avgdata 417896maxresident)k > 0inputs+384outputs (0major+41528minor)pagefaults 0swaps > > Although the ptype command and similar ones cannot be fully removed, > but removing some of them will make the use of crash safer, especially > for an automatic crash analyzer. > > Signed-off-by: Kazuhito Hagio <k-hagio-ab(a)nec.com <mailto: k-hagio-ab(a)nec.com&gt; > > --- > task.c | 25 +++++-------------------- > 1 file changed, 5 insertions(+), 20 deletions(-) > > diff --git a/task.c b/task.c > index 263a8344dd94..a79ed0d96fb5 100644 > --- a/task.c > +++ b/task.c > @@ -4662,8 +4662,6 @@ show_task_times(struct task_context *tcp, ulong flags) > static int > start_time_timespec(void) > { > - char buf[BUFSIZE]; > - > switch(tt->flags & (TIMESPEC | NO_TIMESPEC | START_TIME_NSECS)) > { > case TIMESPEC: > @@ -4677,24 +4675,11 @@ start_time_timespec(void) > > tt->flags |= NO_TIMESPEC; > > - open_tmpfile(); > - sprintf(buf, "ptype struct task_struct"); > - if (!gdb_pass_through(buf, NULL, GNU_RETURN_ON_ERROR)) { > - close_tmpfile(); > - return FALSE; > - } > - > - rewind(pc->tmpfile); > - while (fgets(buf, BUFSIZE, pc->tmpfile)) { > - if (strstr(buf, "start_time;")) { > - if (strstr(buf, "struct timespec")) { > - tt->flags &= ~NO_TIMESPEC; > - tt->flags |= TIMESPEC; > - } > - } > - } > - > - close_tmpfile(); > + if (VALID_MEMBER(task_struct_start_time) && > + STREQ(MEMBER_TYPE_NAME("task_struct", "start_time"), "timespec")) { > + tt->flags &= ~NO_TIMESPEC; > + tt->flags |= TIMESPEC; > + } > > if ((tt->flags & NO_TIMESPEC) && (SIZE(task_struct_start_time) == 8)) { > tt->flags &= ~NO_TIMESPEC; > -- > 2.27.0 > > > > For this one: > Acked-by: Lianbo Jiang <lijiang(a)redhat.com <mailto:lijiang@redhat.com> > > Thank you for reviewing, applied. Kazu ------------------------------ -- Crash-utility mailing list Crash-utility(a)redhat.com https://listman.redhat.com/mailman/listinfo/crash-utility End of Crash-utility Digest, Vol 196, Issue 15 **********************************************