Make it cleaner

Refactor the logic as a tiny wrapper.

(Also a nit that snuck in on the lz4_zfs.c move somehow.)

Signed-off-by: Rich Ercolani <rincebrain@gmail.com>

include/sys/zstd/zstd.h

@@ -90,6 +90,8 @@ void zstd_fini(void);
 
 size_t zfs_zstd_compress(void *s_start, void *d_start, size_t s_len,
     size_t d_len, int level);
+size_t zfs_zstd_compress_wrap(void *s_start, void *d_start, size_t s_len,
+    size_t d_len, int level);
 int zfs_zstd_get_level(void *s_start, size_t s_len, uint8_t *level);
 int zfs_zstd_decompress_level(void *s_start, void *d_start, size_t s_len,
     size_t d_len, uint8_t *level);

module/zcommon/lz4_zfs.c

@@ -883,4 +883,3 @@ EXPORT_SYMBOL(lz4_compress_zfs);
 EXPORT_SYMBOL(lz4_init);
 EXPORT_SYMBOL(lz4_fini);
 EXPORT_SYMBOL(lz4_decompress_zfs);
-

module/zstd/zfs_zstd.c

@@ -50,9 +50,9 @@
 #include "lib/zstd.h"
 #include "lib/common/zstd_errors.h"
 
-int zstd_lz4_pass = 1;
-int zstd_firstpass_level = ZIO_ZSTD_LEVEL_1;
-unsigned int zstd_abort_size = 131072;
+static int zstd_lz4_pass = 1;
+static int zstd_firstpass_level = ZIO_ZSTD_LEVEL_1;
+static unsigned int zstd_abort_size = 131072;
 
 kstat_t *zstd_ksp = NULL;
 
@@ -381,34 +381,20 @@ zstd_enum_to_level(enum zio_zstd_levels level, int16_t *zstd_level)
 }
 
 
-/* Compress block using zstd */
 size_t
