Re: [Crash-utility] About displaying virtual memory information of exiting task

----- Original Message ----- 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? And maybe the error message above should indicate "invalid or stale", given that the user-supplied address may have been valid earlier? 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(): - 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? Thanks, Dave