[Crash-utility] Handle the NT_PRSTATUS lost for the "bt" command

[Toshikazu Nakayama]

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 at redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-update-NT_PRSTATUS-issue-fixed-up-code.patch
Type: text/x-patch
Size: 4236 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/crash-utility/attachments/20120619/5024cfef/attachment.bin>