netdev - Re: [PATCH net-next v2] net: core: change bool members of struct net

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ded0a778-ca20-4e0f-223d-da3eb5cd71b2@gmail.com>
Date:   Tue, 9 Oct 2018 22:24:58 +0200
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     David Ahern <dsahern@...il.com>, David Miller <davem@...emloft.net>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        David Laight <David.Laight@...LAB.COM>
Subject: Re: [PATCH net-next v2] net: core: change bool members of struct
 net_device to bitfield members

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)?