[ovs-dev] [PATCH] ovs-thread: Avoid huge alignment on a base spinlock structure.

[Ilya Maximets]

On 16.12.2019 19:21, William Tu wrote:
> On Mon, Dec 16, 2019 at 03:22:00PM +0100, Ilya Maximets wrote:
>> Marking the structure as 64 bytes aligned forces compiler to produce
>> big holes in the containing structures in order to fulfill this
>> requirement.  Also, any structure that contains this one as a member
>> automatically inherits this huge alignment making resulted memory
>> layout not efficient.  For example, 'struct umem_pool' currently
>> uses 3 full cache lines (192 bytes) with only 32 bytes of actual data:
>>
>>   struct umem_pool {
>>     int                        index;                /*  0   4 */
>>     unsigned int               size;                 /*  4   4 */
>>
>>     /* XXX 56 bytes hole, try to pack */
>>
>>     /* --- cacheline 1 boundary (64 bytes) --- */
>>     struct ovs_spin lock __attribute__((__aligned__(64))); /* 64  64 */
>>
>>     /* XXX last struct has 48 bytes of padding */
>>
>>     /* --- cacheline 2 boundary (128 bytes) --- */
>>     void * *                   array;                /* 128  8 */
>>
>>     /* size: 192, cachelines: 3, members: 4 */
>>     /* sum members: 80, holes: 1, sum holes: 56 */
>>     /* padding: 56 */
>>     /* paddings: 1, sum paddings: 48 */
>>     /* forced alignments: 1, forced holes: 1, sum forced holes: 56 */
>>   } __attribute__((__aligned__(64)));
>>
>> Actual alignment of a spin lock is required only for Tx queue locks
>> inside netdev-afxdp to avoid false sharing, in all other cases
>> alignment only produces inefficient memory usage.
> 
> yes, now I realize umem->lock does not have the false
> sharing problem.
> 
>>
>> Also, CACHE_LINE_SIZE macro should be used instead of 64 as different
>> platforms may have different cache line sizes.
>>
>> Using PADDED_MEMBERS to avoid alignment inheritance.
>>
>> CC: William Tu <u9012063 at gmail.com>
>> Fixes: ae36d63d7e3c ("ovs-thread: Make struct spin lock cache aligned.")
>> Signed-off-by: Ilya Maximets <i.maximets at ovn.org>
>> ---
> 
> Somehow my pahole stops working, but the patch looks
> good to me, and thanks for also fixing the include ordering.

pahole is broken on Ubuntu 18.04 at least.  Had to copy binary with
dependencies from the fedora package.

> 
> Acked-by: William Tu <u9012063 at gmail.com>

Thanks! Applied to master.

Best regards, Ilya Maximets.