On 12/11/2014 12:34 AM, Dave Anderson wrote:
----- Original Message -----
Hello Dave,
The patch has been modified, using pc->cmd_cleanup to clean tc->mm_struct.
Please check the attached patch.
Hello Qiao,
One major point, and a few minor ones...
First, this won't work with the sample vmcore I showed you:
+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;
+}
If you recall my sample vmcore, the mm->users was zero, but because
the mm->count was non-zero, mmdrop() had not called __mmdrop() to
free the mm_struct:
crash> mm_struct.mm_count,owner,mm_users ffff880495120dc0
mm_count = {
counter = 2
}
owner = 0xffff88049863f500
mm_users = {
counter = 0
}
crash>
So it should it test mm->count instead, correct?
Fixed.
And maybe the error message above should indicate "invalid or stale", given
that the user-supplied address may have been valid earlier?
Add an error message("stale mm_struct address") when mm_count equals to 0.
Secondly, wouldn't it make more sense to just pass pc->curcmd_private to
is_valid_mm() below, and if it fails, return NULL? If it's an invalid
argument, it doesn't make much sense to make the switch and set-up the
call to pc->cmd_cleanup():
Yes. Valid check should go first.
- 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;
+ }
And lastly, this works, but is somewhat of an overkill:
+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;
+}
The only way the tc->mm_struct swap is made is if tc->mm_struct was
originally set to NULL. So it could simply be:
tc = task_to_context(task);
tc->mm_struct = NULL;
Right?
I failed to realize tm->mm_struct_addr comes from tc->mm_struct firstly, and
if tm->mm_struct_addr == 0, then tc->mm_struct must equal to 0. Thanks for
pointing it out.
I made a more change, passing tc to vm_cleanup, then task_to_context() will also
be omitted.
Thanks,
Dave
.
--
Regards
Qiao Nuohan
diff --git a/defs.h b/defs.h
index 2e52bc4..7837fe4 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_count;
};
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..6df9ea0 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_count, "mm_struct", "mm_count");
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,44 @@ get_vm_flags(char *vma_buf)
return vm_flags;
}
+static void
+vm_cleanup(void *arg)
+{
+ struct task_context *tc;
+
+ pc->cmd_cleanup = NULL;
+ pc->cmd_cleanup_arg = NULL;
+
+ tc = (struct task_context *)arg;
+ tc->mm_struct = 0;
+}
+
+static int
+is_valid_mm(ulong mm)
+{
+ char kbuf[BUFSIZE];
+ char *p;
+ int mm_count;
+
+ if (!(p = vaddr_to_kmem_cache(mm, kbuf, VERBOSE)))
+ goto bailout;
+
+ if (!STRNEQ(p, "mm_struct"))
+ goto bailout;
+
+ readmem(mm + OFFSET(mm_struct_mm_count), KVADDR, &mm_count, sizeof(int),
+ "mm_struct mm_count", FAULT_ON_ERROR);
+
+ if (mm_count == 0)
+ error(FATAL, "stale mm_struct address\n");
+
+ return mm_count;
+
+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 +3770,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) {
+ if (!is_valid_mm(pc->curcmd_private))
+ return (ulong)NULL;
+
+ tc->mm_struct = tm->mm_struct_addr = pc->curcmd_private;
+
+ /*
+ * tc->mm_struct is changed, use vm_cleanup to
+ * restore it.
+ */
+ pc->cmd_cleanup_arg = (void *)tc;
+ pc->cmd_cleanup = vm_cleanup;
+ } 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
