On Wed, Sep 09, 2009 at 03:11:49PM -0700, Rafael Vanoni wrote:
> webrev @ http://cr.opensolaris.org/~rafaelv/mdb-lbolt/
mdb_ks.c:1598
Isn't this wrong for mdb examining crash dumps - it gives the live
kernel's hrtime, not the dump's? Surely you should be looking for
lbolt_debug_ts in the post-mortem case too. Not sure about a live
dump...
genunix.c:
3441 3440 cid.cid_lbolt = (uintptr_t)sym.st_value;
If we find panic_lbolt, we'll save a pointer to it.
3448 3447 if (lbolt == 0) {
Or if it's zero (no panic), we'll get a pointer to 'lbolt':
3449 - if (mdb_lookup_by_name("lbolt", &sym) == -1) {
3450 - mdb_warn("failed to find lbolt");
3453 - cid.cid_lbolt = (uintptr_t)sym.st_value;
Then, later, we'll use this pointer value:
3213 - if (mdb_vread(&lbolt, sizeof (lbolt),
cid->cid_lbolt) == -1) {
3214 - mdb_warn("failed to read lbolt at %p",
cid->cid_lbolt);
But you made this be:
3213 + if ((lbolt = (clock_t)mdb_get_lbolt()) != -1)
3214 + mdb_printf("t-%-4d ", lbolt -
cpu->cpu_last_swtch);
3215 + else
which will ignore panic_lbolt altogether. I think you need the
mdb_get_lbolt() fix I mention above, and drop cid_lbolt altogether.
At that point, a short comment on the meaning of mdb_get_lbolt() would
be nice.
genunix.c:4348
Please add a help function.
kobj_kdi.c:
The point of the KDI is to isolate KMDB from the kernel - please don't
muddy the interface further. Your calls should be KDI (kernel) side.
There's a (plat) system_claim() call that's the right place. (You'd have
to make the SPARC platform modules call a sparc_system_claim/release(),
which is a slight pain, but much the better way).
regards
john
