On 08/08/2019 10:36, Juergen Gross wrote: > 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: >>>> >>>> <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. > > 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.
Ok, but clearly there is something which distinguishes domain indices from other indices? > >> >>> >>>> 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. > > lock->profile is set only in lock_prof_init(). Ah - so it is. I wonder if this can be done at compile time? Its just a backreference for the forward reference which is done at. (Wow this code is complicated to follow). I think it can be done with a forward declaration of static struct lock_profile __lock_profile_data_##l and passing something other than NULL into _SPIN_LOCK_UNLOCKED(), but that will break the ability to do static DEFINE_SPINLOCK(). Probably not worth messing with this now, but perhaps something to think over. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
