Re: [PATCHv3 3/4] xen: use ticket locks for spin locks

[David Vrabel <david.vrabel@citrix.com>]

On 30/04/15 11:09, Tim Deegan wrote:
> At 00:56 +0100 on 30 Apr (1430355366), Jan Beulich wrote:
>>>>> David Vrabel <david.vrabel@citrix.com> 04/29/15 5:28 PM >>>
>>> On 29/04/15 00:15, Jan Beulich wrote:
>>>>>>> David Vrabel <david.vrabel@citrix.com> 04/28/15 6:16 PM >>>
>>>>> Are there any structures whose size you're particularly concerned about?
>>>>
>>>> No specific ones (but of course structures with an inherent size constraint
>>>> - like struct domain and struct vcpu - are, with all of their sub-structures,
>>>> primary candidates). I just recall that in some cases (and this may no longer
>>>> apply due to later additions) structures got arranged specifically taking in
>>>> mind the 2-byte size of the locks, and the growth here may thus mean a
>>>> structure size growth of more than just two bytes.
>>>
>>> Spin locks are currently 4 bytes (2 bytes for the lock, plus 2 bytes for
>>> the recurse_{cnt,cpu}), and ticket locks are now 8 bytes (an increase in
>>> 4 bytes).
>>>
>>> struct vcpu does not increase in size (there is 56, now 48 bytes, of
>>> padding before the arch field).
>>>
>>> struct domain increases from 3480 to 3544 bytes.
>>
>> Odd - this suggests there's still some 64-byte alignment somewhere, but I
>> can't immediately spot it. Only struct vcpu has an obvious 64-byte aligned
>> field (struct pi_desc) afaics.
> 
> I got slightly different numbers to David (I suspect just starting
> from a different baseline) but the biggest hole came from struct
> arch_domain being __cacheline_aligned (i.e. 128-byte aligned).
> 
> There are some holes above it in struct domain which should pack in
> nicely to counteract the larger spinlocks, e.g. this was enough to
> naturally align arch_domain again in my tree:
> 
> @@ -334,9 +334,9 @@ struct domain
>      spinlock_t       rangesets_lock;
>  
>      /* Event channel information. */
> +    unsigned int     max_evtchns;
>      struct evtchn   *evtchn;                         /* first bucket only */
>      struct evtchn  **evtchn_group[NR_EVTCHN_GROUPS]; /* all other buckets */
> -    unsigned int     max_evtchns;
>      unsigned int     max_evtchn_port;
>      spinlock_t       event_lock;
>      const struct evtchn_port_ops *evtchn_port_ops;

This change makes struct domain 8 bytes bigger with ticket locks.

On x86 there are 12 spinlocks in struct domain which is 48 bytes extra.
 I've clawed back 16 bytes by repacking struct paging_domain and struct
hvm_domain.

David