Fixes and enhancements of SIMD raidz parity

- Implementation lock replaced with atomic variable

- Trailing whitespace is removed from user specified parameter, to enhance
experience when using commands that add newline, e.g. `echo`

- raidz_test: remove dependency on `getrusage()` and RUSAGE_THREAD, Issue openzfs#4813

- silence `cppcheck` in vdev_raidz, partial solution of Issue openzfs#1392

- Minor fixes and cleanups

- Enable use of original parity methods in [fastest] configuration.
New opaque original ops structure, representing native methods, is added
to supported raidz methods. Original parity methods are executed if selected
implementation has NULL fn pointer.

Signed-off-by: Gvozden Neskovic <neskovic@gmail.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Issue openzfs#4813
Issue openzfs#1392

cmd/raidz_test/raidz_bench.c

@@ -32,7 +32,6 @@
 #include <stdio.h>
 
 #include <sys/time.h>
-#include <sys/resource.h>
 
 #include "raidz_test.h"
 
@@ -42,7 +41,6 @@
 #define	MIN_CS_SHIFT		BENCH_ASHIFT
 #define	MAX_CS_SHIFT		SPA_MAXBLOCKSHIFT
 
-
 static zio_t zio_bench;
 static raidz_map_t *rm_bench;
 static size_t max_data_size = SPA_MAXBLOCKSIZE;
@@ -70,28 +68,18 @@ bench_fini_raidz_maps(void)
 	bzero(&zio_bench, sizeof (zio_t));
 }
 
