Re: [Crash-utility] [PATCH] Add -r option to timer
----- Original Message -----
Hello Dave,
The problem is fixed and I changed the display according to your
suggestion. Please check.
I'm going to test it over the weekend on my sample set of vmcores
just to make sure the failure protection is still working.
I'm still not particularly happy about the display. For example, with
the older style without hrtimer_cpu_base addresses, I think it's
unnecessary to display the HRTIMER_BASES[] address next to each cpu
since the each address in the array is shown for each clock. Plus
the "ACTIVE TIMERS:" line is pretty much unnecessary, and perhaps
the OFFSET and GET_TIME fields can be put on one line.
Let me think some more about how it can be made more useful/clear.
One more thing is about current time. I think current time is needed
when debugging, for debuggers can compare the expires of hrtimers with
current time.
When realizing this functionality, I once tried to use xtime plus
wall_to_monotonic. But then I found the value is not that correct. So I
use uptime here. Since it seems strange, what about changing the
display?
If it's not readily obvious what it's showing, and how it relates to
the expire data, then I would leave it out.
One other suggestion...
I probably should have been a bit more clear about using
accessible() to prevent the command from aborting prematurely.
Your fixes do accomplish that, but, for example, while this
usage of accessible() makes perfect sense in your print_timer()
function:
+ if (!accessible((ulong)timer)) {
+ fprintf(fp, "(destroyed timer)\n");
+ return;
+ }
later on in the function, this usage is essentially redundant with
the subsequent readmem():
+ if (accessible((ulong)(timer + OFFSET(hrtimer_function)))) {
+ readmem((ulong)(timer + OFFSET(hrtimer_function)), KVADDR, &function,
+ sizeof(function), "hrtimer function", FAULT_ON_ERROR);
+ fprintf(fp, "%lx ", function);
+ fprintf(fp ,"<%s>", value_to_symstr(function, buf3, 0));
+ }
It can be done with one call by using RETURN_ON_ERROR|QUIET and
checking the return value:
if (readmem((ulong)(timer + OFFSET(hrtimer_function)), KVADDR, &function,
sizeof(function), "hrtimer function", RETURN_ON_ERROR|QUIET)) {
fprintf(fp, "%lx ", function);
fprintf(fp ,"<%s>", value_to_symstr(function, buf3, 0));
}
Thanks,
Dave