diff mbox series

[net-next] netfilter: Reorder fields in 'struct nf_conntrack_expect'

Message ID 5cdb1f50f2e9dc80dbf86cf8667056eacfd36a09.1683564754.git.christophe.jaillet@wanadoo.fr
State New
Headers
Series [net-next] netfilter: Reorder fields in 'struct nf_conntrack_expect' |

Commit Message

Christophe JAILLET May 8, 2023, 4:53 p.m. UTC 
  Group some variables based on their sizes to reduce holes.
On x86_64, this shrinks the size of 'struct nf_conntrack_expect' from 264
to 256 bytes.

This structure deserve a dedicated cache, so reducing its size looks nice.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Using pahole

Before:
======
struct nf_conntrack_expect {
	struct hlist_node          lnode;                /*     0    16 */
	struct hlist_node          hnode;                /*    16    16 */
	struct nf_conntrack_tuple  tuple;                /*    32    40 */
	/* --- cacheline 1 boundary (64 bytes) was 8 bytes ago --- */
	struct nf_conntrack_tuple_mask mask;             /*    72    20 */

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

	void                       (*expectfn)(struct nf_conn *, struct nf_conntrack_expect *); /*    96     8 */
	struct nf_conntrack_helper * helper;             /*   104     8 */
	struct nf_conn *           master;               /*   112     8 */
	struct timer_list          timeout;              /*   120    88 */
	/* --- cacheline 3 boundary (192 bytes) was 16 bytes ago --- */
	refcount_t                 use;                  /*   208     4 */
	unsigned int               flags;                /*   212     4 */
	unsigned int               class;                /*   216     4 */
	union nf_inet_addr         saved_addr;           /*   220    16 */
	union nf_conntrack_man_proto saved_proto;        /*   236     2 */

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

	enum ip_conntrack_dir      dir;                  /*   240     4 */

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

	struct callback_head       rcu __attribute__((__aligned__(8))); /*   248    16 */

	/* size: 264, cachelines: 5, members: 15 */
	/* sum members: 254, holes: 3, sum holes: 10 */
	/* forced alignments: 1, forced holes: 1, sum forced holes: 4 */
	/* last cacheline: 8 bytes */
} __attribute__((__aligned__(8)));


After:
=====
struct nf_conntrack_expect {
	struct hlist_node          lnode;                /*     0    16 */
	struct hlist_node          hnode;                /*    16    16 */
	struct nf_conntrack_tuple  tuple;                /*    32    40 */
	/* --- cacheline 1 boundary (64 bytes) was 8 bytes ago --- */
	struct nf_conntrack_tuple_mask mask;             /*    72    20 */
	refcount_t                 use;                  /*    92     4 */
	unsigned int               flags;                /*    96     4 */
	unsigned int               class;                /*   100     4 */
	void                       (*expectfn)(struct nf_conn *, struct nf_conntrack_expect *); /*   104     8 */
	struct nf_conntrack_helper * helper;             /*   112     8 */
	struct nf_conn *           master;               /*   120     8 */
	/* --- cacheline 2 boundary (128 bytes) --- */
	struct timer_list          timeout;              /*   128    88 */
	/* --- cacheline 3 boundary (192 bytes) was 24 bytes ago --- */
	union nf_inet_addr         saved_addr;           /*   216    16 */
	union nf_conntrack_man_proto saved_proto;        /*   232     2 */

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

	enum ip_conntrack_dir      dir;                  /*   236     4 */
	struct callback_head       rcu __attribute__((__aligned__(8))); /*   240    16 */

	/* size: 256, cachelines: 4, members: 15 */
	/* sum members: 254, holes: 1, sum holes: 2 */
	/* forced alignments: 1 */
} __attribute__((__aligned__(8)));
---
 include/net/netfilter/nf_conntrack_expect.h | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Comments

Simon Horman May 11, 2023, 9:24 a.m. UTC | #1 
On Mon, May 08, 2023 at 06:53:14PM +0200, Christophe JAILLET wrote:
> Group some variables based on their sizes to reduce holes.
> On x86_64, this shrinks the size of 'struct nf_conntrack_expect' from 264
> to 256 bytes.
> 
> This structure deserve a dedicated cache, so reducing its size looks nice.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>

Thanks, this much nicer to me.

I slightly concerned that there may be implications for
fields that were on the same cacheline now being on different cachelines.
But only very slightly.

Reviewed-by: Simon Horman <simon.horman@corigine.com>

