Hello Dave,
I have modified the patch by your idea.
Please check.
Thanks,
Pan
|
From 043f599d0fa02e9f81cd2187b27dff3bacfecdf6 Mon Sep 17 00:00:00 2001
From: panfengyun <panfy.fnst@xxxxxxxxxxxxxx>
Date: Tue, 9 Sep 2014 13:26:22 +0800
Subject: [PATCH] improve ps performance
When using ps command in the system which contains numerous tasks, especilly RHEL 7,
it will spend too much time to get the data of tasks.
we collect an output of ps command as part of summary information of crash dump,
and in catastrophic case, crash dump could have a huge number of tasks due to
the bug causing the system panic. But it's a problem if ps command needs to
spend several hours to finish execution.
After investigation, what we found is, because of the following kernel commit
and a fix in crash 7.0.4, we need a lot time to wait for ps command to calculate
rss.
kernel commit:
<cut>
commit 34e55232e59f7b19050267a05ff1226e5cd122a5
Author: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>
Date: Fri Mar 5 13:41:40 2010 -0800
mm: avoid false sharing of mm_counter
...
<cut>
crash fix:
<cut>
7.0.4 - Fix for the "ps" command's display of per-task RSS and %MEM values
in Linux 2.6.34 and later kernels in which SPLIT_RSS_COUNTING is
enabled. Without the patch, the values are only taken from each
task's mm_struct.rss_stat structure, which may contain stale values
because they may not be synchronized with the RSS values stored
in each per-thread task_struct.rss_stat structure; this may lead
to invalid or slightly low RSS values, and worst-case, the %MEM
value may show garbage percentage values.
(vinayakm.list@xxxxxxxxx)
<cut>
So:
1. we fix rss calculation in get_task_mem_usage() to improve performance of ps
command.
2. And we also fix the part of getting enumerators MM_FILEPAGES and MM_ANONPAGES
which also speeds up ps command.
The 1st fix improves the performance of ps mostly.
we also have some tests. please check the following data:
OS process number without fix fix all 2 parts
---------------------------------------------------------------------
RHEL6 ~100k ~180s ~70s
RHEL7 ~100k ~51870s(~14h) ~58s
Signed-off-by: panfengyun <panfy.fnst@xxxxxxxxxxxxxx>
---
crash-7.0.7/defs.h | 10 ++++++
crash-7.0.7/main.c | 1 +
crash-7.0.7/memory.c | 75 ++++++++++++++++++++++++++++++++++++++++----------
crash-7.0.7/task.c | 49 ++++++++++++++++++++++++++++++++
4 files changed, 120 insertions(+), 15 deletions(-)
diff --git a/crash-7.0.7/defs.h b/crash-7.0.7/defs.h
index 44df6ae..2fb3a4c 100755
--- a/crash-7.0.7/defs.h
+++ b/crash-7.0.7/defs.h
@@ -761,10 +761,18 @@ struct task_context { /* context stored for each task */
struct task_context *tc_next;
};
+struct tgid_task_context{ /* tgid and task stored for each task */
+ ulong tgid;
+ ulong task;
+};
+
struct task_table { /* kernel/local task table data */
struct task_context *current;
struct task_context *context_array;
+ struct tgid_task_context *ttc_array;
void (*refresh_task_table)(void);
+ long filepages;
+ long anonpages;
ulong flags;
ulong task_start;
ulong task_end;
@@ -4834,6 +4842,8 @@ ulong generic_get_stackbase(ulong);
ulong generic_get_stacktop(ulong);
void dump_task_table(int);
void sort_context_array(void);
+void sort_tgid_task_context_array(void);
+int sort_by_tgid(const void *, const void *);
int in_irq_ctx(ulonglong, int, ulong);
/*
diff --git a/crash-7.0.7/main.c b/crash-7.0.7/main.c
index b0524d2..3d0485c 100755
--- a/crash-7.0.7/main.c
+++ b/crash-7.0.7/main.c
@@ -773,6 +773,7 @@ reattempt:
} else if (!(pc->flags & MINIMAL_MODE)) {
tt->refresh_task_table();
sort_context_array();
+ sort_tgid_task_context_array();
}
}
if (!STREQ(pc->curcmd, pc->program_name))
diff --git a/crash-7.0.7/memory.c b/crash-7.0.7/memory.c
index c97dd39..49b86f7 100755
--- a/crash-7.0.7/memory.c
+++ b/crash-7.0.7/memory.c
@@ -225,6 +225,7 @@ static int next_module_vaddr(ulong, ulong *);
static int next_identity_mapping(ulong, ulong *);
static int vm_area_page_dump(ulong, ulong, ulong, ulong, ulong,
struct reference *);
+void set_pages(void);
static int dump_swap_info(ulong, ulong *, ulong *);
static void swap_info_init(void);
static char *get_swapdev(ulong, char *);
@@ -1066,6 +1067,7 @@ vm_init(void)
page_flags_init();
+ set_pages();
vt->flags |= VM_INIT;
}
@@ -4072,6 +4074,31 @@ in_user_stack(ulong task, ulong vaddr)
}
/*
+ * set the const value of filepages and anonpages
+ * according to MM_FILEPAGES and MM_ANONPAGES.
+ */
+void
+set_pages(void)
+{
+ long anonpages, filepages;
+ if (VALID_MEMBER(mm_struct_rss))
+ return;
+ if (VALID_MEMBER(mm_struct_rss_stat))
+ {
+ if (!enumerator_value("MM_FILEPAGES", &filepages) ||
+ !enumerator_value("MM_ANONPAGES", &anonpages))
+ {
+ filepages = 0;
+ anonpages = 1;
+ }
+ tt->filepages = filepages;
+ tt->anonpages = anonpages;
+ } else {
+ return;
+ }
+}
+
+/*
* Fill in the task_mem_usage structure with the RSS, virtual memory size,
* percent of physical memory being used, and the mm_struct address.
*/
@@ -4108,11 +4135,8 @@ get_task_mem_usage(ulong task, struct task_mem_usage *tm)
if (VALID_MEMBER(mm_struct_rss_stat)) {
long anonpages, filepages;
- if (!enumerator_value("MM_FILEPAGES", &filepages) ||
- !enumerator_value("MM_ANONPAGES", &anonpages)) {
- filepages = 0;
- anonpages = 1;
- }
+ anonpages = tt->anonpages;
+ filepages = tt->filepages;
rss += LONG(tt->mm_struct +
OFFSET(mm_struct_rss_stat) +
OFFSET(mm_rss_stat_count) +
@@ -4125,19 +4149,36 @@ get_task_mem_usage(ulong task, struct task_mem_usage *tm)
/* Check whether SPLIT_RSS_COUNTING is enabled */
if (VALID_MEMBER(task_struct_rss_stat)) {
- int i, sync_rss;
- ulong tgid;
- struct task_context *tc1;
+ int sync_rss;
+ struct tgid_task_context *ttc_array, *ttc, *first, *last;
+
+ ttc_array = tt->ttc_array;
+ if (!(ttc = (struct tgid_task_context *)
+ malloc(sizeof(struct tgid_task_context))))
+ error(FATAL, "cannot malloc ttc at get_task_mem_usage()\n");
+ ttc->tgid = task_tgid(task);
+
+ ttc = (struct tgid_task_context *)bsearch(ttc, ttc_array, RUNNING_TASKS(),
+ sizeof(struct tgid_task_context), sort_by_tgid);
+ if (NULL == ttc)
+ error(FATAL,
+ "cannot search the tgid_task_context int ttc_array : %lx\n",task);
- tgid = task_tgid(task);
+ /* find the first element which have same tgid */
+ first = ttc;
+ while ((first > ttc_array) && ((first -1 )->tgid == first->tgid))
+ first--;
- tc1 = FIRST_CONTEXT();
- for (i = 0; i < RUNNING_TASKS(); i++, tc1++) {
- if (task_tgid(tc1->task) != tgid)
- continue;
+ /* find the last element which have same tgid */
+ last = ttc;
+ while ((last < (ttc_array + (RUNNING_TASKS() - 1))) &&
+ (last->tgid == (last + 1)->tgid))
+ last++;
+ while (first <= last)
+ {
/* count 0 -> filepages */
- if (!readmem(tc1->task +
+ if (!readmem(first->task +
OFFSET(task_struct_rss_stat) +
OFFSET(task_rss_stat_count), KVADDR,
&sync_rss,
@@ -4149,7 +4190,7 @@ get_task_mem_usage(ulong task, struct task_mem_usage *tm)
rss += sync_rss;
/* count 1 -> anonpages */
- if (!readmem(tc1->task +
+ if (!readmem(first->task +
OFFSET(task_struct_rss_stat) +
OFFSET(task_rss_stat_count) +
sizeof(int),
@@ -4160,6 +4201,10 @@ get_task_mem_usage(ulong task, struct task_mem_usage *tm)
continue;
rss += sync_rss;
+
+ if(first == last)
+ break;
+ first++;
}
}
diff --git a/crash-7.0.7/task.c b/crash-7.0.7/task.c
index 75b1964..ce7ca59 100755
--- a/crash-7.0.7/task.c
+++ b/crash-7.0.7/task.c
@@ -493,6 +493,7 @@ task_init(void)
}
sort_context_array();
+ sort_tgid_task_context_array();
if (pc->flags & SILENT)
initialize_task_state();
@@ -639,6 +640,11 @@ allocate_task_space(int cnt)
malloc(cnt * sizeof(struct task_context))))
error(FATAL, "cannot malloc context array (%d tasks)",
cnt);
+ if (!(tt->ttc_array = (struct tgid_task_context *)
+ malloc(cnt * sizeof(struct tgid_task_context))))
+ error(FATAL, "cannot malloc ttc array (%d tasks)",
+ cnt);
+
} else {
if (!(tt->task_local = (void *)
realloc(tt->task_local, cnt * sizeof(void *))))
@@ -652,6 +658,13 @@ allocate_task_space(int cnt)
error(FATAL,
"%scannot realloc context array (%d tasks)",
(pc->flags & RUNTIME) ? "" : "\n", cnt);
+
+ if (!(tt->ttc_array = (struct tgid_task_context *)
+ realloc(tt->ttc_array,
+ cnt * sizeof(struct tgid_task_context))))
+ error(FATAL,
+ "%scannot realloc ttc array (%d tasks)",
+ (pc->flags & RUNTIME) ? "" : "\n", cnt);
}
}
@@ -2281,8 +2294,10 @@ store_context(struct task_context *tc, ulong task, char *tp)
int *processor_addr;
ulong *parent_addr;
ulong *mm_addr;
+ ulong *tgid;
int has_cpu;
int do_verify;
+ struct tgid_task_context *ttc;
processor_addr = NULL;
@@ -2320,6 +2335,7 @@ store_context(struct task_context *tc, ulong task, char *tp)
else
parent_addr = (ulong *)(tp + OFFSET(task_struct_parent));
mm_addr = (ulong *)(tp + OFFSET(task_struct_mm));
+ tgid = (ulong *)(tp + OFFSET(task_struct_tgid));
has_cpu = task_has_cpu(task, tp);
tc->pid = (ulong)(*pid_addr);
@@ -2330,6 +2346,14 @@ store_context(struct task_context *tc, ulong task, char *tp)
tc->task = task;
tc->tc_next = NULL;
+ /*
+ * Fill a tgid_task_context structure with the data from tc.
+ */
+ ttc = tt->ttc_array + tt->running_tasks;
+ ttc->tgid = *tgid;
+ ttc->task = task;
+
+
if (do_verify && !verify_task(tc, do_verify)) {
error(INFO, "invalid task address: %lx\n", tc->task);
BZERO(tc, sizeof(struct task_context));
@@ -2445,6 +2469,30 @@ sort_context_array_by_last_run(void)
}
/*
+ * Set the tgid_task_context array by tgid number.
+ */
+void
+sort_tgid_task_context_array(void)
+{
+ if (VALID_MEMBER(mm_struct_rss) || (!VALID_MEMBER(task_struct_rss_stat)))
+ return;
+
+ qsort((void *)tt->ttc_array, (size_t)tt->running_tasks,
+ sizeof(struct tgid_task_context), sort_by_tgid);
+}
+int
+sort_by_tgid(const void *arg1, const void *arg2)
+{
+ struct tgid_task_context *t1, *t2;
+
+ t1 = (struct tgid_task_context *)arg1;
+ t2 = (struct tgid_task_context *)arg2;
+
+ return (t1->tgid < t2->tgid ? -1 :
+ t1->tgid == t2->tgid ? 0 : 1);
+}
+
+/*
* Keep a stash of the last task_struct accessed. Chances are it will
* be hit several times before the next task is accessed.
*/
@@ -6577,6 +6625,7 @@ dump_task_table(int verbose)
fprintf(fp, " .tc_next: %lx\n", (ulong)tc->tc_next);
}
fprintf(fp, " context_array: %lx\n", (ulong)tt->context_array);
+ fprintf(fp, " ttc_array: %lx\n", (ulong)tt->ttc_array);
fprintf(fp, "refresh_task_table: ");
if (tt->refresh_task_table == refresh_fixed_task_table)
fprintf(fp, "refresh_fixed_task_table()\n");
--
1.7.1
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility
