From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: [Patch] support of lock profiling in Xen Date: Thu, 08 Oct 2009 10:27:38 +0200 Message-ID: <4ACDA27A.80603@ts.fujitsu.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Keir Fraser Cc: "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org Keir Fraser wrote: > On 08/10/2009 08:48, "Juergen Gross" wrote: > >> I'm not completely satisfied with the solution for the dynamically initialized >> locks, but I had no better idea in the first run. >> Another enhancement would be to expand the profiling to rw-locks as well, but >> this would have required a rewrite of the lock routines using try_lock like >> for spinlocks. I could do this if the lock profiling is accepted. >> >> Comments welcome :-) > > The method of chaining and initialising the info is kind of icky. Requiring > users to unchain dynamic locks is just asking for this support to be > perpetually broken, or be used only for static locks. > > How about defining new initialisers DEFINE_NAMED_SPINLOCK() and > named_spinlock_init(). This would indicate you consider a lock important > enough to name (and hence profile) and also categorise dynamically-allocated > locks, causing their stats to be aggregated (after all, lock optimisations > will have aggregate effect across all locks of that category). > > If lock profiling is compiled in, have a static array of lock-profile > descriptors (keeps things simple - could make it a growable array, or > something). On lock init, walk the array looking for that name. If found, > write the entry index into a new field in the spinlock struct. If not found, > allocate next entry in lock-profile array, init it, and write its index into > the spinlock struct. Don't you think a solution like for performance counters would be better then? The index would be in a header file and the sums could be held per-cpu to avoid races. It would even be possible to define lock arrays if summing up all data for e.g. domain specific locks is not desired. > > On lock operations, if the index field in the spinlock is non-zero, update > stats in the associated profile structure. Regarding races from locks > aliasing to the same profile structure -- either assume that doesn't matter > much, or perhaps update fields with atomic ops. Juergen -- Juergen Gross Principal Developer Operating Systems TSP ES&S SWE OS6 Telephone: +49 (0) 89 636 47950 Fujitsu Technolgy Solutions e-mail: juergen.gross@ts.fujitsu.com Otto-Hahn-Ring 6 Internet: ts.fujitsu.com D-81739 Muenchen Company details: ts.fujitsu.com/imprint.html