-static double
-get_time_diff(struct rusage *start, struct rusage *stop)
-{
-	return (((double)stop->ru_utime.tv_sec * (double)MICROSEC +
-		(double)stop->ru_utime.tv_usec) -
-		((double)start->ru_utime.tv_sec * (double)MICROSEC +
-		(double)start->ru_utime.tv_usec)) / (double)MICROSEC;
-}
-
 static inline void
 run_gen_bench_impl(const char *impl)
 {
 	int fn, ncols;
 	uint64_t ds, iter_cnt, iter, disksize;
-	struct rusage start, stop;
+	hrtime_t start;
 	double elapsed, d_bw;
 
 	/* Benchmark generate functions */
 	for (fn = 0; fn < RAIDZ_GEN_NUM; fn++) {
 
 		for (ds = MIN_CS_SHIFT; ds <= MAX_CS_SHIFT; ds++) {
-
 			/* create suitable raidz_map */
 			ncols = rto_opts.rto_dcols + fn + 1;
 			zio_bench.io_size = 1ULL << ds;
@@ -102,12 +90,11 @@ run_gen_bench_impl(const char *impl)
 			iter_cnt = GEN_BENCH_MEMORY;
 			iter_cnt /= zio_bench.io_size;
 
-			getrusage(RUSAGE_THREAD, &start);
+			start = gethrtime();
 			for (iter = 0; iter < iter_cnt; iter++)
 				vdev_raidz_generate_parity(rm_bench);
-			getrusage(RUSAGE_THREAD, &stop);
+			elapsed = NSEC2SEC((double) (gethrtime() - start));
 
-			elapsed = get_time_diff(&start, &stop);
 			disksize = (1ULL << ds) / rto_opts.rto_dcols;
 			d_bw = (double)iter_cnt * (double)disksize;
 			d_bw /= (1024.0 * 1024.0 * elapsed);
@@ -147,9 +134,9 @@ run_gen_bench(void)
 static void
 run_rec_bench_impl(const char *impl)
 {
-	struct rusage start, stop;
 	int fn, ncols, nbad;
 	uint64_t ds, iter_cnt, iter, disksize;
+	hrtime_t start;
 	double elapsed, d_bw;
 	static const int tgt[7][3] = {
 		{1, 2, 3},	/* rec_p:   bad QR & D[0]	*/
@@ -187,12 +174,11 @@ run_rec_bench_impl(const char *impl)
 			nbad = MIN(3, raidz_ncols(rm_bench) -
 			    raidz_parity(rm_bench));
 
-			getrusage(RUSAGE_THREAD, &start);
+			start = gethrtime();
 			for (iter = 0; iter < iter_cnt; iter++)
 				vdev_raidz_reconstruct(rm_bench, tgt[fn], nbad);
-			getrusage(RUSAGE_THREAD, &stop);
+			elapsed = NSEC2SEC((double) (gethrtime() - start));
 
-			elapsed = get_time_diff(&start, &stop);
 			disksize = (1ULL << ds) / rto_opts.rto_dcols;
 			d_bw = (double)iter_cnt * (double)(disksize);
 			d_bw /= (1024.0 * 1024.0 * elapsed);

include/sys/vdev_raidz.h

@@ -40,22 +40,22 @@ struct kernel_param {};
 /*
  * vdev_raidz interface
  */
-struct raidz_map *	vdev_raidz_map_alloc(struct zio *, uint64_t, uint64_t,
-			    uint64_t);
+struct raidz_map * vdev_raidz_map_alloc(struct zio *, uint64_t, uint64_t,
+    uint64_t);
 void		vdev_raidz_map_free(struct raidz_map *);
 void 		vdev_raidz_generate_parity(struct raidz_map *);
 int 		vdev_raidz_reconstruct(struct raidz_map *, const int *, int);
 
 /*
  * vdev_raidz_math interface
  */
-void	vdev_raidz_math_init(void);
-void	vdev_raidz_math_fini(void);
-void	vdev_raidz_math_get_ops(struct raidz_map *);
-void	vdev_raidz_math_generate(struct raidz_map *);
-int	vdev_raidz_math_reconstruct(struct raidz_map *, const int *,
-	    const int *, const int);
-int	vdev_raidz_impl_set(const char *);
+void			vdev_raidz_math_init(void);
+void			vdev_raidz_math_fini(void);
+struct raidz_impl_ops *	vdev_raidz_math_get_ops(void);
+int			vdev_raidz_math_generate(struct raidz_map *);
+int			vdev_raidz_math_reconstruct(struct raidz_map *,
+			    const int *, const int *, const int);
+int			vdev_raidz_impl_set(const char *);
 
 #ifdef	__cplusplus
 }

include/sys/vdev_raidz_impl.h

@@ -89,13 +89,15 @@ typedef boolean_t	(*will_work_f)(void);
 typedef void		(*init_impl_f)(void);
 typedef void		(*fini_impl_f)(void);
 
+#define	RAIDZ_IMPL_NAME_MAX	(16)
+
 typedef struct raidz_impl_ops {
 	init_impl_f init;
 	fini_impl_f fini;
 	raidz_gen_f gen[RAIDZ_GEN_NUM];	/* Parity generate functions */
 	raidz_rec_f rec[RAIDZ_REC_NUM];	/* Data reconstruction functions */
 	will_work_f is_supported;	/* Support check function */
-	char *name;			/* Name of the implementation */
+	char name[RAIDZ_IMPL_NAME_MAX];	/* Name of the implementation */
 } raidz_impl_ops_t;
 
 typedef struct raidz_col {
@@ -127,6 +129,8 @@ typedef struct raidz_map {
 	raidz_col_t rm_col[1];		/* Flexible array of I/O columns */
 } raidz_map_t;
 
+#define	RAIDZ_ORIGINAL_IMPL	(INT_MAX)
+
 extern const raidz_impl_ops_t vdev_raidz_scalar_impl;
 #if defined(__x86_64) && defined(HAVE_SSE2)	/* only x86_64 for now */
 extern const raidz_impl_ops_t vdev_raidz_sse2_impl;

man/man5/zfs-module-parameters.5

@@ -1684,7 +1684,7 @@ Default value: \fB4,096\fR.
 \fBzfs_vdev_raidz_impl\fR (string)
 .ad
 .RS 12n
-Parameter for selecting raidz implementation to use.
+Parameter for selecting raidz parity implementation to use.
 
 Options marked (always) below may be selected on module load as they are
 supported on all systems.

module/zfs/vdev_raidz.c

@@ -458,8 +458,8 @@ vdev_raidz_map_alloc(zio_t *zio, uint64_t unit_shift, uint64_t dcols,
 	zio->io_vsd = rm;
 	zio->io_vsd_ops = &vdev_raidz_vsd_ops;
 
-	/* RAIDZ ops init */
-	vdev_raidz_math_get_ops(rm);
+	/* init RAIDZ parity ops */
+	rm->rm_ops = vdev_raidz_math_get_ops();
 
 	return (rm);
 }
@@ -611,10 +611,9 @@ vdev_raidz_generate_parity_pqr(raidz_map_t *rm)
 void
 vdev_raidz_generate_parity(raidz_map_t *rm)
 {
-	if (rm->rm_ops) {
-		vdev_raidz_math_generate(rm);
+	/* Generate using the new math implementation */
+	if (vdev_raidz_math_generate(rm) != RAIDZ_ORIGINAL_IMPL)
 		return;
-	}
 
 	switch (rm->rm_firstdatacol) {
 	case 1:
@@ -1284,7 +1283,7 @@ vdev_raidz_reconstruct(raidz_map_t *rm, const int *t, int nt)
 {
 	int tgts[VDEV_RAIDZ_MAXPARITY], *dt;
 	int ntgts;
-	int i, c;
+	int i, c, ret;
 	int code;
 	int nbadparity, nbaddata;
 	int parity_valid[VDEV_RAIDZ_MAXPARITY];
@@ -1322,14 +1321,11 @@ vdev_raidz_reconstruct(raidz_map_t *rm, const int *t, int nt)
 
 	dt = &tgts[nbadparity];
 
-	/*
-	 * Reconstruct using the new math implementation if
-	 * rm_ops is set.
-	 */
-	if (rm->rm_ops) {
-		return (vdev_raidz_math_reconstruct(rm, parity_valid, dt,
-		    nbaddata));
-	}
+
+	/* Reconstruct using the new math implementation */
+	ret = vdev_raidz_math_reconstruct(rm, parity_valid, dt, nbaddata);
+	if (ret != RAIDZ_ORIGINAL_IMPL)
+		return (ret);
 
 	/*
 	 * See if we can use any of our optimized reconstruction routines.