Hi Dave,
Thanks for your comments.
(2012/06/19 1:01), Dave Anderson wrote:
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?
I see, I'll make consideration about initialization-time rule from now,
to keep crash session, should use readmem() with QUIET|RETURN_ON_ERROR.
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.
I'm not able to test for unexpected, deliberate readmem() failure
because my core file contains "crash_notes" memory field...
Although I've probably misunderstood your advice,
I had better add more debugging messages into
map_cpus_to_prstatus_kdump_cmprs() to handle conditions
where imply "not saved NT_PRSTATUS" or "nt_prstatus_percpu[] is
remapped".
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.
I re-word all "lost" keywords, so too function name with "saved".
And also make this warning valid only when CRASHDEBUG() is enable
because we can check it later by using "help -D".
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)
Looks better, thanks for giving good hint.
[updates by attached patch]
- display messages only if CRASHDEBUG() >= 1
crash -d 1
=> display about NT_PRSTATUS mapping messages as below.
----------------
WARNING: cpu#0: not saved crash_notes
WARNING: cpu#1: not saved crash_notes
crash: NT_PRSTATUS: cpu#2 = 107a34a8 (remapped from cpu#0)
WARNING: cpu#3: not saved crash_notes
WARNING: cpu#4: not saved crash_notes
WARNING: cpu#5: not saved crash_notes
WARNING: cpu#6: not saved crash_notes
WARNING: cpu#7: not saved crash_notes
----------------
- help message is enhanced
crash> help -D | grep notes
num_prstatus_notes: 1
notes_buf: 107a34a8
notes[0]: (not saved)
notes[1]: (not saved)
notes[2]: 107a34a8
notes[3]: (not saved)
notes[4]: (not saved)
notes[5]: (not saved)
notes[6]: (not saved)
notes[7]: (not saved)
Thanks,
Toshi
Thanks,
Dave
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility