Patchwork [net-next,2/6] net: cacheline adjust struct inet_frags for better frag performance

login
register
mail settings
Submitter Jesper Dangaard Brouer
Date Jan. 24, 2013, 2:04 p.m.
Message ID <20130124140417.14119.32738.stgit@dragon>
Download mbox | patch
Permalink /patch/215356/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Jesper Dangaard Brouer - Jan. 24, 2013, 2:04 p.m.
The globally shared rwlock, of struct inet_frags, shares
cacheline with the 'rnd' number, which is used by the hash
calculations.  Fix this, as this obviously is a bad idea, as
unnecessary cache-misses will occur when accessing the 'rnd'
number.

Also small note that, moving function ptr (*match) up in struct,
is to avoid it lands on the next cacheline (on 64-bit).

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---

 include/net/inet_frag.h |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Jan. 29, 2013, 4:23 a.m.
From: Jesper Dangaard Brouer <brouer@redhat.com>
Date: Thu, 24 Jan 2013 15:04:20 +0100

> +	/* --- read-mostly cacheline boundary (was 24 bytes ago) --- */

This "24" is context dependent, I assume it applies to 64-bit builds
rather than 32-bit ones.

Either add the necessary details or just get rid of this part of the
comment altogether.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jesper Dangaard Brouer - Jan. 29, 2013, 7:30 a.m.
On Mon, 2013-01-28 at 23:23 -0500, David Miller wrote:
> From: Jesper Dangaard Brouer <brouer@redhat.com>
> Date: Thu, 24 Jan 2013 15:04:20 +0100
> 
> > +	/* --- read-mostly cacheline boundary (was 24 bytes ago) --- */
> 
> This "24" is context dependent, I assume it applies to 64-bit builds
> rather than 32-bit ones.

True

> Either add the necessary details or just get rid of this part of the
> comment altogether.

I'll remove the comment altogether, and resubmit the patchset.

Thanks for your review. (Hope your flu is gone, and your backlog didn't
grow too big)

Patch

diff --git a/include/net/inet_frag.h b/include/net/inet_frag.h
index 91e7797..99eb4e0 100644
--- a/include/net/inet_frag.h
+++ b/include/net/inet_frag.h
@@ -40,18 +40,22 @@  struct inet_frag_queue {
 
 struct inet_frags {
 	struct hlist_head	hash[INETFRAGS_HASHSZ];
-	rwlock_t		lock;
-	u32			rnd;
-	int			qsize;
+	/* This rwlock is a global lock (seperate per IPv4, IPv6 and
+	 * netfilter). Important to keep this on a seperate cacheline.
+	 */
+	rwlock_t		lock ____cacheline_aligned_in_smp;
 	int			secret_interval;
 	struct timer_list	secret_timer;
+	/* --- read-mostly cacheline boundary (was 24 bytes ago) --- */
+	u32			rnd;
+	int			qsize;
 
 	unsigned int		(*hashfn)(struct inet_frag_queue *);
+	bool			(*match)(struct inet_frag_queue *q, void *arg);
 	void			(*constructor)(struct inet_frag_queue *q,
 						void *arg);
 	void			(*destructor)(struct inet_frag_queue *);
 	void			(*skb_free)(struct sk_buff *);
-	bool			(*match)(struct inet_frag_queue *q, void *arg);
 	void			(*frag_expire)(unsigned long data);
 };