> ---
> Using pahole
> 
> Before:
> ======
> struct nf_conntrack_expect {
> 	struct hlist_node          lnode;                /*     0    16 */
> 	struct hlist_node          hnode;                /*    16    16 */
> 	struct nf_conntrack_tuple  tuple;                /*    32    40 */
> 	/* --- cacheline 1 boundary (64 bytes) was 8 bytes ago --- */
> 	struct nf_conntrack_tuple_mask mask;             /*    72    20 */
> 
> 	/* XXX 4 bytes hole, try to pack */
> 
> 	void                       (*expectfn)(struct nf_conn *, struct nf_conntrack_expect *); /*    96     8 */
> 	struct nf_conntrack_helper * helper;             /*   104     8 */
> 	struct nf_conn *           master;               /*   112     8 */
> 	struct timer_list          timeout;              /*   120    88 */
> 	/* --- cacheline 3 boundary (192 bytes) was 16 bytes ago --- */
> 	refcount_t                 use;                  /*   208     4 */
> 	unsigned int               flags;                /*   212     4 */
> 	unsigned int               class;                /*   216     4 */
> 	union nf_inet_addr         saved_addr;           /*   220    16 */
> 	union nf_conntrack_man_proto saved_proto;        /*   236     2 */
> 
> 	/* XXX 2 bytes hole, try to pack */
> 
> 	enum ip_conntrack_dir      dir;                  /*   240     4 */
> 
> 	/* XXX 4 bytes hole, try to pack */
> 
> 	struct callback_head       rcu __attribute__((__aligned__(8))); /*   248    16 */
> 
> 	/* size: 264, cachelines: 5, members: 15 */
> 	/* sum members: 254, holes: 3, sum holes: 10 */
> 	/* forced alignments: 1, forced holes: 1, sum forced holes: 4 */
> 	/* last cacheline: 8 bytes */
> } __attribute__((__aligned__(8)));
> 
> 
> After:
> =====
> struct nf_conntrack_expect {
> 	struct hlist_node          lnode;                /*     0    16 */
> 	struct hlist_node          hnode;                /*    16    16 */
> 	struct nf_conntrack_tuple  tuple;                /*    32    40 */
> 	/* --- cacheline 1 boundary (64 bytes) was 8 bytes ago --- */
> 	struct nf_conntrack_tuple_mask mask;             /*    72    20 */
> 	refcount_t                 use;                  /*    92     4 */
> 	unsigned int               flags;                /*    96     4 */
> 	unsigned int               class;                /*   100     4 */
> 	void                       (*expectfn)(struct nf_conn *, struct nf_conntrack_expect *); /*   104     8 */
> 	struct nf_conntrack_helper * helper;             /*   112     8 */
> 	struct nf_conn *           master;               /*   120     8 */
> 	/* --- cacheline 2 boundary (128 bytes) --- */
> 	struct timer_list          timeout;              /*   128    88 */
> 	/* --- cacheline 3 boundary (192 bytes) was 24 bytes ago --- */
> 	union nf_inet_addr         saved_addr;           /*   216    16 */
> 	union nf_conntrack_man_proto saved_proto;        /*   232     2 */
> 
> 	/* XXX 2 bytes hole, try to pack */
> 
> 	enum ip_conntrack_dir      dir;                  /*   236     4 */
> 	struct callback_head       rcu __attribute__((__aligned__(8))); /*   240    16 */
> 
> 	/* size: 256, cachelines: 4, members: 15 */
> 	/* sum members: 254, holes: 1, sum holes: 2 */
> 	/* forced alignments: 1 */
> } __attribute__((__aligned__(8)));
> ---
>  include/net/netfilter/nf_conntrack_expect.h | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/include/net/netfilter/nf_conntrack_expect.h b/include/net/netfilter/nf_conntrack_expect.h
> index 0855b60fba17..cf0d81be5a96 100644
> --- a/include/net/netfilter/nf_conntrack_expect.h
> +++ b/include/net/netfilter/nf_conntrack_expect.h
> @@ -26,6 +26,15 @@ struct nf_conntrack_expect {
>  	struct nf_conntrack_tuple tuple;
>  	struct nf_conntrack_tuple_mask mask;
>  
> +	/* Usage count. */
> +	refcount_t use;
> +
> +	/* Flags */
> +	unsigned int flags;
> +
> +	/* Expectation class */
> +	unsigned int class;
> +
>  	/* Function to call after setup and insertion */
>  	void (*expectfn)(struct nf_conn *new,
>  			 struct nf_conntrack_expect *this);
> @@ -39,15 +48,6 @@ struct nf_conntrack_expect {
>  	/* Timer function; deletes the expectation. */
>  	struct timer_list timeout;
>  
> -	/* Usage count. */
> -	refcount_t use;
> -
> -	/* Flags */
> -	unsigned int flags;
> -
> -	/* Expectation class */
> -	unsigned int class;
> -
>  #if IS_ENABLED(CONFIG_NF_NAT)
>  	union nf_inet_addr saved_addr;
>  	/* This is the original per-proto part, used to map the
> -- 
> 2.34.1
> 
>
diff mbox series

Patch

diff --git a/include/net/netfilter/nf_conntrack_expect.h b/include/net/netfilter/nf_conntrack_expect.h
index 0855b60fba17..cf0d81be5a96 100644
--- a/include/net/netfilter/nf_conntrack_expect.h
+++ b/include/net/netfilter/nf_conntrack_expect.h
@@ -26,6 +26,15 @@  struct nf_conntrack_expect {
 	struct nf_conntrack_tuple tuple;
 	struct nf_conntrack_tuple_mask mask;
 
+	/* Usage count. */
+	refcount_t use;
+
+	/* Flags */
+	unsigned int flags;
+
+	/* Expectation class */
+	unsigned int class;
+
 	/* Function to call after setup and insertion */
 	void (*expectfn)(struct nf_conn *new,
 			 struct nf_conntrack_expect *this);
@@ -39,15 +48,6 @@  struct nf_conntrack_expect {
 	/* Timer function; deletes the expectation. */
 	struct timer_list timeout;
 
-	/* Usage count. */
-	refcount_t use;
-
-	/* Flags */
-	unsigned int flags;
-
-	/* Expectation class */
-	unsigned int class;
-
 #if IS_ENABLED(CONFIG_NF_NAT)
 	union nf_inet_addr saved_addr;
 	/* This is the original per-proto part, used to map the