Hi Kazu, On Wed, Jan 26, 2022 at 4:37 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@xxxxxxx> wrote: > > -----Original Message----- > > 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@xxxxxxxxxx> > > Nice, it's a huge improvement in performance! > And the patch looks good to me and tested ok. > > Acked-by: Kazuhito Hagio <k-hagio-ab@xxxxxxx> > > Thanks, > Kazu > Thanks for reviewing the patch! Thanks, Tao Liu > > --- > > 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 > > > > -- > > Crash-utility mailing list > > Crash-utility@xxxxxxxxxx > > https://listman.redhat.com/mailman/listinfo/crash-utility > -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/crash-utility