Kernel mode NEON for XOR and RAID6

Add a source file xor-neon.c (which is really just the reference C implementation passed through the GCC vectorizer) and hook it up to the XOR framework.

Output captured from a Cortex-A15 @ 1.7 GHz:

xor: measuring software checksum speed

   arm4regs  :  2261.600 MB/sec
   8regs     :  1771.600 MB/sec
   32regs    :  1441.600 MB/sec
   neon      :  3619.600 MB/sec

xor: using function: neon (3619.600 MB/sec)

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm/include/asm/xor.h | 73 ++++++++++++++++++++++++++++++++++++++++++++++
 arch/arm/lib/Makefile      |  6 ++++
 arch/arm/lib/xor-neon.c    | 42 ++++++++++++++++++++++++++
 3 files changed, 121 insertions(+)
 create mode 100644 arch/arm/lib/xor-neon.c

arch/arm/include/asm/xor.h +73 -0

index 7604673..4ffb26d 100644
@@ -7,7 +7,10 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
+#include <linux/hardirq.h>
 #include <asm-generic/xor.h>
+#include <asm/hwcap.h>
+#include <asm/neon.h>
 
 #define __XOR(a1, a2) a1 ^= a2
 

@@ -138,4 +141,74 @@ static struct xor_block_template xor_block_arm4regs = {
 		xor_speed(&xor_block_arm4regs);	\
 		xor_speed(&xor_block_8regs);	\
 		xor_speed(&xor_block_32regs);	\
+		NEON_TEMPLATES;			\
 	} while (0)
+
+#ifdef CONFIG_KERNEL_MODE_NEON
+
+extern struct xor_block_template const xor_block_neon_inner;
+
+static void
+xor_neon_2(unsigned long bytes, unsigned long *p1, unsigned long *p2)
+{
+	if (in_interrupt()) {
+		xor_arm4regs_2(bytes, p1, p2);
+	} else {
+		kernel_neon_begin();
+		xor_block_neon_inner.do_2(bytes, p1, p2);
+		kernel_neon_end();
+	}
+}
+
+static void
+xor_neon_3(unsigned long bytes, unsigned long *p1, unsigned long *p2,
+		unsigned long *p3)
+{
+	if (in_interrupt()) {
+		xor_arm4regs_3(bytes, p1, p2, p3);
+	} else {
+		kernel_neon_begin();
+		xor_block_neon_inner.do_3(bytes, p1, p2, p3);
+		kernel_neon_end();
+	}
+}
+
+static void
+xor_neon_4(unsigned long bytes, unsigned long *p1, unsigned long *p2,
+		unsigned long *p3, unsigned long *p4)
+{
+	if (in_interrupt()) {
+		xor_arm4regs_4(bytes, p1, p2, p3, p4);
+	} else {
+		kernel_neon_begin();
+		xor_block_neon_inner.do_4(bytes, p1, p2, p3, p4);
+		kernel_neon_end();
+	}
+}
+
+static void
+xor_neon_5(unsigned long bytes, unsigned long *p1, unsigned long *p2,
+		unsigned long *p3, unsigned long *p4, unsigned long *p5)
+{
+	if (in_interrupt()) {
+		xor_arm4regs_5(bytes, p1, p2, p3, p4, p5);
+	} else {
+		kernel_neon_begin();
+		xor_block_neon_inner.do_5(bytes, p1, p2, p3, p4, p5);
+		kernel_neon_end();
+	}
+}
+
+static struct xor_block_template xor_block_neon = {
+	.name	= "neon",
+	.do_2	= xor_neon_2,
+	.do_3	= xor_neon_3,
+	.do_4	= xor_neon_4,
+	.do_5	= xor_neon_5
+};
+
+#define NEON_TEMPLATES	\
+	do { if (cpu_has_neon()) xor_speed(&xor_block_neon); } while (0)
+#else
+#define NEON_TEMPLATES
+#endif

arch/arm/lib/Makefile +6 -0

index af72969..aaf3a87 100644
@@ -45,3 +45,9 @@ lib-$(CONFIG_ARCH_SHARK)	+= io-shark.o
 
 $(obj)/csumpartialcopy.o:	$(obj)/csumpartialcopygeneric.S
 $(obj)/csumpartialcopyuser.o:	$(obj)/csumpartialcopygeneric.S
+
+ifeq ($(CONFIG_KERNEL_MODE_NEON),y)
+  NEON_FLAGS			:= -mfloat-abi=softfp -mfpu=neon
+  CFLAGS_xor-neon.o		+= $(NEON_FLAGS)
+  lib-$(CONFIG_XOR_BLOCKS)	+= xor-neon.o
+endif

arch/arm/lib/xor-neon.c new file +42 -0

