[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e77ee553-732a-87ab-dcd9-ae1ae283e0c8@gmail.com>
Date: Wed, 5 Jan 2022 10:33:25 -0800
From: Florian Fainelli <f.fainelli@...il.com>
To: Vladimir Oltean <vladimir.oltean@....com>, netdev@...r.kernel.org
Cc: "David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>, Andrew Lunn <andrew@...n.ch>,
Vivien Didelot <vivien.didelot@...il.com>
Subject: Re: [PATCH v2 net-next 5/7] net: dsa: make dsa_switch :: num_ports an
unsigned int
On 1/5/22 5:21 AM, Vladimir Oltean wrote:
> Currently, num_ports is declared as size_t, which is defined as
> __kernel_ulong_t, therefore it occupies 8 bytes of memory.
>
> Even switches with port numbers in the range of tens are exotic, so
> there is no need for this amount of storage.
>
> Additionally, because the max_num_bridges member right above it is also
> 4 bytes, it means the compiler needs to add padding between the last 2
> fields. By reducing the size, we don't need that padding and can reduce
> the struct size.
>
> Before:
>
> pahole -C dsa_switch net/dsa/slave.o
> struct dsa_switch {
> struct device * dev; /* 0 8 */
> struct dsa_switch_tree * dst; /* 8 8 */
> unsigned int index; /* 16 4 */
> u32 setup:1; /* 20: 0 4 */
> u32 vlan_filtering_is_global:1; /* 20: 1 4 */
> u32 needs_standalone_vlan_filtering:1; /* 20: 2 4 */
> u32 configure_vlan_while_not_filtering:1; /* 20: 3 4 */
> u32 untag_bridge_pvid:1; /* 20: 4 4 */
> u32 assisted_learning_on_cpu_port:1; /* 20: 5 4 */
> u32 vlan_filtering:1; /* 20: 6 4 */
> u32 pcs_poll:1; /* 20: 7 4 */
> u32 mtu_enforcement_ingress:1; /* 20: 8 4 */
>
> /* XXX 23 bits hole, try to pack */
>
> struct notifier_block nb; /* 24 24 */
>
> /* XXX last struct has 4 bytes of padding */
>
> void * priv; /* 48 8 */
> void * tagger_data; /* 56 8 */
> /* --- cacheline 1 boundary (64 bytes) --- */
> struct dsa_chip_data * cd; /* 64 8 */
> const struct dsa_switch_ops * ops; /* 72 8 */
> u32 phys_mii_mask; /* 80 4 */
>
> /* XXX 4 bytes hole, try to pack */
>
> struct mii_bus * slave_mii_bus; /* 88 8 */
> unsigned int ageing_time_min; /* 96 4 */
> unsigned int ageing_time_max; /* 100 4 */
> struct dsa_8021q_context * tag_8021q_ctx; /* 104 8 */
> struct devlink * devlink; /* 112 8 */
> unsigned int num_tx_queues; /* 120 4 */
> unsigned int num_lag_ids; /* 124 4 */
> /* --- cacheline 2 boundary (128 bytes) --- */
> unsigned int max_num_bridges; /* 128 4 */
>
> /* XXX 4 bytes hole, try to pack */
>
> size_t num_ports; /* 136 8 */
>
> /* size: 144, cachelines: 3, members: 27 */
> /* sum members: 132, holes: 2, sum holes: 8 */
> /* sum bitfield members: 9 bits, bit holes: 1, sum bit holes: 23 bits */
> /* paddings: 1, sum paddings: 4 */
> /* last cacheline: 16 bytes */
> };
>
> After:
>
> pahole -C dsa_switch net/dsa/slave.o
> struct dsa_switch {
> struct device * dev; /* 0 8 */
> struct dsa_switch_tree * dst; /* 8 8 */
> unsigned int index; /* 16 4 */
> u32 setup:1; /* 20: 0 4 */
> u32 vlan_filtering_is_global:1; /* 20: 1 4 */
> u32 needs_standalone_vlan_filtering:1; /* 20: 2 4 */
> u32 configure_vlan_while_not_filtering:1; /* 20: 3 4 */
> u32 untag_bridge_pvid:1; /* 20: 4 4 */
> u32 assisted_learning_on_cpu_port:1; /* 20: 5 4 */
> u32 vlan_filtering:1; /* 20: 6 4 */
> u32 pcs_poll:1; /* 20: 7 4 */
> u32 mtu_enforcement_ingress:1; /* 20: 8 4 */
>
> /* XXX 23 bits hole, try to pack */
>
> struct notifier_block nb; /* 24 24 */
>
> /* XXX last struct has 4 bytes of padding */
>
> void * priv; /* 48 8 */
> void * tagger_data; /* 56 8 */
> /* --- cacheline 1 boundary (64 bytes) --- */
> struct dsa_chip_data * cd; /* 64 8 */
> const struct dsa_switch_ops * ops; /* 72 8 */
> u32 phys_mii_mask; /* 80 4 */
>
> /* XXX 4 bytes hole, try to pack */
>
> struct mii_bus * slave_mii_bus; /* 88 8 */
> unsigned int ageing_time_min; /* 96 4 */
> unsigned int ageing_time_max; /* 100 4 */
> struct dsa_8021q_context * tag_8021q_ctx; /* 104 8 */
> struct devlink * devlink; /* 112 8 */
> unsigned int num_tx_queues; /* 120 4 */
> unsigned int num_lag_ids; /* 124 4 */
> /* --- cacheline 2 boundary (128 bytes) --- */
> unsigned int max_num_bridges; /* 128 4 */
> unsigned int num_ports; /* 132 4 */
>
> /* size: 136, cachelines: 3, members: 27 */
> /* sum members: 128, holes: 1, sum holes: 4 */
> /* sum bitfield members: 9 bits, bit holes: 1, sum bit holes: 23 bits */
> /* paddings: 1, sum paddings: 4 */
> /* last cacheline: 8 bytes */
> };
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
Reviewed-by: Florian Fainelli <f.fainelli@...il.com>
--
Florian
Powered by blists - more mailing lists