-zfs_zstd_compress(void *s_start, void *d_start, size_t s_len, size_t d_len,
+zfs_zstd_compress_wrap(void *s_start, void *d_start, size_t s_len, size_t d_len,
     int level)
 {
-	size_t c_len;
 	int16_t zstd_level;
-	zfs_zstdhdr_t *hdr;
-	ZSTD_CCtx *cctx;
-	size_t pass_len = 1; /* Default to > 0 to avoid tripping ifs below */
-
-	hdr = (zfs_zstdhdr_t *)d_start;
-
-	/* Skip compression if the specified level is invalid */
 	if (zstd_enum_to_level(level, &zstd_level)) {
 		ZSTDSTAT_BUMP(zstd_stat_com_inval);
 		return (s_len);
 	}
-
-	ASSERT3U(d_len, >=, sizeof (*hdr));
-	ASSERT3U(d_len, <=, s_len);
-	ASSERT3U(zstd_level, !=, 0);
-
 	/*
 	 * A zstd early abort heuristic.
 	 *
-	 * - Zeroth, if this is <= zstd-3, or <= zstd_abort_size (currently 128k),
-	 *   don't try any of this, just go.
+	 * - Zeroth, if this is <= zstd-3, or < zstd_abort_size (currently
+	 *   128k), don't try any of this, just go.
 	 *   (because experimentally that was a reasonable cutoff for a perf win
 	 *   with tiny ratio change)
 	 * - First, we try LZ4 compression, and if it doesn't early abort, we
@@ -424,61 +410,43 @@ zfs_zstd_compress(void *s_start, void *d_start, size_t s_len, size_t d_len,
 	 * worse indications on their own than LZ4, and don't improve the LZ4
 	 * pass noticably if stacked like this.
 	 */
-
 	if (zstd_lz4_pass > 0 && zstd_level > 3 && s_len >= zstd_abort_size) {
-		pass_len = lz4_compress_zfs(s_start, hdr, s_len, d_len, 0);
-			if (pass_len < d_len)
-				goto keep_trying;
-		cctx = ZSTD_createCCtx_advanced(zstd_malloc);
-		/*
-		 * Out of kernel memory, gently fall through -
-		 * this will disable compression in
-		 * zio_compress_data
-		 */
-		if (!cctx) {
-			ZSTDSTAT_BUMP(zstd_stat_com_alloc_fail);
+		int pass_len = 1;
+		pass_len = lz4_compress_zfs(s_start, d_start, s_len, d_len, 0);
+		if (pass_len < d_len)
+			goto keep_trying;
+		pass_len = zfs_zstd_compress(s_start, d_start, s_len, d_len,
+		    zstd_firstpass_level);
+		if (pass_len == s_len || pass_len <= 0 || pass_len > d_len)
 			return (s_len);
-		}
-
-		/* Set the compression level */
-		ZSTD_CCtx_setParameter(cctx, ZSTD_c_compressionLevel,
-		    zstd_enum_to_level(zstd_firstpass_level,
-		    &zstd_level));
+	}
+keep_trying:
+	return (zfs_zstd_compress(s_start, d_start, s_len, d_len, level));
 
-		/*
-		 * Use the "magicless" zstd header which saves us 4
-		 * header bytes
-		 */
-		ZSTD_CCtx_setParameter(cctx, ZSTD_c_format,
-		    ZSTD_f_zstd1_magicless);
+}
 
-		/*
-		 * Disable redundant checksum calculation and content
-		 * size storage since this is already done by ZFS
-		 * itself.
-		 */
-		ZSTD_CCtx_setParameter(cctx, ZSTD_c_checksumFlag, 0);
-		ZSTD_CCtx_setParameter(cctx, ZSTD_c_contentSizeFlag, 0);
+/* Compress block using zstd */
+size_t
+zfs_zstd_compress(void *s_start, void *d_start, size_t s_len, size_t d_len,
+    int level)
+{
+	size_t c_len;
+	int16_t zstd_level;
+	zfs_zstdhdr_t *hdr;
+	ZSTD_CCtx *cctx;
 
+	hdr = (zfs_zstdhdr_t *)d_start;
 
-		pass_len = ZSTD_compress2(cctx,
-		    hdr,
-		    d_len,
-		    s_start, s_len);
+	/* Skip compression if the specified level is invalid */
+	if (zstd_enum_to_level(level, &zstd_level)) {
+		ZSTDSTAT_BUMP(zstd_stat_com_inval);
+		return (s_len);
+	}
 
-		ZSTD_freeCCtx(cctx);
+	ASSERT3U(d_len, >=, sizeof (*hdr));
+	ASSERT3U(d_len, <=, s_len);
+	ASSERT3U(zstd_level, !=, 0);
 
-		/*
-		 * Error in the compression routine, disable
-		 * compression.
-		 */
-		if (ZSTD_isError(pass_len)) {
-			return (s_len);
-		}
-		if (pass_len <= 0 || pass_len >= d_len)
-			return (s_len);
-	}
-keep_trying:
 	cctx = ZSTD_createCCtx_advanced(zstd_malloc);
 
 	/*
@@ -503,7 +471,6 @@ zfs_zstd_compress(void *s_start, void *d_start, size_t s_len, size_t d_len,
 	ZSTD_CCtx_setParameter(cctx, ZSTD_c_checksumFlag, 0);
 	ZSTD_CCtx_setParameter(cctx, ZSTD_c_contentSizeFlag, 0);
 
-
 	c_len = ZSTD_compress2(cctx,
 	    hdr->data,
 	    d_len - sizeof (*hdr),
@@ -862,7 +829,7 @@ module_exit(zstd_fini);
 
 ZFS_MODULE_DESCRIPTION("ZSTD Compression for ZFS");
 ZFS_MODULE_LICENSE("Dual BSD/GPL");
-ZFS_MODULE_VERSION(ZSTD_VERSION_STRING "a");
+ZFS_MODULE_VERSION(ZSTD_VERSION_STRING "b");
 
 ZFS_MODULE_PARAM(zfs, zstd_, lz4_pass, INT, ZMOD_RW,
 	"Try an LZ4 pass to determine whether we should early abort.");