Re: [Xen-devel] [Patch] support of lock profiling in Xen

[Keir Fraser <keir.fraser@xxxxxxxxxxxxx>]

On 09/10/2009 09:27, "Juergen Gross" <juergen.gross@xxxxxxxxxxxxxx> wrote:

>> Okay, well this all sounds an acceptable kind of direction. Worth spinning
>> another patch.
> 
> Here it is.
> I managed to avoid the perfc-like stuff completely.
> 
> Included are 2 global locks defined for profiling and 2 locks in the domain
> structure.
> 
> I've added some comments in spinlock.h how to use the profiling in structures.
> 
> Tested by compilation and boot of Dom0 with and without profiling enabled.

Not keen on exposing the lock_profile_funcs[] stuff; that and a few other
aspects are ratehr clumsy looking. I would rather dynamically
register/unregister lock-profile blocks; something like:
   /* domain creation */
   lock_profile_register_struct(
       LOCKPROF_TYPE_PERDOMAIN, d, d->domid);
   /* domain destruction */
   lock_profile_unregister_struct(d);
Requires a 'struct lock_profile lock_profile' or similar to be declared
within the domain structure. No need to hide that definition within a macro
I expect, nor to specify in advance the 'max # locks'.

Where lock_profile is a struct containing the 'idx' (domid in this case) and
the head of a linked list of locks, which gets appended to whenever a new
pofiled lock is declared. All locks can go back to having the entire profile
struct embedded within them - I don't care about the space wastage for
unprofiled locks. This means that lock_profile is a fixed-size struct and
does not need to contain the profiling data itself -- it just points at a
chain of locks that extends on each spin_lock_init_prof().

lock_profile_register_struct() adds the lock_profile_struct to a linked list
of such structs for this LOCKPROF_TYPE (so you keep an array of list_heads
indexed by LOCKPROF_TYPE, and you can protect that array and all
sub-linked-lists with a big rwlock). Something like:
static DEFINE_RWLOCK lock_profiles_lock;
static struct lock_profile *lock_profiles[NR_LOCKPROF_TYPES]
Or somesuch.

Regarding macro style, crap like starting a multi-statement macro with a
semi-colon, and not terminating it with one (e.g., _spin_lock_init_prof())
is unacceptable style. The C idiom is to enclose multiple statements within
a do{}while(0), or possibly the gcc idiom {()}.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel