[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 0/4] enhance lock debugging

On 08.08.19 10:33, Andrew Cooper wrote:
On 08/08/2019 05:50, Juergen Gross wrote:
On 07.08.19 20:11, Andrew Cooper wrote:

Its not exactly the easiest to dump to follow.

First of all - I don't see why the hold/block time are printed like
that.  It
might be a holdover from the 32bit build, pre PRId64 support.  They
probably use PRI_stime anyway.

Fine with me.

The domid rendering is unfortunate.  Ideally we'd use %pd but that would
involve rearranging the logic to get a struct domain* in hand.
Seeing as
you're the last person to modify this code, how hard would that be to

It would completely break the struct type agnostic design.

Ok.  As an alternatively, how about %pdr which takes a raw domid?  It
would be a trivial adjustment in the vsnprintf code, and could plausibly
be useful elsewhere where we have a domid and not a domain pointer.

Lock profiling has no knowledge that it is working on a struct domain.
It is just working on a lock in a struct and an index to differentiate
the struct instance. Using the domid as the index is just for making it
more easy to understand the printout.

I wouldn't want e.g. a per-event lock to be named "IDLE" just because
it happens to be index 32767.

I have a debug patch adding credit2 run-queue lock. It requires to just
add 5 lines of code (and this includes moving the spinlock_init() out of
an irq-off section). It will produce:

(XEN) credit2-runq 0 lock: addr=ffff830413351010, lockval=de00ddf, cpu=6
(XEN)   lock: 896287(00000000:00FAA6B2), block:  819(00000000:00009C80)

What is the 0 here?

The runq number.

We have several global locks called lock:

    (XEN) Global lock: addr=ffff82d0808181e0, lockval=10001, cpu=4095
    (XEN)   lock:           1(00000000:01322165), block:
    (XEN) Global lock: addr=ffff82d080817cc0, lockval=100010, cpu=4095
    (XEN)   lock:           0(00000000:00000000), block:
    (XEN) Global lock: addr=ffff82d080817800, lockval=0000, cpu=4095
    (XEN)   lock:           0(00000000:00000000), block:
    (XEN) Global lock: addr=ffff82d080817780, lockval=0000, cpu=4095
    (XEN)   lock:           0(00000000:00000000), block:
    (XEN) Global lock: addr=ffff82d080817510, lockval=0000, cpu=4095
    (XEN)   lock:           0(00000000:00000000), block:

The second one in particular has corrupt data, seeing has it has been
and released several times without lock_cnt increasing.

The lock might have been taken/released before lock profiling has been

What is there to initialise?  It all looks statically set up.

lock->profile is set only in lock_prof_init().

For sanity sake, we should enforce unique naming of any lock
registered for

This would be every lock inited via DEFINE_SPINLOCK(). I can do a
followup patch for that purpose.

I was wondering how to do this.  One option which comes to mind is to
put an non-static object into .discard.lock_profile or something, so the
linker will object to repeated symbol names and then throw all of them away.

I could just drop the "static" in the _LOCK_PROFILE_PTR() macro.
At the same time I should move the ".lockprofile.data" section in the
linker scripts to the init part maybe.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.