From mboxrd@z Thu Jan  1 00:00:00 1970
From: Heiner Kallweit <hkallweit1@gmail.com>
Subject: Re: [PATCH net-next v2] net: core: change bool members of struct
 net_device to bitfield members
Date: Tue, 9 Oct 2018 22:24:58 +0200
Message-ID: <ded0a778-ca20-4e0f-223d-da3eb5cd71b2@gmail.com>
References: <e0150624-a317-eb4f-126e-92c99db8ce13@gmail.com>
 <35349fe9-94ac-e2d0-f02c-078c9fd58090@gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        David Laight <David.Laight@ACULAB.COM>
To: David Ahern <dsahern@gmail.com>, David Miller <davem@davemloft.net>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-wr1-f67.google.com ([209.85.221.67]:45442 "EHLO
        mail-wr1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1726563AbeJJDn4 (ORCPT
        <rfc822;netdev@vger.kernel.org>); Tue, 9 Oct 2018 23:43:56 -0400
Received: by mail-wr1-f67.google.com with SMTP id q5-v6so3213739wrw.12
        for <netdev@vger.kernel.org>; Tue, 09 Oct 2018 13:25:15 -0700 (PDT)
In-Reply-To: <35349fe9-94ac-e2d0-f02c-078c9fd58090@gmail.com>
Content-Language: en-US
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

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