new file mode 100644
index 0000000..f485e5a
@@ -0,0 +1,42 @@
+/*
+ * linux/arch/arm/lib/xor-neon.c
+ *
+ * Copyright (C) 2013 Linaro Ltd <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/raid/xor.h>
+
+#ifndef __ARM_NEON__
+#error You should compile this file with '-mfloat-abi=softfp -mfpu=neon'
+#endif
+
+/*
+ * Pull in the reference implementations while instructing GCC (through
+ * -ftree-vectorize) to attempt to exploit implicit parallelism and emit
+ * NEON instructions.
+ */
+#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)
+#pragma GCC optimize "tree-vectorize"
+#else
+/*
+ * While older versions of GCC do not generate incorrect code, they fail to
+ * recognize the parallel nature of these functions, and emit plain ARM code,
+ * which is known to be slower than the optimized ARM code in asm-arm/xor.h.
+ */
+#warning This code requires at least version 4.6 of GCC
+#endif
+
+#pragma GCC diagnostic ignored "-Wunused-variable"
+#include <asm-generic/xor.h>
+
+struct xor_block_template const xor_block_neon_inner = {
+	.name	= "__inner_neon__",
+	.do_2	= xor_8regs_2,
+	.do_3	= xor_8regs_3,
+	.do_4	= xor_8regs_4,
+	.do_5	= xor_8regs_5,
+};

Signature · 1 line

1.8.1.2

Rebased/reworked a patch contributed by Rob Herring that uses NEON intrinsics to perform the RAID-6 syndrome calculations. It uses the existing unroll.awk code to generate several unrolled versions of which the best performing one is selected at boot time.

Output captured from an ARM Cortex-A15 @ 1.7 GHz:

raid6: int32x1 200 MB/s
raid6: int32x2 304 MB/s
raid6: int32x4 388 MB/s
raid6: int32x8 423 MB/s
raid6: neonx1 799 MB/s
raid6: neonx2 1364 MB/s
raid6: neonx4 1731 MB/s
raid6: neonx8 1676 MB/s
raid6: using algorithm neonx4 (1731 MB/s)
raid6: using intx1 recovery algorithm

Signed-off-by Ard Biesheuvel <[email protected]>

---
 include/linux/raid/pq.h |  5 ++++
 lib/raid6/.gitignore    |  1 +
 lib/raid6/Makefile      | 31 +++++++++++++++++++
 lib/raid6/algos.c       |  6 ++++
 lib/raid6/neon.c        | 58 +++++++++++++++++++++++++++++++++++
 lib/raid6/neon.uc       | 80 +++++++++++++++++++++++++++++++++++++++++++++++++
 lib/raid6/test/Makefile | 19 +++++++++++-
 7 files changed, 199 insertions(+), 1 deletion(-)
 create mode 100644 lib/raid6/neon.c
 create mode 100644 lib/raid6/neon.uc

include/linux/raid/pq.h +5 -0

index 8dfaa2c..0f42469 100644
@@ -114,6 +114,11 @@ extern const struct raid6_recov_calls raid6_recov_intx1;
 extern const struct raid6_recov_calls raid6_recov_ssse3;
 extern const struct raid6_recov_calls raid6_recov_avx2;
 
+extern const struct raid6_calls raid6_neonx1;
+extern const struct raid6_calls raid6_neonx2;
+extern const struct raid6_calls raid6_neonx4;
+extern const struct raid6_calls raid6_neonx8;
+
 /* Algorithm list */
 extern const struct raid6_calls * const raid6_algos[];
 extern const struct raid6_recov_calls *const raid6_recov_algos[];

lib/raid6/.gitignore +1 -0

index 162beca..0a7e494 100644
@@ -2,3 +2,4 @@ mktables
 altivec*.c
 int*.c
 tables.c
+neon?.c

lib/raid6/Makefile +31 -0

index 9f7c184..6a51f1a 100644
@@ -5,6 +5,7 @@ raid6_pq-y	+= algos.o recov.o tables.o int1.o int2.o int4.o \
 
 raid6_pq-$(CONFIG_X86) += recov_ssse3.o recov_avx2.o mmx.o sse1.o sse2.o avx2.o
 raid6_pq-$(CONFIG_ALTIVEC) += altivec1.o altivec2.o altivec4.o altivec8.o
+raid6_pq-$(CONFIG_KERNEL_MODE_NEON) += neon.o neon1.o neon2.o neon4.o neon8.o
 
 hostprogs-y	+= mktables
 

@@ -16,6 +17,12 @@ ifeq ($(CONFIG_ALTIVEC),y)
 altivec_flags := -maltivec -mabi=altivec
 endif
 
+# The GCC option -ffreestanding is required in order to compile code containing
+# ARM/NEON intrinsics in a non C99-compliant environment (such as the kernel)
+ifeq ($(CONFIG_KERNEL_MODE_NEON),y)
+NEON_FLAGS := -mfloat-abi=softfp -mfpu=neon -ffreestanding
+endif
+
 targets += int1.c
 $(obj)/int1.c:   UNROLL := 1
 $(obj)/int1.c:   $(src)/int.uc $(src)/unroll.awk FORCE

@@ -70,6 +77,30 @@ $(obj)/altivec8.c:   UNROLL := 8
 $(obj)/altivec8.c:   $(src)/altivec.uc $(src)/unroll.awk FORCE
 	$(call if_changed,unroll)
 
+CFLAGS_neon1.o += $(NEON_FLAGS)
+targets += neon1.c
+$(obj)/neon1.c:   UNROLL := 1
+$(obj)/neon1.c:   $(src)/neon.uc $(src)/unroll.awk FORCE
+	$(call if_changed,unroll)
+
+CFLAGS_neon2.o += $(NEON_FLAGS)
+targets += neon2.c
+$(obj)/neon2.c:   UNROLL := 2
+$(obj)/neon2.c:   $(src)/neon.uc $(src)/unroll.awk FORCE
+	$(call if_changed,unroll)
+
+CFLAGS_neon4.o += $(NEON_FLAGS)
+targets += neon4.c
+$(obj)/neon4.c:   UNROLL := 4
+$(obj)/neon4.c:   $(src)/neon.uc $(src)/unroll.awk FORCE
+	$(call if_changed,unroll)
+
+CFLAGS_neon8.o += $(NEON_FLAGS)
+targets += neon8.c
+$(obj)/neon8.c:   UNROLL := 8
+$(obj)/neon8.c:   $(src)/neon.uc $(src)/unroll.awk FORCE
+	$(call if_changed,unroll)
+
 quiet_cmd_mktable = TABLE   $@
       cmd_mktable = $(obj)/mktables > $@ || ( rm -f $@ && exit 1 )
 

lib/raid6/algos.c +6 -0

index 6d7316f..74e6f56 100644
@@ -70,6 +70,12 @@ const struct raid6_calls * const raid6_algos[] = {
 	&raid6_intx2,
 	&raid6_intx4,
 	&raid6_intx8,
+#ifdef CONFIG_KERNEL_MODE_NEON
+	&raid6_neonx1,
+	&raid6_neonx2,
+	&raid6_neonx4,
+	&raid6_neonx8,
+#endif
 	NULL
 };
 

lib/raid6/neon.c new file +58 -0

new file mode 100644
index 0000000..dad7102
@@ -0,0 +1,58 @@
+/*
+ * linux/lib/raid6/neon.c - RAID6 syndrome calculation using ARM NEON intrinsics
+ *
+ * Copyright (C) 2013 Linaro Ltd <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/raid/pq.h>
+
+#ifdef __KERNEL__
+#include <asm/neon.h>
+#else
+#define kernel_neon_begin()
+#define kernel_neon_end()
+#define cpu_has_neon()		(1)
+#endif
+
+/*
+ * There are 2 reasons these wrappers are kept in a separate compilation unit
+ * from the actual implementations in neonN.c (generated from neon.uc by
+ * unroll.awk):
+ * - the actual implementations use NEON intrinsics, and the GCC support header
+ *   (arm_neon.h) is not fully compatible (type wise) with the kernel;
+ * - the neonN.c files are compiled with -mfpu=neon and optimization enabled,
+ *   and we have to make sure that we never use *any* NEON/VFP instructions
+ *   outside a kernel_neon_begin()/kernel_neon_end() pair.
+ */
+
+#define RAID6_NEON_WRAPPER(_n)						\
+	static void raid6_neon ## _n ## _gen_syndrome(int disks,	\
+					size_t bytes, void **ptrs)	\
+	{								\
+		void raid6_neon ## _n  ## _gen_syndrome_real(int,	\
+						unsigned int, void**);	\
+		kernel_neon_begin();					\
+		raid6_neon ## _n ## _gen_syndrome_real(disks,		\
+					(unsigned int)bytes, ptrs);	\
+		kernel_neon_end();					\
+	}								\
+	struct raid6_calls const raid6_neonx ## _n = {			\
+		raid6_neon ## _n ## _gen_syndrome,			\
+		raid6_have_neon,					\
+		"neonx" #_n,						\
+		0							\
+	};
+
+static int raid6_have_neon(void)
+{
+	return cpu_has_neon();
+}
+
+RAID6_NEON_WRAPPER(1)
+RAID6_NEON_WRAPPER(2)
+RAID6_NEON_WRAPPER(4)
+RAID6_NEON_WRAPPER(8)

lib/raid6/neon.uc new file +80 -0

new file mode 100644
index 0000000..f2d7ec0
@@ -0,0 +1,80 @@
+/* -----------------------------------------------------------------------
+ *
+ *   neon.uc - RAID-6 syndrome calculation using ARM NEON instructions
+ *
+ *   Copyright (C) 2012 Rob Herring
+ *
+ *   Based on altivec.uc:
+ *     Copyright 2002-2004 H. Peter Anvin - All Rights Reserved
+ *
+ *   This program is free software; you can redistribute it and/or modify
+ *   it under the terms of the GNU General Public License as published by
+ *   the Free Software Foundation, Inc., 53 Temple Place Ste 330,
+ *   Boston MA 02111-1307, USA; either version 2 of the License, or
+ *   (at your option) any later version; incorporated herein by reference.
+ *
+ * ----------------------------------------------------------------------- */
+
+/*
+ * neon$#.c
+ *
+ * $#-way unrolled NEON intrinsics math RAID-6 instruction set
+ *
+ * This file is postprocessed using unroll.awk
+ */
+
+#include <arm_neon.h>
+
+typedef uint8x16_t unative_t;
+
+#define NBYTES(x) ((unative_t){x,x,x,x, x,x,x,x, x,x,x,x, x,x,x,x})
+#define NSIZE	sizeof(unative_t)
+
+/*
+ * The SHLBYTE() operation shifts each byte left by 1, *not*
+ * rolling over into the next byte
+ */
+static inline unative_t SHLBYTE(unative_t v)
+{
+	return vshlq_n_u8(v, 1);
+}
+
+/*
+ * The MASK() operation returns 0xFF in any byte for which the high
+ * bit is 1, 0x00 for any byte for which the high bit is 0.
+ */
+static inline unative_t MASK(unative_t v)
+{
+	const uint8x16_t temp = NBYTES(0);
+	return (unative_t)vcltq_s8((int8x16_t)v, (int8x16_t)temp);
+}
+
+void raid6_neon$#_gen_syndrome_real(int disks, unsigned int bytes, void **ptrs)
+{
+	uint8_t **dptr = (uint8_t **)ptrs;
+	uint8_t *p, *q;
+	int d, z, z0;
+
+	register unative_t wd$$, wq$$, wp$$, w1$$, w2$$;
+	const unative_t x1d = NBYTES(0x1d);
+
+	z0 = disks - 3;		/* Highest data disk */
+	p = dptr[z0+1];		/* XOR parity */
+	q = dptr[z0+2];		/* RS syndrome */
+
+	for ( d = 0 ; d < bytes ; d += NSIZE*$# ) {
+		wq$$ = wp$$ = vld1q_u8(&dptr[z0][d+$$*NSIZE]);
+		for ( z = z0-1 ; z >= 0 ; z-- ) {
+			wd$$ = vld1q_u8(&dptr[z][d+$$*NSIZE]);
+			wp$$ = veorq_u8(wp$$, wd$$);
+			w2$$ = MASK(wq$$);
+			w1$$ = SHLBYTE(wq$$);
+
+			w2$$ = vandq_u8(w2$$, x1d);
+			w1$$ = veorq_u8(w1$$, w2$$);
+			wq$$ = veorq_u8(w1$$, wd$$);
+		}
+		vst1q_u8(&p[d+NSIZE*$$], wp$$);
+		vst1q_u8(&q[d+NSIZE*$$], wq$$);
+	}
+}

lib/raid6/test/Makefile +18 -1

index 087332d..bbe6450 100644
@@ -34,6 +34,11 @@ else
         ifeq ($(HAS_ALTIVEC),yes)
                 OBJS += altivec1.o altivec2.o altivec4.o altivec8.o
         endif
+        ifeq ($(ARCH),arm)
+                CFLAGS += -I../../../arch/arm/include -mfpu=neon \
+                          -DCONFIG_KERNEL_MODE_NEON=1
+                OBJS += neon.o neon1.o neon2.o neon4.o neon8.o
+        endif
 endif
 
 .c.o:

@@ -55,6 +60,18 @@ raid6.a: $(OBJS)
 raid6test: test.c raid6.a
 	$(CC) $(CFLAGS) -o raid6test $^
 
+neon1.c: neon.uc ../unroll.awk
+	$(AWK) ../unroll.awk -vN=1 < neon.uc > $@
+
+neon2.c: neon.uc ../unroll.awk
+	$(AWK) ../unroll.awk -vN=2 < neon.uc > $@
+
+neon4.c: neon.uc ../unroll.awk
+	$(AWK) ../unroll.awk -vN=4 < neon.uc > $@
+
+neon8.c: neon.uc ../unroll.awk
+	$(AWK) ../unroll.awk -vN=8 < neon.uc > $@
+
 altivec1.c: altivec.uc ../unroll.awk
 	$(AWK) ../unroll.awk -vN=1 < altivec.uc > $@
 

@@ -89,7 +106,7 @@ tables.c: mktables
 	./mktables > tables.c
 
 clean:
-	rm -f *.o *.a mktables mktables.c *.uc int*.c altivec*.c tables.c raid6test
+	rm -f *.o *.a mktables mktables.c *.uc int*.c altivec*.c neon*.c tables.c raid6test
 
 spotless: clean
 	rm -f *~

Signature · 1 line

1.8.1.2

Hi all,

This is a partial repost of the patches I proposed a couple of weeks ago to add support for VFP/NEON in kernel mode.

This time, I have included two use cases that I have been using, XOR and RAID-6 checksumming. The former gets a 60% performance boost on the NEON, the latter over 400%.

lib/raid6: add ARM-NEON accelerated syndrome calculation

This is a port of the RAID-6 checksumming code in altivec.uc ported to use NEON intrinsics. It is about 4x faster than the sequential code. As this code does not live under arch/arm, I will send this patch separately to the appropriate list if/when the prerequisite patches from this series have been accepted.

Quoted 3 lines · Ard Biesheuvel

This time, I have included two use cases that I have been using, XOR and RAID-6 checksumming. The former gets a 60% performance boost on the NEON, the latter over 400%.

Whilst that sounds impressive, can you achieve similar results across all NEON-capable CPUs? In particular, we need to make sure this doesn't cause performance regressions on some cores. Furthermore, do you have any power

figures to complement your findings? The increased context-switch overhead is also worth measuring if you can (i.e. run some userspace NEON-based benchmarks in parallel with NEON and non-NEON implementations of the checksumming).

Quoted 6 lines

lib/raid6: add ARM-NEON accelerated syndrome calculation

This is a port of the RAID-6 checksumming code in altivec.uc ported to use NEON intrinsics. It is about 4x faster than the sequential code. As this code does not live under arch/arm, I will send this patch separately to the appropriate list if/when the prerequisite patches from this series have been accepted.

We support building the kernel with older toolchains, so I don't see the benefit of using intrinsics here. Have you tried writing an implementation with NEON instructions directly?

Quoted 1 line · Ard Biesheuvel

Hi all,

Hi Ard,

Quoted 3 lines

This is a partial repost of the patches I proposed a couple of weeks ago to add support for VFP/NEON in kernel mode.

Quoted 3 lines

This time, I have included two use cases that I have been using, XOR and RAID-6 checksumming. The former gets a 60% performance boost on the NEON, the latter over 400%.

Whilst that sounds impressive, can you achieve similar results across all NEON-capable CPUs? In particular, we need to make sure this doesn't cause performance regressions on some cores.

Furthermore, do you have any power figures to complement your findings?

The increased context-switch overhead
is also worth measuring if you can (i.e. run some userspace NEON-based benchmarks in parallel with NEON and non-NEON implementations of the checksumming).

We support building the kernel with older toolchains, so I don't see the benefit of using intrinsics here.

Quoted 8 lines · Will Deacon

Quoted 1 line · Ard Biesheuvel

Hi all,

Hi Ard,

Quoted 3 lines

This is a partial repost of the patches I proposed a couple of weeks ago to add support for VFP/NEON in kernel mode.

Quoted 7 lines

Quoted 3 lines

This time, I have included two use cases that I have been using, XOR and RAID-6 checksumming. The former gets a 60% performance boost on the NEON, the latter over 400%.

Whilst that sounds impressive, can you achieve similar results across all NEON-capable CPUs? In particular, we need to make sure this doesn't cause performance regressions on some cores.

Note that the kernel performs runtime benchmarking of all the different implementations it has available at boot time and selects the best one. So if this would turn out to make things worse on some cores then the Neon code would simply not be used.

Quoted 2 lines

Furthermore, do you have any power figures to complement your findings?

This is going to be most useful in server type environments where a bit more power is not such an issue but throughput is ... unless you start using RAID6 arrays on your phone that is. :-) Otherwise this can be left configured out for mobile targets.

Quoted 4 lines

The increased context-switch overhead
is also worth measuring if you can (i.e. run some userspace NEON-based benchmarks in parallel with NEON and non-NEON implementations of the checksumming).

Do we know the context switch cost of normal task scheduling between tasks using FP operations? The in-kernel Neon usage should bring about the same cost. Measuring it would be interesting albeit probably difficult.

Quoted 2 lines

We support building the kernel with older toolchains, so I don't see the benefit of using intrinsics here.

These days the compiler tends to do a better job than humans at properly scheduling instructions for some code. We shouldn't deprive ourselves from it when a recent enough gcc is available.

Nicolas

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 5 lines · Will Deacon

Quoted 3 lines · Ard Biesheuvel

This time, I have included two use cases that I have been using, XOR and RAID-6 checksumming. The former gets a 60% performance boost on the NEON, the latter over 400%.

Quoted 3 lines

Whilst that sounds impressive, can you achieve similar results across all NEON-capable CPUs? In particular, we need to make sure this doesn't cause performance regressions on some cores.

Note that the kernel performs runtime benchmarking of all the different implementations it has available at boot time and selects the best one. So if this would turn out to make things worse on some cores then the Neon code would simply not be used.

Quoted 2 lines

Furthermore, do you have any power figures to complement your findings?

