Re: [PATCH net-next v2] net: core: change bool members of struct net_device to bitfield members

[Heiner Kallweit <hkallweit1@gmail.com>]

On 09.10.2018 17:20, David Ahern wrote:
> On 10/8/18 2:17 PM, Heiner Kallweit wrote:
>> bool is good as parameter type or function return type, but if used
>> for struct members it consumes more memory than needed.
>> Changing the bool members of struct net_device to bitfield members
>> allows to decrease the memory footprint of this struct.
> 
> What does pahole show for the size of the struct before and after? I
> suspect you have not really changed the size and certainly not the
> actual memory allocated.
> 
> 
Thanks for the hint to use pahole. Indeed we gain nothing,
so there's no justification for this patch.

before:
        /* size: 2496, cachelines: 39, members: 116 */
        /* sum members: 2396, holes: 8, sum holes: 80 */
        /* padding: 20 */
        /* paddings: 4, sum paddings: 19 */
        /* bit_padding: 31 bits */

after:	
        /* size: 2496, cachelines: 39, members: 116 */
        /* sum members: 2394, holes: 8, sum holes: 82 */
        /* bit holes: 1, sum bit holes: 8 bits */
        /* padding: 20 */
        /* paddings: 4, sum paddings: 19 */
        /* bit_padding: 27 bits */

The biggest hole is here, because _tx is annotated to be cacheline-aligned.

        struct hlist_node          index_hlist;          /*   888    16 */

        /* XXX 56 bytes hole, try to pack */

        /* --- cacheline 15 boundary (960 bytes) --- */
        struct netdev_queue *      _tx;                  /*   960     8 */

Reordering the struct members to fill the holes could be a little tricky
and could have side effects because it may make a performance difference
whether certain members are in one cacheline or not.
And whether it's worth to spend this effort (incl. the related risks)
just to save a few bytes (also considering that typically we have quite
few instances of struct net_device)?