Hi Kazu,
On Wed, Jan 26, 2022 at 4:37 PM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab(a)nec.com> 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(a)redhat.com>
 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(a)nec.com>
 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(a)redhat.com
 > 
https://listman.redhat.com/mailman/listinfo/crash-utility