10:15 a.m.
This series decreases crash startup and 'ps' processing time when handling dumps
with many tasks. Prior to the series a 1M task dump took 45m to load and 45m
more to run ps. Once patched, startup+ps time drops below 40 seconds.
Greg Thelen (4):
refactor store_context => add_context
refactor task_to_pid
remove unreachable (and slow) code
index task_context by task
defs.h | 2 +
task.c | 191 +++++++++++++++++++++++++++------------------------------
2 files changed, 92 insertions(+), 101 deletions(-)
--
2.17.0.484.g0c8726318c-goog
10:15 a.m.
New subject: [PATCH 1/4] refactor store_context => add_context
store_context is always used add entries to the tt->context_array, so
simply the code. No functional change.
Signed-off-by: Greg Thelen <gthelen(a)google.com>
---
task.c | 106 +++++++++++++++------------------------------------------
1 file changed, 28 insertions(+), 78 deletions(-)
diff --git a/task.c b/task.c
index 6d31699ca209..51aa2a255326 100644
--- a/task.c
+++ b/task.c
@@ -32,7 +32,7 @@ static void refresh_hlist_task_table_v3(void);
static void refresh_active_task_table(void);
static int radix_tree_task_callback(ulong);
static void refresh_radix_tree_task_table(void);
-static struct task_context *store_context(struct task_context *, ulong, char *);
+static struct task_context *add_context(ulong, char *);
static void refresh_context(ulong, ulong);
static ulong parent_of(ulong);
static void parent_list(ulong);
@@ -822,7 +822,6 @@ refresh_fixed_task_table(void)
{
int i;
ulong *tlp;
- struct task_context *tc;
ulong curtask;
ulong retries;
ulong curpid;
@@ -868,8 +867,7 @@ retry:
clear_task_cache();
- for (i = 0, tlp = (ulong *)tt->task_local,
- tt->running_tasks = 0, tc = tt->context_array;
+ for (i = 0, tlp = (ulong *)tt->task_local, tt->running_tasks = 0;
i < tt->max_tasks; i++, tlp++) {
if (TASK_IN_USE(*tlp)) {
if (!(tp = fill_task_struct(*tlp))) {
@@ -879,10 +877,7 @@ retry:
goto retry;
}
- if (store_context(tc, *tlp, tp)) {
- tc++;
- tt->running_tasks++;
- }
+ add_context(*tlp, tp);
}
}
@@ -966,7 +961,6 @@ refresh_unlimited_task_table(void)
{
int i;
ulong *tlp;
- struct task_context *tc;
ulong curtask;
ulong curpid;
struct list_data list_data, *ld;
@@ -1080,8 +1074,7 @@ retry:
clear_task_cache();
- for (i = 0, tlp = (ulong *)tt->task_local,
- tt->running_tasks = 0, tc = tt->context_array;
+ for (i = 0, tlp = (ulong *)tt->task_local, tt->running_tasks = 0;
i < tt->max_tasks; i++, tlp++) {
if (!(*tlp))
continue;
@@ -1108,10 +1101,7 @@ retry:
goto retry;
}
- if (store_context(tc, *tlp, tp)) {
- tc++;
- tt->running_tasks++;
- }
+ add_context(*tlp, tp);
}
if (DUMPFILE()) {
@@ -1143,7 +1133,6 @@ refresh_pidhash_task_table(void)
char *pidhash, *tp;
ulong *pp, next, pnext;
int len, cnt;
- struct task_context *tc;
ulong curtask;
ulong curpid;
ulong retries;
@@ -1284,8 +1273,7 @@ retry_pidhash:
clear_task_cache();
- for (i = 0, tlp = (ulong *)tt->task_local,
- tt->running_tasks = 0, tc = tt->context_array;
+ for (i = 0, tlp = (ulong *)tt->task_local, tt->running_tasks = 0;
i < tt->max_tasks; i++, tlp++) {
if (!(*tlp))
continue;
@@ -1317,10 +1305,7 @@ retry_pidhash:
goto retry_pidhash;
}
- if (store_context(tc, *tlp, tp)) {
- tc++;
- tt->running_tasks++;
- }
+ add_context(*tlp, tp);
}
FREEBUF(pidhash);
@@ -1361,7 +1346,6 @@ refresh_pid_hash_task_table(void)
char *tp;
ulong next, pnext;
int len, cnt;
- struct task_context *tc;
ulong curtask;
ulong curpid;
ulong retries;
@@ -1496,8 +1480,7 @@ retry_pid_hash:
clear_task_cache();
- for (i = 0, tlp = (ulong *)tt->task_local,
- tt->running_tasks = 0, tc = tt->context_array;
+ for (i = 0, tlp = (ulong *)tt->task_local, tt->running_tasks = 0;
i < tt->max_tasks; i++, tlp++) {
if (!(*tlp))
continue;
@@ -1529,10 +1512,7 @@ retry_pid_hash:
goto retry_pid_hash;
}
- if (store_context(tc, *tlp, tp)) {
- tc++;
- tt->running_tasks++;
- }
+ add_context(*tlp, tp);
}
FREEBUF(pid_hash);
@@ -1565,7 +1545,6 @@ refresh_hlist_task_table(void)
char *nodebuf;
int plen, len, cnt;
long value;
- struct task_context *tc;
ulong curtask;
ulong curpid;
ulong retries;
@@ -1768,8 +1747,7 @@ retry_pid_hash:
clear_task_cache();
- for (i = 0, tlp = (ulong *)tt->task_local,
- tt->running_tasks = 0, tc = tt->context_array;
+ for (i = 0, tlp = (ulong *)tt->task_local, tt->running_tasks = 0;
i < tt->max_tasks; i++, tlp++) {
if (!(*tlp))
continue;
@@ -1801,10 +1779,7 @@ retry_pid_hash:
goto retry_pid_hash;
}
- if (store_context(tc, *tlp, tp)) {
- tc++;
- tt->running_tasks++;
- }
+ add_context(*tlp, tp);
}
FREEBUF(pid_hash);
@@ -1835,7 +1810,6 @@ refresh_hlist_task_table_v2(void)
ulong next, pnext, pprev;
char *nodebuf;
int len, cnt;
- struct task_context *tc;
ulong curtask;
ulong curpid;
ulong retries;
@@ -2003,8 +1977,7 @@ retry_pid_hash:
clear_task_cache();
- for (i = 0, tlp = (ulong *)tt->task_local,
- tt->running_tasks = 0, tc = tt->context_array;
+ for (i = 0, tlp = (ulong *)tt->task_local, tt->running_tasks = 0;
i < tt->max_tasks; i++, tlp++) {
if (!(*tlp))
continue;
@@ -2036,10 +2009,7 @@ retry_pid_hash:
goto retry_pid_hash;
}
- if (store_context(tc, *tlp, tp)) {
- tc++;
- tt->running_tasks++;
- }
+ add_context(*tlp, tp);
}
FREEBUF(pid_hash);
@@ -2070,7 +2040,6 @@ refresh_hlist_task_table_v3(void)
ulong upid;
char *nodebuf;
int len, cnt;
- struct task_context *tc;
ulong curtask;
ulong curpid;
ulong retries;
@@ -2266,8 +2235,7 @@ chain_next:
clear_task_cache();
- for (i = 0, tlp = (ulong *)tt->task_local,
- tt->running_tasks = 0, tc = tt->context_array;
+ for (i = 0, tlp = (ulong *)tt->task_local, tt->running_tasks = 0;
i < tt->max_tasks; i++, tlp++) {
if (!(*tlp))
continue;
@@ -2299,10 +2267,7 @@ chain_next:
goto retry_pid_hash;
}
- if (store_context(tc, *tlp, tp)) {
- tc++;
- tt->running_tasks++;
- }
+ add_context(*tlp, tp);
}
FREEBUF(pid_hash);
@@ -2341,7 +2306,6 @@ refresh_radix_tree_task_table(void)
ulong *tlp;
char *tp;
struct radix_tree_pair rtp;
- struct task_context *tc;
char *pidbuf;
if (DUMPFILE() && (tt->flags & TASK_INIT_DONE)) /* impossible */
@@ -2494,8 +2458,7 @@ retry_radix_tree:
clear_task_cache();
- for (i = 0, tlp = (ulong *)tt->task_local,
- tt->running_tasks = 0, tc = tt->context_array;
+ for (i = 0, tlp = (ulong *)tt->task_local, tt->running_tasks = 0;
i < tt->max_tasks; i++, tlp++) {
if (!(*tlp))
continue;
@@ -2527,10 +2490,7 @@ retry_radix_tree:
goto retry_radix_tree;
}
- if (store_context(tc, *tlp, tp)) {
- tc++;
- tt->running_tasks++;
- }
+ add_context(*tlp, tp);
}
FREEBUF(pidbuf);
@@ -2549,7 +2509,6 @@ refresh_active_task_table(void)
int i;
char *tp;
int cnt;
- struct task_context *tc;
ulong curtask;
ulong curpid;
ulong retries;
@@ -2610,8 +2569,7 @@ retry_active:
clear_task_cache();
- for (i = 0, tlp = (ulong *)tt->task_local,
- tt->running_tasks = 0, tc = tt->context_array;
+ for (i = 0, tlp = (ulong *)tt->task_local, tt->running_tasks = 0;
i < tt->max_tasks; i++, tlp++) {
if (!(*tlp))
continue;
@@ -2643,10 +2601,7 @@ retry_active:
goto retry_active;
}
- if (store_context(tc, *tlp, tp)) {
- tc++;
- tt->running_tasks++;
- } else if (DUMPFILE())
+ if (!add_context(*tlp, tp) && DUMPFILE())
error(WARNING, "corrupt/invalid active task: %lx\n",
*tlp);
}
@@ -2667,11 +2622,11 @@ retry_active:
}
/*
- * Fill a task_context structure with the data from a task. If a NULL
- * task_context pointer is passed in, use the next available one.
+ * Initialize and return a new task_context structure with data from a task.
+ * NULL is returned on error.
*/
static struct task_context *
-store_context(struct task_context *tc, ulong task, char *tp)
+add_context(ulong task, char *tp)
{
pid_t *pid_addr, *tgid_addr;
char *comm_addr;
@@ -2680,6 +2635,7 @@ store_context(struct task_context *tc, ulong task, char *tp)
ulong *mm_addr;
int has_cpu;
int do_verify;
+ struct task_context *tc;
struct tgid_context *tg;
processor_addr = NULL;
@@ -2699,8 +2655,7 @@ store_context(struct task_context *tc, ulong task, char *tp)
else
do_verify = 0;
- if (!tc)
- tc = tt->context_array + tt->running_tasks;
+ tc = tt->context_array + tt->running_tasks;
pid_addr = (pid_t *)(tp + OFFSET(task_struct_pid));
tgid_addr = (pid_t *)(tp + OFFSET(task_struct_tgid));
@@ -2755,6 +2710,7 @@ store_context(struct task_context *tc, ulong task, char *tp)
if (has_cpu && (tt->flags & POPULATE_PANIC))
tt->panic_threads[tc->processor] = tc->task;
+ tt->running_tasks++;
return tc;
}
@@ -5907,12 +5863,8 @@ get_panic_context(void)
error(WARNING,
"active task %lx on cpu %d not found in PID hash\n\n",
task, i);
- if ((tp = fill_task_struct(task))) {
- if ((tc = store_context(NULL, task, tp)))
- tt->running_tasks++;
- else
- continue;
- }
+ if ((tp = fill_task_struct(task)))
+ add_context(task, tp);
}
}
@@ -7180,10 +7132,8 @@ found_panic_task:
* If the task list was corrupted, add this one in.
*/
if ((tp = fill_task_struct(lasttask))) {
- if ((tc = store_context(NULL, lasttask, tp))) {
- tt->running_tasks++;
+ if ((tc = add_context(lasttask, tp)))
return tc;
- }
}
}
--
2.17.0.484.g0c8726318c-goog
10:15 a.m.
New subject: [PATCH 2/4] refactor task_to_pid
task_to_pid and task_to_context both scan the set of tasks.
Simplify the code by refactoring task_to_pid to use task_to_context.
No functional change.
Signed-off-by: Greg Thelen <gthelen(a)google.com>
---
task.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/task.c b/task.c
index 51aa2a255326..82fa015805cb 100644
--- a/task.c
+++ b/task.c
@@ -4618,15 +4618,12 @@ pid_to_task(ulong pid)
ulong
task_to_pid(ulong task)
{
- int i;
struct task_context *tc;
-
- tc = FIRST_CONTEXT();
- for (i = 0; i < RUNNING_TASKS(); i++, tc++)
- if (tc->task == task)
- return(tc->pid);
-
+ tc = task_to_context(task);
+ if (tc != NULL)
+ return tc->pid;
+
return(NO_PID);
}
--
2.17.0.484.g0c8726318c-goog
10:15 a.m.
New subject: [PATCH 3/4] remove unreachable (and slow) code
task_exists() scans known tasks for a given task address.
refresh_radix_tree_task_table() looks like:
hq_open()
for cpus {
hq_enter(per_cpu_idle_thread)
}
for ... {
hq_enter(task)
}
hq_close()
tt->running_tasks = 0
for task in retrieve_list(hq_entries) {
if (task_exists()) {
duplicate task address
continue
}
add_context()
}
Because retrieve_list returns unique tasks, the above task_exists()
check will never find a duplicate task. So remove the check. This
converts the above loop from O(N^2) to O(N) which saves considerable
startup time when there are a large number of tasks. On a 1M task dump
this patch reduces startup time: 44m => 1m.
I was not able to test the other pid table routines, but I suspect they
may also benefit from similar removal.
Signed-off-by: Greg Thelen <gthelen(a)google.com>
---
task.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/task.c b/task.c
index 82fa015805cb..cddc1f5b651a 100644
--- a/task.c
+++ b/task.c
@@ -2473,16 +2473,6 @@ retry_radix_tree:
goto retry_radix_tree;
}
- if (task_exists(*tlp)) {
- error(WARNING,
- "%sduplicate task address found in task list: %lx\n",
- DUMPFILE() ? "\n" : "", *tlp);
- if (DUMPFILE())
- continue;
- retries++;
- goto retry_radix_tree;
- }
-
if (!(tp = fill_task_struct(*tlp))) {
if (DUMPFILE())
continue;
--
2.17.0.484.g0c8726318c-goog
1:31 p.m.
New subject: [PATCH 3/4] remove unreachable (and slow) code
----- Original Message -----
task_exists() scans known tasks for a given task address.
refresh_radix_tree_task_table() looks like:
hq_open()
for cpus {
hq_enter(per_cpu_idle_thread)
}
for ... {
hq_enter(task)
}
hq_close()
tt->running_tasks = 0
for task in retrieve_list(hq_entries) {
if (task_exists()) {
duplicate task address
continue
}
add_context()
}
Because retrieve_list returns unique tasks, the above task_exists()
check will never find a duplicate task. So remove the check. This
converts the above loop from O(N^2) to O(N) which saves considerable
startup time when there are a large number of tasks. On a 1M task dump
this patch reduces startup time: 44m => 1m.
I was not able to test the other pid table routines, but I suspect they
may also benefit from similar removal.
Hi Greg,
I'm thinking that the task_exists() removal may be the most beneficial
piece of the patch set, right?
I have a set of ~250 vmcore files gathered over the years, but only 2 of
them are from kernels using the idr radix tree scheme. I timed how long
it took to simply bring up all 250 vmcores to their crash prompt and then
quit immediately. These are the total times running with and without your
patch:
without the patch:
real 46m44.023s
user 17m29.956s
sys 0m58.940s
with your patch applied:
real 46m39.752s
user 17m36.496s
sys 0m58.269s
But as you've noted, given that all of the various refresh_xxx functions
use hq_enter(), the task_exists() call is pretty much useless.
I'm going to remove the task_exists() call from the remaining refresh_xxx
functions, rerun the test, and see how it affects the numbers. I suspect
it will help a bit, although the task count in the ~250 vmcores range from
22 to 4178, with only ~20 dumpfiles having more than 1000 tasks.
Thanks,
Dave
Signed-off-by: Greg Thelen <gthelen(a)google.com>
---
task.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/task.c b/task.c
index 82fa015805cb..cddc1f5b651a 100644
--- a/task.c
+++ b/task.c
@@ -2473,16 +2473,6 @@ retry_radix_tree:
goto retry_radix_tree;
}
- if (task_exists(*tlp)) {
- error(WARNING,
- "%sduplicate task address found in task list: %lx\n",
- DUMPFILE() ? "\n" : "", *tlp);
- if (DUMPFILE())
- continue;
- retries++;
- goto retry_radix_tree;
- }
-
if (!(tp = fill_task_struct(*tlp))) {
if (DUMPFILE())
continue;
--
2.17.0.484.g0c8726318c-goog
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
2:36 p.m.
New subject: [PATCH 3/4] remove unreachable (and slow) code
On Mon, Apr 9, 2018 at 11:32 AM Dave Anderson <anderson(a)redhat.com> wrote:
----- Original Message -----
> task_exists() scans known tasks for a given task address.
>
> refresh_radix_tree_task_table() looks like:
> hq_open()
> for cpus {
> hq_enter(per_cpu_idle_thread)
> }
> for ... {
> hq_enter(task)
> }
> hq_close()
>
> tt->running_tasks = 0
> for task in retrieve_list(hq_entries) {
> if (task_exists()) {
> duplicate task address
> continue
> }
> add_context()
> }
>
> Because retrieve_list returns unique tasks, the above task_exists()
> check will never find a duplicate task. So remove the check. This
> converts the above loop from O(N^2) to O(N) which saves considerable
> startup time when there are a large number of tasks. On a 1M task dump
> this patch reduces startup time: 44m => 1m.
>
> I was not able to test the other pid table routines, but I suspect they
> may also benefit from similar removal.
Hi Greg,
I'm thinking that the task_exists() removal may be the most
beneficial
piece of the patch set, right?
This patch (3/4) is the most beneficial part for startup time. But after
startup, 'ps' is still slow with a lot of tasks. We can speed up 'ps'
with
the final patch (4/4) ("index task_context by task"), which depends on 1/4
("refactor store_context => add_context") and 2/4 ("refactor
task_to_pid").
In my experiments on a 1M task dump:
* patch 3/4 saves ~45m of startup time
* patches 1,2,4/4 saves ~45m of ps time
Together patches 1-4 save ~90m for startup+ps.
I have a set of ~250 vmcore files gathered over the years, but only 2
of
them are from kernels using the idr radix tree scheme. I timed how long
it took to simply bring up all 250 vmcores to their crash prompt and then
quit immediately. These are the total times running with and without your
patch:
without the patch:
real 46m44.023s
user 17m29.956s
sys 0m58.940s
with your patch applied:
real 46m39.752s
user 17m36.496s
sys 0m58.269s
But as you've noted, given that all of the various refresh_xxx
functions
use hq_enter(), the task_exists() call is pretty much useless.
I'm going to remove the task_exists() call from the remaining
refresh_xxx
functions, rerun the test, and see how it affects the numbers. I suspect
it will help a bit, although the task count in the ~250 vmcores range from
22 to 4178, with only ~20 dumpfiles having more than 1000 tasks.
Thanks. I also suspect the other refresh_xxx routines will benefit.
Though I don't have such dumps handy to test.
3:38 p.m.
New subject: [PATCH 3/4] remove unreachable (and slow) code
----- Original Message -----
----- Original Message -----
> task_exists() scans known tasks for a given task address.
>
> refresh_radix_tree_task_table() looks like:
> hq_open()
> for cpus {
> hq_enter(per_cpu_idle_thread)
> }
> for ... {
> hq_enter(task)
> }
> hq_close()
>
> tt->running_tasks = 0
> for task in retrieve_list(hq_entries) {
> if (task_exists()) {
> duplicate task address
> continue
> }
> add_context()
> }
>
> Because retrieve_list returns unique tasks, the above task_exists()
> check will never find a duplicate task. So remove the check. This
> converts the above loop from O(N^2) to O(N) which saves considerable
> startup time when there are a large number of tasks. On a 1M task dump
> this patch reduces startup time: 44m => 1m.
>
> I was not able to test the other pid table routines, but I suspect they
> may also benefit from similar removal.
Hi Greg,
I'm thinking that the task_exists() removal may be the most beneficial
piece of the patch set, right?
I have a set of ~250 vmcore files gathered over the years, but only 2 of
them are from kernels using the idr radix tree scheme. I timed how long
it took to simply bring up all 250 vmcores to their crash prompt and then
quit immediately. These are the total times running with and without your
patch:
without the patch:
real 46m44.023s
user 17m29.956s
sys 0m58.940s
with your patch applied:
real 46m39.752s
user 17m36.496s
sys 0m58.269s
But as you've noted, given that all of the various refresh_xxx functions
use hq_enter(), the task_exists() call is pretty much useless.
I'm going to remove the task_exists() call from the remaining refresh_xxx
functions, rerun the test, and see how it affects the numbers. I suspect
it will help a bit, although the task count in the ~250 vmcores range from
22 to 4178, with only ~20 dumpfiles having more than 1000 tasks.
Thanks,
Dave
As it turns out, there wasn't any noticeable benefit with my set of dumpfiles:
without the patch:
real 46m58.872s
user 17m43.273s
sys 1m0.144s
with your patch plus all task_exists() calls removed:
real 46m58.617s
user 17m50.023s
sys 0m58.708s
But obviously the extremely large task count in your dumpfile shows the
benefit. In addition to the task_exists() removals, I also need to update
dump_task_table() to add a display of the context_by_task array (for help -T),
and the INDEXED_CONTEXTS flag (for help -[tT]). When I get it checked in to
github, I'll post the link here as confirmation.
Thanks,
Dave
10:15 a.m.
New subject: [PATCH 4/4] index task_context by task
For dumps with large task count, ps spends most its time in the
following stack:
task_to_context
task_to_pid
show_ps_data
show_ps
cmd_ps
exec_command
main_loop
captured_command_loop
catch_errors
captured_main
catch_errors
gdb_main_entry
gdb_main_loop
main
The above shows use of task_to_pid on each process. task_to_context is
O(N), thus ps is O(N^2).
Reduce task_to_context to O(N*log(N)) by using a binary search.
On a 1M task dump, this reduces ps time 45m => 17s.
Signed-off-by: Greg Thelen <gthelen(a)google.com>
---
defs.h | 2 ++
task.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++------
2 files changed, 60 insertions(+), 6 deletions(-)
diff --git a/defs.h b/defs.h
index 3a3b6f66573c..15346f01fd4e 100644
--- a/defs.h
+++ b/defs.h
@@ -811,6 +811,7 @@ struct tgid_context { /* tgid and task stored for each
task */
struct task_table { /* kernel/local task table data */
struct task_context *current;
struct task_context *context_array;
+ struct task_context **context_by_task; /* task_context sorted by task addr */
void (*refresh_task_table)(void);
ulong flags;
ulong task_start;
@@ -871,6 +872,7 @@ struct task_table { /* kernel/local task table
data */
#define START_TIME_NSECS (0x8000)
#define THREAD_INFO_IN_TASK (0x10000)
#define PID_RADIX_TREE (0x20000)
+#define INDEXED_CONTEXTS (0x40000)
#define TASK_SLUSH (20)
diff --git a/task.c b/task.c
index cddc1f5b651a..5ad77408530d 100644
--- a/task.c
+++ b/task.c
@@ -783,6 +783,10 @@ allocate_task_space(int cnt)
malloc(cnt * sizeof(struct task_context))))
error(FATAL, "cannot malloc context array (%d tasks)",
cnt);
+ if (!(tt->context_by_task = (struct task_context **)
+ malloc(cnt * sizeof(struct task_context*))))
+ error(FATAL, "cannot malloc context_by_task array (%d
tasks)",
+ cnt);
if (!(tt->tgid_array = (struct tgid_context *)
malloc(cnt * sizeof(struct tgid_context))))
error(FATAL, "cannot malloc tgid array (%d tasks)",
@@ -802,6 +806,13 @@ allocate_task_space(int cnt)
"%scannot realloc context array (%d tasks)",
(pc->flags & RUNTIME) ? "" : "\n", cnt);
+ if (!(tt->context_by_task = (struct task_context **)
+ realloc(tt->context_by_task,
+ cnt * sizeof(struct task_context*))))
+ error(FATAL,
+ "%scannot realloc context_by_task array (%d
tasks)",
+ (pc->flags & RUNTIME) ? "" :
"\n", cnt);
+
if (!(tt->tgid_array = (struct tgid_context *)
realloc(tt->tgid_array,
cnt * sizeof(struct tgid_context))))
@@ -2700,6 +2711,7 @@ add_context(ulong task, char *tp)
if (has_cpu && (tt->flags & POPULATE_PANIC))
tt->panic_threads[tc->processor] = tc->task;
+ tt->flags &= ~INDEXED_CONTEXTS;
tt->running_tasks++;
return tc;
}
@@ -2747,6 +2759,33 @@ refresh_context(ulong curtask, ulong curpid)
}
}
+static int
+sort_by_task(const void *arg1, const void *arg2)
+{
+ const struct task_context *t1, *t2;
+
+ t1 = *(const struct task_context **)arg1;
+ t2 = *(const struct task_context **)arg2;
+
+ if (t1->task == t2->task)
+ return 0;
+
+ return (t1->task < t2->task) ? -1 : 1;
+}
+
+/* sort context_by_task by task address */
+static void
+sort_context_by_task(void)
+{
+ int i;
+
+ for (i = 0; i < tt->running_tasks; i++)
+ tt->context_by_task[i] = &tt->context_array[i];
+ qsort(tt->context_by_task, tt->running_tasks,
+ sizeof(*tt->context_by_task), sort_by_task);
+ tt->flags |= INDEXED_CONTEXTS;
+}
+
/*
* Sort the task_context array by PID number; for PID 0, sort by processor.
*/
@@ -2759,6 +2798,8 @@ sort_context_array(void)
qsort((void *)tt->context_array, (size_t)tt->running_tasks,
sizeof(struct task_context), sort_by_pid);
set_context(curtask, NO_PID);
+
+ sort_context_by_task();
}
static int
@@ -2804,6 +2845,8 @@ sort_context_array_by_last_run(void)
qsort((void *)tt->context_array, (size_t)tt->running_tasks,
sizeof(struct task_context), sort_by_last_run);
set_context(curtask, NO_PID);
+
+ sort_context_by_task();
}
/*
@@ -4640,15 +4683,24 @@ task_exists(ulong task)
struct task_context *
task_to_context(ulong task)
{
- int i;
- struct task_context *tc;
+ struct task_context key, *tc, **found;
+ int i;
+
+ /* Binary search the context_by_task array. */
+ if (tt->flags & INDEXED_CONTEXTS) {
+ key.task = task;
+ tc = &key;
+ found = bsearch(&tc, tt->context_by_task, tt->running_tasks,
+ sizeof(*tt->context_by_task), sort_by_task);
+ return found ? *found : NULL;
+ }
tc = FIRST_CONTEXT();
- for (i = 0; i < RUNNING_TASKS(); i++, tc++)
+ for (i = 0; i < RUNNING_TASKS(); i++, tc++)
if (tc->task == task)
- return tc;
-
- return NULL;
+ return tc;
+
+ return NULL;
}
/*
--
2.17.0.484.g0c8726318c-goog
11:05 a.m.
----- Original Message -----
This series decreases crash startup and 'ps' processing time
when handling dumps
with many tasks. Prior to the series a 1M task dump took 45m to load and 45m
more to run ps. Once patched, startup+ps time drops below 40 seconds.
Hi Greg,
This sounds really good, although I have to say I've never encountered a dumpfile
that had such a large task count. I will review and test the patch-set next week.
BTW, did you test this on a live system that has such a large task count?
Like for example, as I recall, the task_exists() checks that you removed are
checking for tasks that were found when scanning the lists, but gone by the
time it got around to storing them.
Thanks,
Dave
Greg Thelen (4):
refactor store_context => add_context
refactor task_to_pid
remove unreachable (and slow) code
index task_context by task
defs.h | 2 +
task.c | 191 +++++++++++++++++++++++++++------------------------------
2 files changed, 92 insertions(+), 101 deletions(-)
--
2.17.0.484.g0c8726318c-goog
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
5:03 p.m.
On Fri, Apr 6, 2018 at 9:10 AM Dave Anderson <anderson(a)redhat.com> wrote:
----- Original Message -----
> This series decreases crash startup and 'ps' processing time when
handling dumps
> with many tasks. Prior to the series a 1M task dump took 45m to
load
and 45m
> more to run ps. Once patched, startup+ps time drops below 40
seconds.
Hi Greg,
This sounds really good, although I have to say I've never
encountered a
dumpfile
that had such a large task count. I will review and test the
patch-set
next week.
Thanks.
BTW, did you test this on a live system that has such a large task
count?
Like for example, as I recall, the task_exists() checks that you removed
are
checking for tasks that were found when scanning the lists, but gone
by
the
time it got around to storing them.
I have used these patches on a live machine. 'ps' runs and appears
correct. Though it's a moving target, so difficult to say if every line is
perfect. I don't see any duplicates. And I see a similar set of pids to
what /bin/ps reports. Though /bin/ps consistently reports a subset (~1000
fewer pids than crash reports). For these /bin/ps-mising pids, they are
visible in /proc/$pid and "kill -0 $pid" also works. So I think it's a
either a procfs bug or /bin/ps bug. stracing /bin/ps doesn't show any
syscall errors.
10:19 a.m.
----- Original Message -----
This series decreases crash startup and 'ps' processing time
when handling dumps
with many tasks. Prior to the series a 1M task dump took 45m to load and 45m
more to run ps. Once patched, startup+ps time drops below 40 seconds.
Thanks Greg -- the patch is queued for crash-7.2.2:
https://github.com/crash-utility/crash/commit/c1a8d0c968a256fc4617524aedb...
Dave
Greg Thelen (4):
refactor store_context => add_context
refactor task_to_pid
remove unreachable (and slow) code
index task_context by task
defs.h | 2 +
task.c | 191 +++++++++++++++++++++++++++------------------------------
2 files changed, 92 insertions(+), 101 deletions(-)
--
2.17.0.484.g0c8726318c-goog
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
12:27 p.m.
On Tue, Apr 10, 2018 at 8:20 AM Dave Anderson <anderson(a)redhat.com> wrote:
----- Original Message -----
> This series decreases crash startup and 'ps' processing time when
handling dumps
> with many tasks. Prior to the series a 1M task dump took 45m to
load
and 45m
> more to run ps. Once patched, startup+ps time drops below 40
seconds.
Thanks Greg -- the patch is queued for crash-7.2.2:
https://github.com/crash-utility/crash/commit/c1a8d0c968a256fc4617524aedb...
Thanks Dave. I tested the c1a8d0 github patch and the performance is good.
It might be too late to fix the commit log, but the patch contains all 4 of
my original patches, so it reduces startup+ps time 90m=>40s (rather than
45m=>40s). Not a big deal though.
2:08 p.m.
New subject: [PATCH 0/4] speed up handling of dumps with many tasks
----- Original Message -----
On Tue, Apr 10, 2018 at 8:20 AM Dave Anderson
<anderson(a)redhat.com> wrote:
> ----- Original Message -----
> > This series decreases crash startup and 'ps' processing time when
handling dumps
> > with many tasks. Prior to the series a 1M task dump took 45m to load and 45m
> > more to run ps. Once patched, startup+ps time drops below 40 seconds.
> Thanks Greg -- the patch is queued for crash-7.2.2:
https://github.com/crash-utility/crash/commit/c1a8d0c968a256fc4617524aedb...
Thanks Dave. I tested the c1a8d0 github patch and the performance is good.
It might be too late to fix the commit log, but the patch contains all 4 of
my original patches, so it reduces startup+ps time 90m=>40s (rather than
45m=>40s). Not a big deal though.
Yeah, since it looks like it would require a "force push" with an amended
message (which github strongly discourages because it screws up people who've
cloned the repo), I'm not going to do that.
But for what it's worth, I will change it in the changelog that gets
mailed out in the crash-7.2.2 ANNOUNCE message on this mailing list,
and it will also will be seen in the updated version of
http://people.redhat.com/anderson/crash.changelog.html.
Dave
10:26 p.m.
On Sun, Apr 15, 2018 at 1:27 AM Dave Anderson <anderson(a)redhat.com> wrote:
----- Original Message -----
> On Tue, Apr 10, 2018 at 8:20 AM Dave Anderson <anderson(a)redhat.com>
wrote:
>
> > ----- Original Message -----
> > > This series decreases crash startup and 'ps' processing time when
handling dumps
> > > with many tasks. Prior to the series a 1M task dump
took 45m to
load and 45m
> > > more to run ps. Once patched, startup+ps time drops
below 40
seconds.
>
> > Thanks Greg -- the patch is queued for crash-7.2.2:
>
>
>
https://github.com/crash-utility/crash/commit/c1a8d0c968a256fc4617524aedb...
>
> Thanks Dave. I tested the c1a8d0 github patch and the performance is
good.
>
> It might be too late to fix the commit log, but the patch contains all
4 of
> my original patches, so it reduces startup+ps time 90m=>40s
(rather than
> 45m=>40s). Not a big deal though.
Yeah, since it looks like it would require a "force push"
with an amended
message (which github strongly discourages because it screws up people
who've
cloned the repo), I'm not going to do that.
But for what it's worth, I will change it in the changelog that
gets
mailed out in the crash-7.2.2 ANNOUNCE message on this mailing list,
and it will also will be seen in the updated version of
http://people.redhat.com/anderson/crash.changelog.html.
Makes sense. Thanks.
2345
days inactive
2355
days old
devel@lists.crash-utility.osci.io
13 comments
2 participants
participants (2)
-
Dave Anderson -
Greg Thelen