This is going to be most useful in server type environments where a bit more power is not such an issue but throughput is ... unless you start using RAID6 arrays on your phone that is. :-) Otherwise this can be left configured out for mobile targets.

Quoted 4 lines

The increased context-switch overhead
is also worth measuring if you can (i.e. run some userspace NEON-based benchmarks in parallel with NEON and non-NEON implementations of the checksumming).

Do we know the context switch cost of normal task scheduling between tasks using FP operations? The in-kernel Neon usage should bring about the same cost. Measuring it would be interesting albeit probably difficult.

Quoted 2 lines

We support building the kernel with older toolchains, so I don't see the benefit of using intrinsics here.

These days the compiler tends to do a better job than humans at properly scheduling instructions for some code. We shouldn't deprive ourselves from it when a recent enough gcc is available.

Quoted 10 lines · Nicolas Pitre

Quoted 8 lines · Will Deacon

Quoted 3 lines · Ard Biesheuvel

This time, I have included two use cases that I have been using, XOR and RAID-6 checksumming. The former gets a 60% performance boost on the NEON, the latter over 400%.

Whilst that sounds impressive, can you achieve similar results across all NEON-capable CPUs? In particular, we need to make sure this doesn't cause performance regressions on some cores.

Quoted 4 lines

Note that the kernel performs runtime benchmarking of all the different implementations it has available at boot time and selects the best one. So if this would turn out to make things worse on some cores then the Neon code would simply not be used.

That will be all sorts of fun if we try to run this on big.LITTLE... Perhaps we don't care about that either.

Quoted 7 lines

Quoted 2 lines

Furthermore, do you have any power figures to complement your findings?

This is going to be most useful in server type environments where a bit more power is not such an issue but throughput is ... unless you start using RAID6 arrays on your phone that is. :-) Otherwise this can be left configured out for mobile targets.

Agreed, but this patch series also sets a precedent for using NEON in the kernel. Whilst I'd love to hook up some SCSI arrays to my Nexus 4 (!), much more likely is that people might start reworking some of the crypto algorithms to use the NEON/SIMD register file (especially with the crypto extensions in ARMv8) so it would be good to have *some* feel of the power impact off the bat.

Quoted 9 lines

Quoted 4 lines

The increased context-switch overhead
is also worth measuring if you can (i.e. run some userspace NEON-based benchmarks in parallel with NEON and non-NEON implementations of the checksumming).

Do we know the context switch cost of normal task scheduling between tasks using FP operations? The in-kernel Neon usage should bring about the same cost. Measuring it would be interesting albeit probably difficult.

Sure, this stuff is hard to measure and we don't have a feel for the normal context-switch penalities. I just think it would be useful to try and get a feel for the increased overhead of saving/restoring this state if userspace is trying to use the registers in parallel with the kernel.

Quoted 6 lines

Quoted 2 lines

We support building the kernel with older toolchains, so I don't see the benefit of using intrinsics here.

These days the compiler tends to do a better job than humans at properly scheduling instructions for some code. We shouldn't deprive ourselves from it when a recent enough gcc is available.

What's the earliest toolchain we claim to support nowadays? If that can't deal with the intrinsics then we either need to bump the requirement, or write this using hand-coded asm. In the case of the latter, I don't think the maintenance overhead of having two implementations is worth it.

Hello Nicolas, Ard,

Quoted 10 lines · Nicolas Pitre

Quoted 8 lines · Will Deacon

Quoted 3 lines · Ard Biesheuvel

This time, I have included two use cases that I have been using, XOR and RAID-6 checksumming. The former gets a 60% performance boost on the NEON, the latter over 400%.

Whilst that sounds impressive, can you achieve similar results across all NEON-capable CPUs? In particular, we need to make sure this doesn't cause performance regressions on some cores.

Quoted 4 lines

Note that the kernel performs runtime benchmarking of all the different implementations it has available at boot time and selects the best one. So if this would turn out to make things worse on some cores then the Neon code would simply not be used.

That will be all sorts of fun if we try to run this on big.LITTLE... Perhaps we don't care about that either.

Quoted 7 lines

Quoted 2 lines

Furthermore, do you have any power figures to complement your findings?

This is going to be most useful in server type environments where a bit more power is not such an issue but throughput is ... unless you start using RAID6 arrays on your phone that is. :-) Otherwise this can be left configured out for mobile targets.

Agreed, but this patch series also sets a precedent for using NEON in the kernel. Whilst I'd love to hook up some SCSI arrays to my Nexus 4 (!), much more likely is that people might start reworking some of the crypto algorithms to use the NEON/SIMD register file (especially with the crypto extensions in ARMv8) so it would be good to have *some* feel of the power impact off the bat.

Quoted 6 lines

Quoted 2 lines

We support building the kernel with older toolchains, so I don't see the benefit of using intrinsics here.

These days the compiler tends to do a better job than humans at properly scheduling instructions for some code. We shouldn't deprive ourselves from it when a recent enough gcc is available.

What's the earliest toolchain we claim to support nowadays? If that can't deal with the intrinsics then we either need to bump the requirement, or write this using hand-coded asm. In the case of the latter, I don't think the maintenance overhead of having two implementations is worth it.

Quoted 7 lines · Will Deacon

Quoted 4 lines

Note that the kernel performs runtime benchmarking of all the different implementations it has available at boot time and selects the best one. So if this would turn out to make things worse on some cores then the Neon code would simply not be used.

That will be all sorts of fun if we try to run this on big.LITTLE... Perhaps we don't care about that either.

Probably not at the present time.

Quoted 4 lines

What's the earliest toolchain we claim to support nowadays? If that can't deal with the intrinsics then we either need to bump the requirement, or write this using hand-coded asm. In the case of the latter, I don't think the maintenance overhead of having two implementations is worth it.

We have many different minimum toolchain version requirements attached to different features being enabled already, ftrace being one of them if I remember correctly. For these Neon optimizations the minimum gcc version is v4.6.

Given that this is going to be interesting mostly to server systems, and given that ARM server deployments are rather new, I don't see the point of compiling a new server environment using an older gcc version.

Quoted 6 lines · Nicolas Pitre

Quoted 4 lines · Will Deacon

What's the earliest toolchain we claim to support nowadays? If that can't deal with the intrinsics then we either need to bump the requirement, or write this using hand-coded asm. In the case of the latter, I don't think the maintenance overhead of having two implementations is worth it.

Quoted 8 lines

We have many different minimum toolchain version requirements attached to different features being enabled already, ftrace being one of them if I remember correctly. For these Neon optimizations the minimum gcc version is v4.6.

Given that this is going to be interesting mostly to server systems, and given that ARM server deployments are rather new, I don't see the point of compiling a new server environment using an older gcc version.

I've mulled over this, had some discussions with our toolchain guys and have concluded the following:

- The intrinsics are actually ok. I was sceptical at first, but I've been assured that they should do a reasonable job (echoing your performance figures).

- The current approach is targetting servers and isn't (yet) suitable for mobile.

So, given that the patches do the right thing wrt GCC version, the only remaining point is that we need to keep an eye out for people trying to re-use this stuff for mobile (likely crypto, as I mentioned earlier). When that happens, we should consider revisiting the benchmark/power figures.

Quoted 23 lines · Will Deacon

Quoted 6 lines · Nicolas Pitre

Quoted 4 lines · Will Deacon

What's the earliest toolchain we claim to support nowadays? If that can't deal with the intrinsics then we either need to bump the requirement, or write this using hand-coded asm. In the case of the latter, I don't think the maintenance overhead of having two implementations is worth it.

Quoted 8 lines

We have many different minimum toolchain version requirements attached to different features being enabled already, ftrace being one of them if I remember correctly. For these Neon optimizations the minimum gcc version is v4.6.

Given that this is going to be interesting mostly to server systems, and given that ARM server deployments are rather new, I don't see the point of compiling a new server environment using an older gcc version.

I've mulled over this, had some discussions with our toolchain guys and have concluded the following:

- The intrinsics are actually ok. I was sceptical at first, but I've been assured that they should do a reasonable job (echoing your performance figures).

Quoted 8 lines

- The current approach is targetting servers and isn't (yet) suitable for mobile.

So, given that the patches do the right thing wrt GCC version, the only remaining point is that we need to keep an eye out for people trying to re-use this stuff for mobile (likely crypto, as I mentioned earlier). When that happens, we should consider revisiting the benchmark/power figures.

OK, so a number of points have been raised in this discussion, let me address them one by one:

Should we allow NEON to be used in the kernel?

The consensus is not to allow floating point. However, NEON is different, as the performance gains are considerable and there is no dependency on support code, which makes it not as hairy as conventional (pre-v3) VFP. Also, managing the vfpstates is easily doable if NEON is only used outside interrupt context and with preemption disabled.

Does my series implement it correctly?

I have addressed Russell's first round of comments. Happy to take another round if necessary.

Should we allow NEON intrinsics in the kernel?
Should we allow GCC-generated NEON in the kernel?

Only if the implementation is clear on which minimum version of GCC it requires. We could use my examples to set a precedent on what is a suitable way to use NEON intrinsics or the vectorizer in kernel code (which includes coding it such that it can be reused for arm64 with no modifications)

Is kernel mode NEON suitable for mobile?

To me, it is unclear why kernel and userland are so different in this respect. However, kernel mode NEON is separately configurable from Kconfig so it can be disabled at will.

Is there a point to doing a boot time benchmark to select the optimal implementation of an algorithm?

Perhaps not but unrelated to kernel mode NEON.

Hi Ard,

