[Crash-utility] [PATCH v2] Add read diagnostics to crash

[David Mair]

Hi Dave,

On 01/11/2012 01:50 PM, Dave Anderson wrote:
>
>
> ----- Original Message -----
>>
>> ----- Original Message -----
>>
>>>> But there is still no need to redundantly display the virtual and physical
>>>> addresses.  And the displays of the calculated file offsets, zero-fill, etc.
>>>> will still end up showing the complete function sequence by putting the
>>>> function name in the output.  For example, the updated readmem() output would
>>>> show a call to read_kdump(), but the file offset display would show that it
>>>> has transitioned to read_netdump(), so there's no need for any debug output
>>>> at all in read_kdump().
>>>
>>>
>>> Well, read_kdump() in the case of a non-hyper-mode XEN dump has code
>>> that has the appearance of a route of change for paddr, i.e. the
>>> following doesn't result in no change or in P2M_FAILURE:
>>>
>>> paddr = xen_kdump_p2m(paddr)
>>>
>>> The code I posted can show two unique paths through read_kdump() but if
>>> as you say, you log calling it with the physical address known and log
>>> in read_netdump() with the physical address included then you get the
>>> same result.
>>
>> No, you're right -- that particular "switch" of the paddr value
>> should probably have its own debug statement.
>>
>>> Also, are there any cases of overlapped/threaded execution of reads?
>>> If  not then removal of redundant output is wise but the virt/phys addr
>>> would identify which thread of execution each line refers to among
>>> overlapped output in most cases.
>>
>> Well, at least in the Xen case, yes it is possible.  But they would be
>> encapsulated by the debug statements that indicate the "temporary" change
>> from read_kdump() to read_netdump() in x86.c, x86_64.c (and ia64.c).
>>
>> Dave
>
>
> Hi Dave,
>
> I've attached what I'm going with for now.  It covers all bases that
> your original patch did, and then some.  More specifically I added
> some additional debug statements to xen_kdump_p2m() in order
> to clarify the xen kdump read path through read_kdump(), because
> xen_kdump_p2m() calls read_netdump() directly to get a MFN frame
> before returning to read_kdump() to complete the original read
> via read_netdump().  And so because of that twisty path xen kdump
> reads do take, I left the addr/paddr/cnt display in read_netdump()
> to allay any confusion.  The diskdump path is always straight-forward,
> so the debug statements there only show the paddr/pfn pair, since
> that's all it ever deals with, and the readmem()-generated statement
> just above them would give you all the rest of the arguments.  I didn't
> make individual debug statements in read_netdump() w/respect to whether
> the offset came from a 32-bit ELF vmcore, or from a single or multiple
> PT_LOAD 64-bit ELF vmcore, because you get that information immediately
> with "-d1" on the invocation command line, or from "help -n" during runtime.
> I added a few simple statements in ia64.c, but this patch is primarily
> concerned with x86/x86_64.  The readmem-assignment display is done
> in one place, using a new readmem_function_name() function, which is
> also used in readmem() and dump_program_context().  Everything is still
> CRASHDEBUG(8) or less.  So I'm hoping that you'll be happy with the
> modifications to your original, quite useful, proposal.

I agree your version is improved over my original idea so I'm certainly 
happy using your approach. Thanks for putting the time into it.

-- 
David Mair
SUSE Linux