Re: [PATCH bpf-next] xsk: introduce padding between ring pointers

[Daniel Borkmann <daniel@xxxxxxxxxxxxx>]

On 10/8/20 4:12 PM, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@xxxxxxxxx>
>
> Introduce one cache line worth of padding between the producer and
> consumer pointers in all the lockless rings. This so that the HW
> adjacency prefetcher will not prefetch the consumer pointer when the
> producer pointer is used and vice versa. This improves throughput
> performance for the l2fwd sample app with 2% on my machine with HW
> prefetching turned on.
>
> Signed-off-by: Magnus Karlsson <magnus.karlsson@xxxxxxxxx>

Applied, thanks!

>   net/xdp/xsk_queue.h | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
> index dc1dd5e..3c235d2 100644
> --- a/net/xdp/xsk_queue.h
> +++ b/net/xdp/xsk_queue.h
> @@ -15,6 +15,10 @@
>   
>   struct xdp_ring {
>   	u32 producer ____cacheline_aligned_in_smp;
> +	/* Hinder the adjacent cache prefetcher to prefetch the consumer pointer if the producer
> +	 * pointer is touched and vice versa.
> +	 */
> +	u32 pad ____cacheline_aligned_in_smp;
>   	u32 consumer ____cacheline_aligned_in_smp;
>   	u32 flags;
>   };

I was wondering whether we should even generalize this further for reuse
elsewhere e.g. ...

diff --git a/include/linux/cache.h b/include/linux/cache.h
index 1aa8009f6d06..5521dab01649 100644
--- a/include/linux/cache.h
+++ b/include/linux/cache.h
@@ -85,4 +85,17 @@
 #define cache_line_size()      L1_CACHE_BYTES
 #endif

+/*
+ * Dummy element for use in structs in order to pad a cacheline
+ * aligned element with an extra cacheline to hinder the adjacent
+ * cache prefetcher to prefetch the subsequent struct element.
+ */
+#ifndef ____cacheline_padding_in_smp
+# ifdef CONFIG_SMP
+#  define ____cacheline_padding_in_smp u8 :8 ____cacheline_aligned_in_smp
+# else
+#  define ____cacheline_padding_in_smp
+# endif /* CONFIG_SMP */
+#endif
+
 #endif /* __LINUX_CACHE_H */
diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
index cdb9cf3cd136..1da36423e779 100644
--- a/net/xdp/xsk_queue.h
+++ b/net/xdp/xsk_queue.h
@@ -15,11 +15,9 @@

 struct xdp_ring {
        u32 producer ____cacheline_aligned_in_smp;
-       /* Hinder the adjacent cache prefetcher to prefetch the consumer
-        * pointer if the producer pointer is touched and vice versa.
-        */
-       u32 pad ____cacheline_aligned_in_smp;
+       ____cacheline_padding_in_smp;
        u32 consumer ____cacheline_aligned_in_smp;
+       ____cacheline_padding_in_smp;
        u32 flags;
 };

... was there any improvement to also pad after the consumer given the struct
xdp_ring is also embedded into other structs?

Thanks,
Daniel