----- Original Message ----- Toshi, I don't want to add any new initialization-time code -- especially if it's related to the NT_PRSTATUS notes -- that can abort a crash session unnecessarily. In your new crash_was_lost_crash_note() function, there are these two FAULT_ON_ERROR readmem() calls: readmem(symbol_value("crash_notes"), KVADDR, &crash_notes_ptr, sizeof(ulong), "crash_notes", FAULT_ON_ERROR); and readmem(crash_notes_ptr, KVADDR, buf, SIZE(note_buf), "cpu crash_notes", FAULT_ON_ERROR); Although they are highly unlikely to fail, can you please make both of them RETURN_ON_ERROR, and if the readmem() fails, have it bail out and return FALSE? And then, if necessary, make any adjustments to map_cpus_to_prstatus_kdump_cmprs() to handle that remote possibility. You should be able to test it with your patched kernel. Also, I don't understand the wording of this error message at the end of crash_was_lost_crash_note(): error(WARNING, "catch lost crash_notes at cpu#%d\n", cpu); Can you re-word that? The notes were not "lost", but rather were "not saved" by the crashing system. Lastly, in __diskdump_memory_dump(), you just skip the "lost" notes sections: for (i = 0, j = 0; j < dd->num_prstatus_notes; i++) { if (dd->nt_prstatus_percpu[i] == NULL) continue; fprintf(fp, " notes[%d]: %lx\n", i, (ulong)dd->nt_prstatus_percpu[i]); j++; } Can you make it more obvious, say, by displaying something like: notes[6]: (not saved) Thanks, Dave

----- Original Message ----- OK, now I'm getting confused... I note that currently __diskdump_memory_dump() does this: for (i = 0; i < dd->num_prstatus_notes; i++) { fprintf(fp, " notes[%d]: %lx\n", i, (ulong)dd->nt_prstatus_percpu[i]); } But your patch does this: nrcpus = kt->kernel_NR_CPUS ? kt->kernel_NR_CPUS : NR_CPUS; for (i = 0; i < nrcpus; i++) { if (dd->nt_prstatus_percpu[i] == NULL) { fprintf(fp, " notes[%d]: (not saved)\n", i); continue; } fprintf(fp, " notes[%d]: %lx\n", i, (ulong)dd->nt_prstatus_percpu[i]); } Now surely we don't want to dump "NR_CPUS" notes pointers, unless there actually are that many cpus in the system. For example, in RHEL6 sets CONFIG_NR_CPUS to 4096 for x86_64, but rarely do we see a dumpfile with that many cpus. So I would prefer that the for loop continue to be bounded by "dd->num_prstatus_notes". However, given that process_elf32_notes() and process_elf64_notes() do a cursory test of each note by checking for the NT_PRSTATUS type, I'm presuming that in your dumpfile, dd->num_prstatus_notes must be equal to 1, correct? void process_elf64_notes(void *note_buf, unsigned long size_note) { Elf64_Nhdr *nt; size_t index, len = 0; int num = 0; for (index = 0; index < size_note; index += len) { nt = note_buf + index; if(nt->n_type == NT_PRSTATUS) { dd->nt_prstatus_percpu[num] = nt; num++; } len = sizeof(Elf64_Nhdr); len = roundup(len + nt->n_namesz, 4); len = roundup(len + nt->n_descsz, 4); } if (num > 0) { pc->flags2 |= ELF_NOTES; dd->num_prstatus_notes = num; } return; } But then map_cpus_to_prstatus_kdump_cmprs() remaps the notes without modifying the dd->num_prstatus_notes counter. So then there's this: void * diskdump_get_prstatus_percpu(int cpu) { if ((cpu < 0) || (cpu >= dd->num_prstatus_notes)) return NULL; return dd->nt_prstatus_percpu[cpu]; } So in a case such as your example, where cpu 2 was the only cpu that saved its notes, the function above would incorrectly return NULL when called with diskdump_get_prstatus_percpu(2). What am I missing? Dave

