Resolve QAT issues with incompressible data

Currently, when ZFS wants to accelerate compression with QAT, it
passes a destination buffer of the same size as the source buffer.
Unfortunately, if the data is incompressible, QAT can actually
"compress" the data to be larger than the source buffer. When this
happens, the QAT driver will return a FAILED error code and print
warnings to dmesg. This patch fixes these issues by providing the
QAT driver with an additional buffer to work with so that even
completely incompressible source data will not cause an overflow.

This patch also resolves an error handling issue where
incompressible data attempts compression twice: once by QAT and
once in software. To fix this issue, a new (and fake) error code
CPA_STATUS_INOMPRESSIBLE has been added so that the calling code
can correctly account for the difference between a hardware
failure and data that simply cannot be compressed.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Weigang Li <[email protected]>
Signed-off-by: Tom Caputi <[email protected]>
Closes #7338 

3 files changed, 106 insertions, 37 deletions

diff --git a/module/zfs/gzip.c b/module/zfs/gzip.c
index 6e4db718c..40166b3fe 100644
--- a/module/zfs/gzip.c
+++ b/module/zfs/gzip.c
@@ -53,16 +53,25 @@ typedef uLongf zlen_t;
 size_t
 gzip_compress(void *s_start, void *d_start, size_t s_len, size_t d_len, int n)
 {
+	int ret;
 	zlen_t dstlen = d_len;
 
 	ASSERT(d_len <= s_len);
 
 	/* check if hardware accelerator can be used */
 	if (qat_dc_use_accel(s_len)) {
-		if (qat_compress(QAT_COMPRESS, s_start,
-		    s_len, d_start, d_len, &dstlen) == CPA_STATUS_SUCCESS)
+		ret = qat_compress(QAT_COMPRESS, s_start, s_len, d_start,
+		    d_len, &dstlen);
+		if (ret == CPA_STATUS_SUCCESS) {
 			return ((size_t)dstlen);
-		/* if hardware compress fail, do it again with software */
+		} else if (ret == CPA_STATUS_INCOMPRESSIBLE) {
+			if (d_len != s_len)
+				return (s_len);
+
+			bcopy(s_start, d_start, s_len);
+			return (s_len);
+		}
+		/* if hardware compression fails, do it again with software */
 	}
 
 	if (compress_func(d_start, &dstlen, s_start, s_len, n) != Z_OK) {
diff --git a/module/zfs/qat.h b/module/zfs/qat.h
index dc8825de2..b2cd5a9c1 100644
--- a/module/zfs/qat.h
+++ b/module/zfs/qat.h
@@ -172,6 +172,9 @@ extern void qat_crypt_fini(void);
 extern int qat_init(void);
 extern void qat_fini(void);
 
+/* fake CpaStatus used to indicate data was not compressible */
+#define	CPA_STATUS_INCOMPRESSIBLE				(-127)
+
 extern boolean_t qat_dc_use_accel(size_t s_len);
 extern boolean_t qat_crypt_use_accel(size_t s_len);
 extern boolean_t qat_checksum_use_accel(size_t s_len);
@@ -184,6 +187,7 @@ extern int qat_checksum(uint64_t cksum, uint8_t *buf, uint64_t size,
     zio_cksum_t *zcp);
 #else
 #define	CPA_STATUS_SUCCESS					0
+#define	CPA_STATUS_INCOMPRESSIBLE				(-127)
 #define	qat_init()
 #define	qat_fini()
 #define	qat_dc_use_accel(s_len)					0
diff --git a/module/zfs/qat_compress.c b/module/zfs/qat_compress.c
index fff0751fb..115316297 100644
--- a/module/zfs/qat_compress.c
+++ b/module/zfs/qat_compress.c
@@ -25,6 +25,7 @@
 #include <linux/pagemap.h>
 #include <linux/completion.h>
 #include <sys/zfs_context.h>
+#include <sys/zio.h>
 #include "qat.h"
 
 /*
@@ -224,9 +225,16 @@ qat_dc_fini(void)
 	qat_dc_clean();
 }
 
-int
-qat_compress(qat_compress_dir_t dir, char *src, int src_len,
-    char *dst, int dst_len, size_t *c_len)
+/*
+ * The "add" parameter is an additional buffer which is passed
+ * to QAT as a scratch buffer alongside the destination buffer
+ * in case the "compressed" data ends up being larger than the
+ * original source data. This is necessary to prevent QAT from
+ * generating buffer overflow warnings for incompressible data.
+ */
+static int
+qat_compress_impl(qat_compress_dir_t dir, char *src, int src_len,
+    char *dst, int dst_len, char *add, int add_len, size_t *c_len)
 {
 	CpaInstanceHandle dc_inst_handle;
 	CpaDcSessionHandle session_handle;
@@ -243,14 +251,16 @@ qat_compress(qat_compress_dir_t dir, char *src, int src_len,
 	Cpa32U compressed_sz;
 	Cpa32U num_src_buf = (src_len >> PAGE_SHIFT) + 2;
 	Cpa32U num_dst_buf = (dst_len >> PAGE_SHIFT) + 2;
+	Cpa32U num_add_buf = (add_len >> PAGE_SHIFT) + 2;
 	Cpa32U bytes_left;
+	Cpa32U dst_pages = 0;
 	char *data;
-	struct page *in_page, *out_page;
+	struct page *page;
 	struct page **in_pages = NULL;
 	struct page **out_pages = NULL;
+	struct page **add_pages = NULL;
 	Cpa32U page_off = 0;
 	struct completion complete;
-	size_t ret = -1;
 	Cpa32U page_num = 0;
 	Cpa16U i;
 
@@ -262,7 +272,7 @@ qat_compress(qat_compress_dir_t dir, char *src, int src_len,
 	Cpa32U src_buffer_list_mem_size = sizeof (CpaBufferList) +
 	    (num_src_buf * sizeof (CpaFlatBuffer));
 	Cpa32U dst_buffer_list_mem_size = sizeof (CpaBufferList) +
-	    (num_dst_buf * sizeof (CpaFlatBuffer));
+	    ((num_dst_buf + num_add_buf) * sizeof (CpaFlatBuffer));
 
 	if (QAT_PHYS_CONTIG_ALLOC(&in_pages,
 	    num_src_buf * sizeof (struct page *)) != CPA_STATUS_SUCCESS)
@@ -272,6 +282,10 @@ qat_compress(qat_compress_dir_t dir, char *src, int src_len,
 	    num_dst_buf * sizeof (struct page *)) != CPA_STATUS_SUCCESS)
 		goto fail;
 
+	if (QAT_PHYS_CONTIG_ALLOC(&add_pages,
+	    num_add_buf * sizeof (struct page *)) != CPA_STATUS_SUCCESS)
+		goto fail;
+
 	i = atomic_inc_32_nv(&inst_num) % num_inst;
 	dc_inst_handle = dc_inst_handles[i];
 	session_handle = session_handles[i];
@@ -282,7 +296,7 @@ qat_compress(qat_compress_dir_t dir, char *src, int src_len,
 	    CPA_STATUS_SUCCESS)
 		goto fail;
 
-	cpaDcBufferListGetMetaSize(dc_inst_handle, num_dst_buf,
+	cpaDcBufferListGetMetaSize(dc_inst_handle, num_dst_buf + num_add_buf,
 	    &buffer_meta_size);
 	if (QAT_PHYS_CONTIG_ALLOC(&buffer_meta_dst, buffer_meta_size) !=
 	    CPA_STATUS_SUCCESS)
@@ -313,9 +327,9 @@ qat_compress(qat_compress_dir_t dir, char *src, int src_len,
 	page_num = 0;
 	while (bytes_left > 0) {
 		page_off = ((long)data & ~PAGE_MASK);
-		in_page = qat_mem_to_page(data);
-		in_pages[page_num] = in_page;
-		flat_buf_src->pData = kmap(in_page) + page_off;
+		page = qat_mem_to_page(data);
+		in_pages[page_num] = page;
+		flat_buf_src->pData = kmap(page) + page_off;
 		flat_buf_src->dataLenInBytes =
 		    min((long)PAGE_SIZE - page_off, (long)bytes_left);
 
@@ -333,9 +347,29 @@ qat_compress(qat_compress_dir_t dir, char *src, int src_len,
 	page_num = 0;
 	while (bytes_left > 0) {
 		page_off = ((long)data & ~PAGE_MASK);
-		out_page = qat_mem_to_page(data);
-		flat_buf_dst->pData = kmap(out_page) + page_off;
-		out_pages[page_num] = out_page;
+		page = qat_mem_to_page(data);
+		flat_buf_dst->pData = kmap(page) + page_off;
+		out_pages[page_num] = page;
+		flat_buf_dst->dataLenInBytes =
+		    min((long)PAGE_SIZE - page_off, (long)bytes_left);
+
+		bytes_left -= flat_buf_dst->dataLenInBytes;
+		data += flat_buf_dst->dataLenInBytes;
+		flat_buf_dst++;
+		buf_list_dst->numBuffers++;
+		page_num++;
+		dst_pages++;
+	}
+
+	/* map additional scratch pages into the destination buffer list */
+	bytes_left = add_len;
+	data = add;
+	page_num = 0;
+	while (bytes_left > 0) {
+		page_off = ((long)data & ~PAGE_MASK);
+		page = qat_mem_to_page(data);
+		flat_buf_dst->pData = kmap(page) + page_off;
+		add_pages[page_num] = page;
 		flat_buf_dst->dataLenInBytes =
 		    min((long)PAGE_SIZE - page_off, (long)bytes_left);
 
@@ -379,6 +413,7 @@ qat_compress(qat_compress_dir_t dir, char *src, int src_len,
 
 		compressed_sz = dc_results.produced;
 		if (compressed_sz + hdr_sz + ZLIB_FOOT_SZ > dst_len) {
+			status = CPA_STATUS_INCOMPRESSIBLE;
 			goto fail;
 		}
 
@@ -388,8 +423,10 @@ qat_compress(qat_compress_dir_t dir, char *src, int src_len,
 
 		/* no space for gzip footer in the last page */
 		if (((compressed_sz + hdr_sz) % PAGE_SIZE)
-		    + ZLIB_FOOT_SZ > PAGE_SIZE)
+		    + ZLIB_FOOT_SZ > PAGE_SIZE) {
+			status = CPA_STATUS_INCOMPRESSIBLE;
 			goto fail;
+		}
 
 		/* jump to the end of the buffer and append footer */
 		flat_buf_dst->pData =
@@ -400,16 +437,11 @@ qat_compress(qat_compress_dir_t dir, char *src, int src_len,
 		dc_results.produced = 0;
 		status = cpaDcGenerateFooter(session_handle,
 		    flat_buf_dst, &dc_results);
-		if (status != CPA_STATUS_SUCCESS) {
+		if (status != CPA_STATUS_SUCCESS)
 			goto fail;
-		}
 
 		*c_len = compressed_sz + dc_results.produced + hdr_sz;
-
 		QAT_STAT_INCR(comp_total_out_bytes, *c_len);
-
-		ret = 0;
-
 	} else {
 		ASSERT3U(dir, ==, QAT_DECOMPRESS);
 		QAT_STAT_BUMP(decomp_requests);
@@ -417,12 +449,8 @@ qat_compress(qat_compress_dir_t dir, char *src, int src_len,
 
 		buf_list_src->pBuffers->pData += ZLIB_HEAD_SZ;
 		buf_list_src->pBuffers->dataLenInBytes -= ZLIB_HEAD_SZ;
-		status = cpaDcDecompressData(dc_inst_handle,
-		    session_handle,
-		    buf_list_src,
-		    buf_list_dst,
-		    &dc_results,
-		    CPA_DC_FLUSH_FINAL,
+		status = cpaDcDecompressData(dc_inst_handle, session_handle,
+		    buf_list_src, buf_list_dst, &dc_results, CPA_DC_FLUSH_FINAL,
 		    &complete);
 
 		if (CPA_STATUS_SUCCESS != status) {
@@ -443,16 +471,12 @@ qat_compress(qat_compress_dir_t dir, char *src, int src_len,
 		}
 
 		*c_len = dc_results.produced;
-
 		QAT_STAT_INCR(decomp_total_out_bytes, *c_len);
-
-		ret = 0;
 	}
 
 fail:
-	if (status != CPA_STATUS_SUCCESS) {
+	if (status != CPA_STATUS_SUCCESS && status != CPA_STATUS_INCOMPRESSIBLE)
 		QAT_STAT_BUMP(dc_fails);
-	}
 
 	if (in_pages) {
 		for (page_num = 0;
@@ -464,19 +488,51 @@ fail:
 	}
 
 	if (out_pages) {
-		for (page_num = 0;
-		    page_num < buf_list_dst->numBuffers;
-		    page_num++) {
+		for (page_num = 0; page_num < dst_pages; page_num++) {
 			kunmap(out_pages[page_num]);
 		}
 		QAT_PHYS_CONTIG_FREE(out_pages);
 	}
 
+	if (add_pages) {
+		for (page_num = 0;
+		    page_num < buf_list_dst->numBuffers - dst_pages;
+		    page_num++) {
+			kunmap(add_pages[page_num]);
+		}
+		QAT_PHYS_CONTIG_FREE(add_pages);
+	}
+
 	QAT_PHYS_CONTIG_FREE(buffer_meta_src);
 	QAT_PHYS_CONTIG_FREE(buffer_meta_dst);
 	QAT_PHYS_CONTIG_FREE(buf_list_src);
 	QAT_PHYS_CONTIG_FREE(buf_list_dst);
 
+	return (status);
+}
+
+/*
+ * Entry point for QAT accelerated compression / decompression.
+ */
+int
+qat_compress(qat_compress_dir_t dir, char *src, int src_len,
+    char *dst, int dst_len, size_t *c_len)
+{
+	int ret;
+	size_t add_len = 0;
+	void *add = NULL;
+
+	if (dir == QAT_COMPRESS) {
+		add_len = dst_len;
+		add = zio_data_buf_alloc(add_len);
+	}
+
+	ret = qat_compress_impl(dir, src, src_len, dst,
+	    dst_len, add, add_len, c_len);
+
+	if (dir == QAT_COMPRESS)
+		zio_data_buf_free(add, add_len);
+
 	return (ret);
 }