From: Florian Fainelli <f.fainelli@gmail•com>
To: Vladimir Oltean <vladimir.oltean@nxp•com>, netdev@vger•kernel.org
Cc: "David S. Miller" <davem@davemloft•net>,
Jakub Kicinski <kuba@kernel•org>, Andrew Lunn <andrew@lunn•ch>,
Vivien Didelot <vivien.didelot@gmail•com>
Subject: Re: [PATCH v2 net-next 4/7] net: dsa: merge all bools of struct dsa_switch into a single u32
Date: Wed, 5 Jan 2022 10:32:48 -0800 [thread overview]
Message-ID: <ac686bdb-470b-07ab-2ef9-3d47fd06e6cd@gmail.com> (raw)
In-Reply-To: <20220105132141.2648876-5-vladimir.oltean@nxp.com>
On 1/5/22 5:21 AM, Vladimir Oltean wrote:
> struct dsa_switch has 9 boolean properties, many of which are in fact
> set by drivers for custom behavior (vlan_filtering_is_global,
> needs_standalone_vlan_filtering, etc etc). The binary layout of the
> structure could be improved. For example, the "bool setup" at the
> beginning introduces a gratuitous 7 byte hole in the first cache line.
>
> The change merges all boolean properties into bitfields of an u32, and
> places that u32 in the first cache line of the structure, since many
> bools are accessed from the data path (untag_bridge_pvid, vlan_filtering,
> vlan_filtering_is_global).
>
> We place this u32 after the existing ds->index, which is also 4 bytes in
> size. As a positive side effect, ds->tagger_data now fits into the first
> cache line too, because 4 bytes are saved.
>
> Before:
>
> pahole -C dsa_switch net/dsa/slave.o
> struct dsa_switch {
> bool setup; /* 0 1 */
>
> /* XXX 7 bytes hole, try to pack */
>
> struct device * dev; /* 8 8 */
> struct dsa_switch_tree * dst; /* 16 8 */
> unsigned int index; /* 24 4 */
>
> /* XXX 4 bytes hole, try to pack */
>
> struct notifier_block nb; /* 32 24 */
>
> /* XXX last struct has 4 bytes of padding */
>
> void * priv; /* 56 8 */
> /* --- cacheline 1 boundary (64 bytes) --- */
> void * tagger_data; /* 64 8 */
> struct dsa_chip_data * cd; /* 72 8 */
> const struct dsa_switch_ops * ops; /* 80 8 */
> u32 phys_mii_mask; /* 88 4 */
>
> /* XXX 4 bytes hole, try to pack */
>
> struct mii_bus * slave_mii_bus; /* 96 8 */
> unsigned int ageing_time_min; /* 104 4 */
> unsigned int ageing_time_max; /* 108 4 */
> struct dsa_8021q_context * tag_8021q_ctx; /* 112 8 */
> struct devlink * devlink; /* 120 8 */
> /* --- cacheline 2 boundary (128 bytes) --- */
> unsigned int num_tx_queues; /* 128 4 */
> bool vlan_filtering_is_global; /* 132 1 */
> bool needs_standalone_vlan_filtering; /* 133 1 */
> bool configure_vlan_while_not_filtering; /* 134 1 */
> bool untag_bridge_pvid; /* 135 1 */
> bool assisted_learning_on_cpu_port; /* 136 1 */
> bool vlan_filtering; /* 137 1 */
> bool pcs_poll; /* 138 1 */
> bool mtu_enforcement_ingress; /* 139 1 */
> unsigned int num_lag_ids; /* 140 4 */
> unsigned int max_num_bridges; /* 144 4 */
>
> /* XXX 4 bytes hole, try to pack */
>
> size_t num_ports; /* 152 8 */
>
> /* size: 160, cachelines: 3, members: 27 */
> /* sum members: 141, holes: 4, sum holes: 19 */
> /* paddings: 1, sum paddings: 4 */
> /* last cacheline: 32 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 */
>
> /* 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 */
> };
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp•com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail•com>
--
Florian
next prev parent reply other threads:[~2022-01-05 18:33 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-05 13:21 [PATCH v2 net-next 0/7] Cleanup to main DSA structures Vladimir Oltean
2022-01-05 13:21 ` [PATCH v2 net-next 1/7] net: dsa: move dsa_port :: stp_state near dsa_port :: mac Vladimir Oltean
2022-01-05 18:30 ` Florian Fainelli
2022-01-05 13:21 ` [PATCH v2 net-next 2/7] net: dsa: merge all bools of struct dsa_port into a single u8 Vladimir Oltean
2022-01-05 18:30 ` Florian Fainelli
2022-01-05 18:39 ` Vladimir Oltean
2022-01-05 18:46 ` Florian Fainelli
2022-01-05 18:56 ` Vladimir Oltean
2022-01-05 19:42 ` Florian Fainelli
2022-01-05 22:10 ` Andrew Lunn
2022-01-05 13:21 ` [PATCH v2 net-next 3/7] net: dsa: move dsa_port :: type near dsa_port :: index Vladimir Oltean
2022-01-05 18:31 ` Florian Fainelli
2022-01-05 13:21 ` [PATCH v2 net-next 4/7] net: dsa: merge all bools of struct dsa_switch into a single u32 Vladimir Oltean
2022-01-05 18:32 ` Florian Fainelli [this message]
2022-01-05 13:21 ` [PATCH v2 net-next 5/7] net: dsa: make dsa_switch :: num_ports an unsigned int Vladimir Oltean
2022-01-05 18:33 ` Florian Fainelli
2022-01-05 13:21 ` [PATCH v2 net-next 6/7] net: dsa: move dsa_switch_tree :: ports and lags to first cache line Vladimir Oltean
2022-01-05 18:34 ` Florian Fainelli
2022-01-05 13:21 ` [PATCH v2 net-next 7/7] net: dsa: combine two holes in struct dsa_switch_tree Vladimir Oltean
2022-01-05 18:34 ` Florian Fainelli
2022-01-05 14:28 ` [PATCH v2 net-next 0/7] Cleanup to main DSA structures Vladimir Oltean
2022-01-05 18:37 ` Florian Fainelli
2022-01-05 18:39 ` Florian Fainelli
2022-01-05 18:59 ` Vladimir Oltean
2022-01-05 19:04 ` Florian Fainelli
2022-01-05 19:22 ` Vladimir Oltean
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ac686bdb-470b-07ab-2ef9-3d47fd06e6cd@gmail.com \
--to=f.fainelli@gmail$(echo .)com \
--cc=andrew@lunn$(echo .)ch \
--cc=davem@davemloft$(echo .)net \
--cc=kuba@kernel$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=vivien.didelot@gmail$(echo .)com \
--cc=vladimir.oltean@nxp$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox