Hello Dave,
The patch has been modified, using pc->cmd_cleanup to clean tc->mm_struct.
Please check the attached patch.
On 12/09/2014 10:30 PM, Dave Anderson wrote:
----- Original Message -----
On 12/08/2014 10:28 PM, Dave Anderson wrote:
----- Original Message -----
On 12/06/2014 04:11 AM, Dave Anderson wrote:
Interestingly enough, today I was asked to look at a vmcore in which an
oops
occurred during task exit after tsk->mm had been NULL'd out in exit_mm():
It almost matches what I am facing, when tsk->mm is set to NULL and memory
mapping is supposed to be displayed. This is a more simple implementation.
I have tried to command like vm [taskp | pid | [-M mm_struct]]. But it have
to modify a lot of thing.
By the way, I feel the code is becoming more and more complicated, maybe a
reconstruction is needed.
Well, the vm_area_dump() function is relatively stable, so let's not go crazy
here for what's essentially an "experimental" option.
I see.
Of course it has its limitations. Since the page tables are being broken down in this case,
"vm -p" fails:
crash> vm -M ffff880495120dc0 -p
PID: 4563 TASK: ffff88049863f500 CPU: 8 COMMAND: "postgres"
MM PGD RSS TOTAL_VM
0 0 0k 0k
VMA START END FLAGS FILE
ffff8804a085ce90 400000 f56000 8001875 /usr/local/greenplum-db-4.3.3.1/bin/postgres
VIRTUAL PHYSICAL
vm: invalid kernel virtual address: 50 type: "mm_struct pgd"
crash>
After a glance, the pgd comes from the mm of task_struct. We need a lot of work to make it
replaced by argument of -M, I don't think it worse it right now.
Actually it doesn't take much work at all. If both tc->mm_struct and tm->mm_struct_addr
are replaced with the supplied address:
tc->mm_struct = tm->mm_struct_addr = pc->curcmd_private;
then "vm -M ffff880495120dc0 -p" also works OK with my sample vmcore.
Yes, vm -p will tc->mm_struct to get pgd. But I was afraid permanently changing tc->mm_struct
is not good.
Take my core into consideration, the case is:
<cut>
crash> help -t | grep mm_struct
.mm_struct: 0
mm_struct: 354cc80
crash> vm -M 0xffff88003ae98ac0
PID: 4860 TASK: ffff88003ae7eaf0 CPU: 0 COMMAND: "bash"
MM PGD RSS TOTAL_VM
0 0 0k 0k
VMA START END FLAGS FILE
ffff88003acc3ad8 400000 4d4000 8001875 /bin/bash
...
crash> help -t | grep mm_struct
.mm_struct: ffff88003ae98ac0
mm_struct: 354cc80
crash>
<cut>
Is it OK to have ".mm_struct" changed here?
Probably not...
I did take a quick look at the other usages of tc->mm_struct, and I think
the only major difference is that the "search -u" command would search
the user-space memory of the changed task. But there's also an oddball
check for an i386 hypervisor callback from user-space, and it looks like
get_task_mem_usage() would use it from that point on, so I agree with
you that it should be restored to NULL.
Since there's a strong possibility of an error(FATAL, ...) call while
executing the command, it's not safe to simply restore it to NULL at the
end of the command, but rather the pc->cmd_cleanup() facility should
be used with the task address as the pc->cmd_cleanup_arg.
But it does seems like a worthwhile addition.
The patch doesn't check whether mm->owner or mm->mm_count are legitimate, but I'm not
sure whether it's even worth it? If it fails, it fails, and the help page should just
indicate that the command option is not guaranteed to work. Does the attached patch work
for you?
Similar to the core I got. And I modified the patch to add some check. At least I think
we need to make sure the address still belongs to a mm_struct object.
I suppose you could, although in all probability it's going to be stay in the mm_struct
slab cache, and worst case, have been re-used by another task.
So you like the modification.
Sure -- with the pc->cmd_cleanup in place, I don't see how it can hurt.
Dave
.
--
Regards
Qiao Nuohan
diff --git a/defs.h b/defs.h
index 2e52bc4..e2e2277 100755
--- a/defs.h
+++ b/defs.h
@@ -464,6 +464,7 @@ struct program_context {
#define MEMTYPE_KVADDR (0x2000)
#define MOD_SECTIONS (0x4000)
#define MOD_READNOW (0x8000)
+#define MM_STRUCT_FORCE (0x10000)
ulonglong curcmd_private; /* general purpose per-command info */
int cur_gdb_cmd; /* current gdb command */
int last_gdb_cmd; /* previously-executed gdb command */
@@ -1923,6 +1924,7 @@ struct offset_table { /* stash of commonly-used offsets */
long kernfs_node_parent;
long kmem_cache_cpu_partial;
long kmem_cache_cpu_cache;
+ long mm_struct_mm_users;
};
struct size_table { /* stash of commonly-used sizes */
diff --git a/main.c b/main.c
index d16ef12..c4ed229 100755
--- a/main.c
+++ b/main.c
@@ -1606,6 +1606,8 @@ dump_program_context(void)
fprintf(fp, "%sMOD_SECTIONS", others ? "|" : "");
if (pc->curcmd_flags & MOD_READNOW)
fprintf(fp, "%sMOD_READNOW", others ? "|" : "");
+ if (pc->curcmd_flags & MM_STRUCT_FORCE)
+ fprintf(fp, "%sMM_STRUCT_FORCE", others ? "|" : "");
fprintf(fp, ")\n");
fprintf(fp, " curcmd_private: %llx\n", pc->curcmd_private);
fprintf(fp, " cmd_cleanup: %lx\n", (ulong)pc->cmd_cleanup);
diff --git a/memory.c b/memory.c
index 3ac928d..85584eb 100755
--- a/memory.c
+++ b/memory.c
@@ -357,6 +357,7 @@ vm_init(void)
}
MEMBER_OFFSET_INIT(mm_struct_total_vm, "mm_struct", "total_vm");
MEMBER_OFFSET_INIT(mm_struct_start_code, "mm_struct", "start_code");
+ MEMBER_OFFSET_INIT(mm_struct_mm_users, "mm_struct", "mm_users");
MEMBER_OFFSET_INIT(vm_area_struct_vm_mm, "vm_area_struct", "vm_mm");
MEMBER_OFFSET_INIT(vm_area_struct_vm_next, "vm_area_struct", "vm_next");
MEMBER_OFFSET_INIT(vm_area_struct_vm_end, "vm_area_struct", "vm_end");
@@ -3302,9 +3303,14 @@ cmd_vm(void)
ref = NULL;
BZERO(&reference, sizeof(struct reference));
- while ((c = getopt(argcnt, args, "f:pmvR:P:xd")) != EOF) {
+ while ((c = getopt(argcnt, args, "f:pmvR:P:xdM:")) != EOF) {
switch(c)
{
+ case 'M':
+ pc->curcmd_private = htoll(optarg, FAULT_ON_ERROR, NULL);
+ pc->curcmd_flags |= MM_STRUCT_FORCE;
+ break;
+
case 'f':
if (flag)
argerrs++;
@@ -3594,6 +3600,48 @@ get_vm_flags(char *vma_buf)
return vm_flags;
}
+static void
+vm_cleanup(void *arg)
+{
+ ulong task;
+ struct task_context *tc;
+ ulong mm_struct;
+
+ pc->cmd_cleanup = NULL;
+ pc->cmd_cleanup_arg = NULL;
+
+ task = (ulong)arg;
+
+ fill_task_struct(task);
+ mm_struct = ULONG(tt->task_struct + OFFSET(task_struct_mm));
+
+ tc = task_to_context(task);
+ tc->mm_struct = mm_struct;
+}
+
+static int
+is_valid_mm(ulong mm)
+{
+ char kbuf[BUFSIZE];
+ char *p;
+ int mm_users;
+
+ if (!(p = vaddr_to_kmem_cache(mm, kbuf, VERBOSE)))
+ goto bailout;
+
+ if (!STRNEQ(p, "mm_struct"))
+ goto bailout;
+
+ readmem(mm + OFFSET(mm_struct_mm_users), KVADDR, &mm_users, sizeof(int),
+ "mm_struct mm_users", FAULT_ON_ERROR);
+
+ return mm_users;
+
+bailout:
+ error(FATAL, "invalid mm_struct address\n");
+ return 0;
+}
+
/*
* vm_area_dump() primarily does the work for cmd_vm(), but is also called
* from IN_TASK_VMA(), do_vtop(), and foreach(). How it behaves depends
@@ -3726,8 +3774,22 @@ vm_area_dump(ulong task, ulong flag, ulong vaddr, struct reference *ref)
!DO_REF_SEARCH(ref))
PRINT_VM_DATA();
- if (!tm->mm_struct_addr)
- return (ulong)NULL;
+ if (!tm->mm_struct_addr) {
+ if (pc->curcmd_flags & MM_STRUCT_FORCE) {
+ tc->mm_struct = tm->mm_struct_addr = pc->curcmd_private;
+
+ /*
+ * tc->mm_struct may be changed, use vm_cleanup to
+ * restore it.
+ */
+ pc->cmd_cleanup_arg = (void *)task;
+ pc->cmd_cleanup = vm_cleanup;
+
+ if (!is_valid_mm(tm->mm_struct_addr))
+ return (ulong)NULL;
+ } else
+ return (ulong)NULL;
+ }
if (flag & PRINT_MM_STRUCT) {
dump_struct("mm_struct", tm->mm_struct_addr, radix);
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility
