On 08/08/2019 05:50, Juergen Gross wrote: > On 07.08.19 20:11, Andrew Cooper wrote: >> >> <snip> >> 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 >> should >> 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 >> do? > > 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. > > 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? > >> We have several global locks called lock: >> >> (XEN) Global lock: addr=ffff82d0808181e0, lockval=10001, cpu=4095 >> (XEN) lock: 1(00000000:01322165), block: >> 0(00000000:00000000) >> (XEN) Global lock: addr=ffff82d080817cc0, lockval=100010, cpu=4095 >> (XEN) lock: 0(00000000:00000000), block: >> 0(00000000:00000000) >> (XEN) Global lock: addr=ffff82d080817800, lockval=0000, cpu=4095 >> (XEN) lock: 0(00000000:00000000), block: >> 0(00000000:00000000) >> (XEN) Global lock: addr=ffff82d080817780, lockval=0000, cpu=4095 >> (XEN) lock: 0(00000000:00000000), block: >> 0(00000000:00000000) >> (XEN) Global lock: addr=ffff82d080817510, lockval=0000, cpu=4095 >> (XEN) lock: 0(00000000:00000000), block: >> 0(00000000:00000000) >> >> The second one in particular has corrupt data, seeing has it has been >> taken >> and released several times without lock_cnt increasing. > > The lock might have been taken/released before lock profiling has been > initialized. What is there to initialise? It all looks statically set up. > >> For sanity sake, we should enforce unique naming of any lock >> registered for >> profiling. > > 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. ~Andrew _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
