diff mbox

[v2,net-next,1/3] net: Add cache alignment in sock_common for socket lookup fields

Message ID 1432658049-3400132-2-git-send-email-tom@herbertland.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Tom Herbert May 26, 2015, 4:34 p.m. UTC
Use ____cacheline_aligned_in_smp to keep the fields used on socket
lookup in their own cachelines. These are read only fields and will
be often accessed on accross CPUs (would be very common with
SO_REUSEPORT for instance).

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 include/net/sock.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Eric Dumazet May 26, 2015, 5:19 p.m. UTC | #1
On Tue, 2015-05-26 at 09:34 -0700, Tom Herbert wrote:
> Use ____cacheline_aligned_in_smp to keep the fields used on socket
> lookup in their own cachelines. These are read only fields and will
> be often accessed on accross CPUs (would be very common with
> SO_REUSEPORT for instance).
> 
> Signed-off-by: Tom Herbert <tom@herbertland.com>
> ---
>  include/net/sock.h | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 26c1c31..bcf6114 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -211,7 +211,13 @@ struct sock_common {
>  		struct hlist_node	skc_node;
>  		struct hlist_nulls_node skc_nulls_node;
>  	};
> -	int			skc_tx_queue_mapping;
> +
> +	/* Cachelines above this point are read mostly and are used in socket
> +	 * lookup.
> +	 */
> +	int			skc_tx_queue_mapping
> +				____cacheline_aligned_in_smp;
> +
>  	atomic_t		skc_refcnt;
>  	/* private: */
>  	int                     skc_dontcopy_end[0];

No, we do not want to increase the size of sock_common with such a
hammer.

I am pretty sure you never tested this patch, since you placed the
attribute at the wrong place anyway.




--
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
Eric Dumazet May 26, 2015, 5:54 p.m. UTC | #2
On Tue, 2015-05-26 at 10:19 -0700, Eric Dumazet wrote:

> No, we do not want to increase the size of sock_common with such a
> hammer.

Current sizeof(struct sock_common) is 0x78 bytes.

So moving 2 read_mostly pointers into this structure would be enough to
make 2 first cache lines read mostly.

One candidate would be sk_rx_dst, as it is used in early demux.


--
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
Tom Herbert May 26, 2015, 6:02 p.m. UTC | #3
On Tue, May 26, 2015 at 10:54 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2015-05-26 at 10:19 -0700, Eric Dumazet wrote:
>
>> No, we do not want to increase the size of sock_common with such a
>> hammer.
>
> Current sizeof(struct sock_common) is 0x78 bytes.
>
> So moving 2 read_mostly pointers into this structure would be enough to
> make 2 first cache lines read mostly.
>
Right, the problem is refcnt out of those cachelines.

> One candidate would be sk_rx_dst, as it is used in early demux.
>
>
--
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
diff mbox

Patch

diff --git a/include/net/sock.h b/include/net/sock.h
index 26c1c31..bcf6114 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -211,7 +211,13 @@  struct sock_common {
 		struct hlist_node	skc_node;
 		struct hlist_nulls_node skc_nulls_node;
 	};
-	int			skc_tx_queue_mapping;
+
+	/* Cachelines above this point are read mostly and are used in socket
+	 * lookup.
+	 */
+	int			skc_tx_queue_mapping
+				____cacheline_aligned_in_smp;
+
 	atomic_t		skc_refcnt;
 	/* private: */
 	int                     skc_dontcopy_end[0];