On Wed 2017-10-04 14:52 -0400, Dave Anderson wrote:
> Couple minor things about the patch...
[ ... ]
> I'm not sure why it couldn't be used on a live dump:
>
> + if (Tflag && LIVE())
> + error(FATAL,
> + "-T option cannot be used on a live "
> + "system or live dump\n");
> +
>
> I agree that it should be restricted for a live system, but it
> seems safe to allow it for a live dump? (i.e., use ACTIVE() instead)
> With "bt", it does make sense to restrict it, because it's trying to
> unwind/translate what's being actively written on the stack when the
> dump was taken. But just a simple search should be safe.
Fair enough - I agree a 'live dump' (i.e. with pc->flags2 & LIVE_DUMP set)
is fine for this option. I'll use ACTIVE() instead.
> And lastly, this minor nit -- this kind of seems like overkill:
[ ... ]
> Why not something simple like this:
>
> - if (tflag) {
> + if (tflag || Tflag) {
> searchinfo.tasks_found = 0;
> tc = FIRST_CONTEXT();
> for (i = 0; i < RUNNING_TASKS(); i++, tc++) {
> + if (Tflag && !is_task_active(tc->task))
> + continue;
> searchinfo.vaddr_start =
> GET_STACKBASE(tc->task);
> searchinfo.vaddr_end = GET_STACKTOP(tc->task);
> searchinfo.task_context = tc;
> searchinfo.do_task_header = TRUE;
> search_virtual(&searchinfo);
> }
> break;
> }
I don't like the above since we'd have to traverse RUNNING_TASKS() just to
find each active task. If I understand correctly, under a live dump the
panic_threads[] should be populated. So it should be safe to use?
> It would also keep the searchinfo initializations contained within
> the cmd_search() function like all of the other arguments.
How about the following then:
>From 46710702ca2fa2b25411293c98863d353f646c40 Mon Sep 17 00:00:00 2001
From: Aaron Tomlin <[email protected]>
Date: Tue, 3 Oct 2017 17:30:04 +0100
Subject: [PATCH] search: Introduce -T option
Like the -t option, except consider only the "active" task
on each CPU and cannot be used on a live system.
The -t and -T options are mutually exclusive.
Signed-off-by: Aaron Tomlin <[email protected]>
---
help.c | 4 +++-
memory.c | 53 +++++++++++++++++++++++++++++++++++++++--------------
2 files changed, 42 insertions(+), 15 deletions(-)
diff --git a/help.c b/help.c
index 2d80202..3e65b15 100644
--- a/help.c
+++ b/help.c
@@ -2862,7 +2862,7 @@ NULL
char *help_search[] = {
"search",
"search memory",
-"[-s start] [ -[kKV] | -u | -p | -t ] [-e end | -l length] [-m mask]\n"
+"[-s start] [ -[kKV] | -u | -p | -t|-T ] [-e end | -l length] [-m mask]\n"
" [-x count] -[cwh] [value | (expression) | symbol | string] ...",
" This command searches for a given value within a range of user virtual,
kernel",
" virtual, or physical memory space. If no end nor length value is entered,
",
@@ -2893,6 +2893,8 @@ char *help_search[] = {
" -t Search only the kernel stack pages of every task. If one or
more",
" matches are found in a task's kernel stack, precede the output",
" with a task-identifying header.",
+" -T Same as -t, except only the active task(s) are considered.",
+" This option cannot be used on a live system.",
" -e end Stop the search at this hexadecimal user or kernel virtual",
" address, kernel symbol, or physical address. The end address",
" must be appropriate for the memory type specified.",
diff --git a/memory.c b/memory.c
index 8efe0b2..8b026ce 100644
--- a/memory.c
+++ b/memory.c
@@ -13864,6 +13864,12 @@ generic_get_kvaddr_ranges(struct vaddr_range *rp)
return cnt;
}
+#define PREPARE_SEARCHINFO_FOR_TASK_SEARCH(si, tc) \
+ si.tasks_found = 0; \
+ si.vaddr_start = GET_STACKBASE((tc)->task); \
+ si.vaddr_end = GET_STACKTOP((tc)->task); \
+ si.task_context = tc; \
+ si.do_task_header = TRUE;
/*
* Search for a given value between a starting and ending address range,
@@ -13882,7 +13888,7 @@ cmd_search(void)
ulong value, mask, len;
ulong uvaddr_start, uvaddr_end;
ulong kvaddr_start, kvaddr_end, range_end;
- int sflag, Kflag, Vflag, pflag, tflag;
+ int sflag, Kflag, Vflag, pflag, Tflag, tflag;
struct searchinfo searchinfo;
struct syment *sp;
struct node_table *nt;
@@ -13896,7 +13902,7 @@ cmd_search(void)
context = max = 0;
start = end = 0;
- value = mask = sflag = pflag = Kflag = Vflag = memtype = len = tflag =
0;
+ value = mask = sflag = pflag = Kflag = Vflag = memtype = len = Tflag =
tflag = 0;
kvaddr_start = kvaddr_end = 0;
uvaddr_start = UNINITIALIZED;
uvaddr_end = COMMON_VADDR_SPACE() ? (ulong)(-1) : machdep->kvbase;
@@ -13933,7 +13939,7 @@ cmd_search(void)
searchinfo.mode = SEARCH_ULONG; /* default search */
- while ((c = getopt(argcnt, args, "tl:ukKVps:e:v:m:hwcx:")) != EOF) {
+ while ((c = getopt(argcnt, args, "Ttl:ukKVps:e:v:m:hwcx:")) != EOF) {
switch(c)
{
case 'u':
@@ -14038,12 +14044,17 @@ cmd_search(void)
context = dtoi(optarg, FAULT_ON_ERROR, NULL);
break;
+ case 'T':
case 't':
if (XEN_HYPER_MODE())
error(FATAL,
- "-t option is not applicable to the "
- "Xen hypervisor\n");
- tflag++;
+ "-%c option is not applicable to the "
+ "Xen hypervisor\n", c);
+
+ if (c == 'T')
+ Tflag++;
+ else if (c == 't')
+ tflag++;
break;
default:
@@ -14052,10 +14063,19 @@ cmd_search(void)
}
}
- if (tflag && (memtype || start || end || len))
+ if ((tflag || Tflag) && (memtype || start || end || len))
+ error(FATAL,
+ "-%c option cannot be used with other "
+ "memory-selection options\n",
+ tflag ? 't' : 'T');
+
+ if (Tflag && ACTIVE())
error(FATAL,
- "-t option cannot be used with other "
- "memory-selection options\n");
+ "-T option cannot be used on a live system\n");
+
+ if (tflag && Tflag)
+ error(FATAL,
+ "-t and -T options are mutually exclusive\n");
if (XEN_HYPER_MODE()) {
memtype = KVADDR;
@@ -14328,14 +14348,19 @@ cmd_search(void)
break;
}
+ if (Tflag) {
+ for (c = 0; c < NR_CPUS; c++)
+ if ((tc =
task_to_context(tt->panic_threads[c]))) {
+
PREPARE_SEARCHINFO_FOR_TASK_SEARCH(searchinfo, tc);
+ search_virtual(&searchinfo);
+ }
+ break;
+ }
+
if (tflag) {
- searchinfo.tasks_found = 0;
tc = FIRST_CONTEXT();
for (i = 0; i < RUNNING_TASKS(); i++, tc++) {
- searchinfo.vaddr_start =
GET_STACKBASE(tc->task);
- searchinfo.vaddr_end = GET_STACKTOP(tc->task);
- searchinfo.task_context = tc;
- searchinfo.do_task_header = TRUE;
+ PREPARE_SEARCHINFO_FOR_TASK_SEARCH(searchinfo,
tc);
search_virtual(&searchinfo);
}
break;
--
2.9.4
--
Crash-utility mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/crash-utility