(2012/06/20 3:25), Dave Anderson wrote: A dd->num_prstatus_notes is equal to number of NT_PRSTATUS notes in dump file and unless nt_prstatus_percpu[] is continuous from 0, diskdump_get_prstatus_percpu() can possibly return NULL. I seem to be minor bug, if some of cpus are offline while panic, can't backtrace the same number of cpus from tail because of returning NULL. I think current task's PC/SP can not obtain from thread_struct corectly, then NT_PRSTATUS notes need to be stored for them instead. But anyway, panic under offline is very unlikely condition as same as the NMI failed. At first, my patch is trying to find out tainted note area in base kernel. Such notes can not be found out from dumpfile, however, found out other ones are set continuously into nt_prstatus_percpu[from 0 to found out notes]. I just want to repaire nt_prstatus_percpu[#index] corresponding to cpu#index. [example] cpu#0 cpu#1 cpu#2 cpu#3 saved tainted saved tainted [by looking at base kernel] => nt_prstatus_percpu[0] = cpu#0's note, nt_prstatus_percpu[1] = cpu#3's note I want to repaire array like, => nt_prstatus_percpu[0] = cpu#0's note, nt_prstatus_percpu[3] = cpu#3's note Since ELF note format dose not contain information which cpu saved me, I think there is no way to identify mapping between cpu and nt_prstatus_percpu[] from ELF note when both are not equal to. That's why I try to resolve from base kernel area. I think offline cpus never ack IPI, thus lost notes everytime(not concerned). I only assume the case that cpu was online but could not ack IPI caused by any reasons in interrupts off. I can agree, x86 architecture own NMI which is most reliable way to deliver interrupts to other cpus while powerpc may not own NMI. I am going to rework patch so that it can be effective only in ppc arch. get_diskdump_regs_ppc() is touching dd->nt_prstatus_percpu[bt->tc->processor] when "bt" for panic_task. --------- if (KDUMP_CMPRS_VALID() && (bt->task == tt->panic_task || (is_task_active(bt->task) && (dd->num_prstatus_notes > 1)))) { note = (Elf32_Nhdr*) dd->nt_prstatus_percpu[bt->tc->processor]; if (!note) error(FATAL, "cannot determine NT_PRSTATUS ELF note " "for %s task: %lx\n", (bt->task == tt->panic_task) ? "panic" : "active", bt->task); len = sizeof(Elf32_Nhdr); len = roundup(len + note->n_namesz, 4); bt->machdep = (void *)((char *)note + len + MEMBER_OFFSET("elf_prstatus", "pr_reg")); } machdep->get_stack_frame(bt, eip, esp); --------- In my dumpfile case, bt->tc->processor is 2 and its note is contained at dd->nt_prstatus_percpu[0]. note = (Elf32_Nhdr*) dd->nt_prstatus_percpu[2]; * Here is no note instance which "bt" expects. A dd->nt_prstatus_percpu[] is allocated by malloc() and not initialied with 0. When note->n_namesz is executed, it is cause of segmentation violation. This malloc() is moved to calloc() in my first patch. Other problem which is also appeared in get_netdump_regs_ppc(). A dd->num_prstatus_notes is 1 in my dumpfile although there were 8 online cpus. If next condition "is_task_active(bt->task)" is required by "bt", get_diskdump_regs_ppc() unfortunately do machdep->get_stack_frame() for active tasks and surely failed. "cannot determine NT_PRSTATUS ELF note ..." should be proper. Yes I'm going to rework, and thanks for your reviews. I'm modifying difficult parts this time, very helpful. I'll do more tests before sending reworked PPC only patch. I also have to study about kexectools or makedumpfile more about how NT_PRSTATUS or others are treated, haven't understood enough. Toshi

(2012/06/20 1:20), Dave Anderson wrote: I also bothered about condition whether loop should break at dd->nt_prstatus_percpu or not, even though remaining online cpu's notes can't display as "not saved". However, I choised nrcpus way since my environment could break with online cpu nums, not NR_CPUS. Now, I check kernel_NR_CPUS validation. if (STREQ(ln, "CONFIG_NR_CPUS")) { kt->kernel_NR_CPUS = atoi(val); I'm aware if ikconfig is not valid, break condition of loop is always NR_CPUS without doubt and display gets noisy like as your counsel. Yes, dd->num_prstatus_notes in my dumpfile is 1. No, you are entirely right and I could not condier about x86 parts. I'm getting the feeling that there might be more affects for any other arches. I reconsider different approach to fix NT_PRSTATUS up. Thanks, Toshi