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