If this is indeed the consensus (I don't disagree with any of it myself), perhaps committing the main points, guidelines, and examples to Documentation/arm/* would be useful.

Quoted 6 lines · Christopher Covington

Hi Ard,

If this is indeed the consensus (I don't disagree with any of it myself), perhaps committing the main points, guidelines, and examples to Documentation/arm/* would be useful.

Hello Chris,

I agree that it makes sense to document any guidelines we set regarding the use of NEON in kernel mode, and I will happily do so. However, let's first try to wrap up this discussion, haven't heard back from Russell yet on his current position in this matter.

I think it's probably fine now - you have ample justification, and you seem to be ensuring that things are done safely.

The only thing I'm left wondering about is whether the bits outside arch/arm should go to anyone else, but MAINTAINERS is being unhelpful on that - so I'm tempted to say that if someone should've been copied who hasn't, and they're not in MAINTAINERS that's their problem.

Hi Ard,

Quoted 73 lines · Ard Biesheuvel

Quoted 23 lines · Will Deacon

Quoted 6 lines · Nicolas Pitre

Quoted 4 lines · Will Deacon

What's the earliest toolchain we claim to support nowadays? If that can't deal with the intrinsics then we either need to bump the requirement, or write this using hand-coded asm. In the case of the latter, I don't think the maintenance overhead of having two implementations is worth it.

Quoted 8 lines

We have many different minimum toolchain version requirements attached to different features being enabled already, ftrace being one of them if I remember correctly. For these Neon optimizations the minimum gcc version is v4.6.

Given that this is going to be interesting mostly to server systems, and given that ARM server deployments are rather new, I don't see the point of compiling a new server environment using an older gcc version.

I've mulled over this, had some discussions with our toolchain guys and have concluded the following:

- The intrinsics are actually ok. I was sceptical at first, but I've been assured that they should do a reasonable job (echoing your performance figures).

Quoted 8 lines

- The current approach is targetting servers and isn't (yet) suitable for mobile.

So, given that the patches do the right thing wrt GCC version, the only remaining point is that we need to keep an eye out for people trying to re-use this stuff for mobile (likely crypto, as I mentioned earlier). When that happens, we should consider revisiting the benchmark/power figures.

OK, so a number of points have been raised in this discussion, let me address them one by one:

Should we allow NEON to be used in the kernel?

The consensus is not to allow floating point. However, NEON is different, as the performance gains are considerable and there is no dependency on support code, which makes it not as hairy as conventional (pre-v3) VFP. Also, managing the vfpstates is easily doable if NEON is only used outside interrupt context and with preemption disabled.

Does my series implement it correctly?

I have addressed Russell's first round of comments. Happy to take another round if necessary.

Should we allow NEON intrinsics in the kernel?
Should we allow GCC-generated NEON in the kernel?

Only if the implementation is clear on which minimum version of GCC it requires. We could use my examples to set a precedent on what is a suitable way to use NEON intrinsics or the vectorizer in kernel code (which includes coding it such that it can be reused for arm64 with no modifications)

Is kernel mode NEON suitable for mobile?

To me, it is unclear why kernel and userland are so different in this respect. However, kernel mode NEON is separately configurable from Kconfig so it can be disabled at will.

Is there a point to doing a boot time benchmark to select the optimal implementation of an algorithm?

Perhaps not but unrelated to kernel mode NEON.

Significant benchmarks on the boot path would be unacceptable, unless they are *fast* (and by fast, I mean fast on all platforms, not just fast on the fast platforms). If one second gets added onto the boot path for each optimised algorithm, that sounds like a fail. If all the benchmarks combined take one second in total, that's no quite as bad.

Maybe benchmarks could be time-bounded (i.e., see how much data we can chug though in X milliseconds) instead of size-bounded. This would avoid unreasonable slowdown on slow platforms, while avoiding trivially small benchmark payloads on faster platforms which may typically have a more complex architecture, bigger caches etc. which would cause them to take longer to reach saturated performance when running a particular algorithm.

Quoted 6 lines · Dave Martin

Significant benchmarks on the boot path would be unacceptable, unless they are *fast* (and by fast, I mean fast on all platforms, not just fast on the fast platforms). If one second gets added onto the boot path for each optimised algorithm, that sounds like a fail. If all the benchmarks combined take one second in total, that's no quite as bad.

Quoted 7 lines

Maybe benchmarks could be time-bounded (i.e., see how much data we can chug though in X milliseconds) instead of size-bounded. This would avoid unreasonable slowdown on slow platforms, while avoiding trivially small benchmark payloads on faster platforms which may typically have a more complex architecture, bigger caches etc. which would cause them to take longer to reach saturated performance when running a particular algorithm.

Benchmarks are already time bounded, at least the instances I am aware of (xor and raid6) are. They each measure, for each available implementation, the amount of work performed during a fixed time. For RAID6, this is 16 jiffies, for XOR it's only 1 jiffy but each test is repeated 5 times.

So I think this should not be a problem, especially as it is unlikely that newly added implementations (such as NEON) will be able to execute on older/slower platforms in the first place.