zio_compress: use spa_max_ashift as threshold

Now default compression is lz4, which can stop compression process by itself on incompressible data. If there is additional size checks - we will only make our compressratio worse. So we can check only for sector size. Signed-off-by: George Melikov <[email protected]>
openzfs · Apr 17, 2020 · f7523c3 · f7523c3
1 parent a7929f3
commit f7523c3
Show file tree

Hide file tree

Showing 6 changed files with 39 additions and 22 deletions.
diff --git a/include/sys/zio_compress.h b/include/sys/zio_compress.h
@@ -23,6 +23,7 @@
  * Copyright 2009 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  * Copyright (c) 2015, 2016 by Delphix. All rights reserved.
+ * Copyright (c) 2020 by George Melikov. All rights reserved.
  */
 
 #ifndef _SYS_ZIO_COMPRESS_H
@@ -110,7 +111,7 @@ extern int lz4_decompress_zfs(void *src, void *dst, size_t s_len, size_t d_len,
  * Compress and decompress data if necessary.
  */
 extern size_t zio_compress_data(enum zio_compress c, abd_t *src, void *dst,
-    size_t s_len);
+    size_t s_len, int compress_threshold);
 extern int zio_decompress_data(enum zio_compress c, abd_t *src, void *dst,
     size_t s_len, size_t d_len);
 extern int zio_decompress_data_buf(enum zio_compress c, void *src, void *dst,

diff --git a/man/man5/zfs-module-parameters.5 b/man/man5/zfs-module-parameters.5
@@ -408,7 +408,7 @@ Default value: \fB16,777,216\fR (16MB)
 .RS 12n
 If we are not searching forward (due to metaslab_df_max_search,
 metaslab_df_free_pct, or metaslab_df_alloc_threshold), this tunable controls
-what segment is used.  If it is set, we will use the largest free segment. 
+what segment is used.  If it is set, we will use the largest free segment.
 If it is not set, we will use a segment of exactly the requested size (or
 larger).
 .sp
@@ -3103,7 +3103,7 @@ Default value: \fB25\fR.
 \fBzfs_sync_pass_dont_compress\fR (int)
 .ad
 .RS 12n
-Starting in this sync pass, we disable compression (including of metadata). 
+Starting in this sync pass, we disable compression (including of metadata).
 With the default setting, in practice, we don't have this many sync passes,
 so this has no effect.
 .sp

diff --git a/man/man8/zfsprops.8 b/man/man8/zfsprops.8
@@ -854,11 +854,11 @@ is selected, compression will explicitly check for blocks consisting of only
 zeroes (the NUL byte).  When a zero-filled block is detected, it is stored as
 a hole and not compressed using the indicated compression algorithm.
 .Pp
-Any block being compressed must be no larger than 7/8 of its original size
-after compression, otherwise the compression will not be considered worthwhile
-and the block saved uncompressed. Note that when the logical block is less than
-8 times the disk sector size this effectively reduces the necessary compression
-ratio; for example 8k blocks on disks with 4k disk sectors must compress to 1/2
+Block must be compressed by at least max ashift of pool vdevs, otherwise
+the compression will not be considered worthwhile and the block saved
+uncompressed. Note that when the logical block is less than 8 times the disk
+sector size this effectively reduces the necessary compression ratio;
+for example 8k blocks on disks with 4k disk sectors must compress to 1/2
 or less of their original size.
 .It Xo
 .Sy context Ns = Ns Sy none Ns | Ns

diff --git a/module/zfs/arc.c b/module/zfs/arc.c
@@ -26,6 +26,7 @@
  * Copyright (c) 2017, Nexenta Systems, Inc.  All rights reserved.
  * Copyright (c) 2019, loli10K <[email protected]>. All rights reserved.
  * Copyright (c) 2020, George Amanakis. All rights reserved.
+ * Copyright (c) 2020 by George Melikov. All rights reserved.
  */
 
 /*
@@ -1755,7 +1756,7 @@ arc_hdr_authenticate(arc_buf_hdr_t *hdr, spa_t *spa, uint64_t dsobj)
 		abd_take_ownership_of_buf(abd, B_TRUE);
 
 		csize = zio_compress_data(HDR_GET_COMPRESS(hdr),
-		    hdr->b_l1hdr.b_pabd, tmpbuf, lsize);
+		    hdr->b_l1hdr.b_pabd, tmpbuf, lsize, 0);
 		ASSERT3U(csize, <=, psize);
 		abd_zero_off(abd, csize, psize - csize);
 	}
@@ -8531,7 +8532,8 @@ l2arc_apply_transforms(spa_t *spa, arc_buf_hdr_t *hdr, uint64_t asize,
 		cabd = abd_alloc_for_io(asize, ismd);
 		tmp = abd_borrow_buf(cabd, asize);
 
-		psize = zio_compress_data(compress, to_write, tmp, size);
+		psize = zio_compress_data(compress, to_write, tmp, size,
+		    spa->spa_max_ashift);
 		ASSERT3U(psize, <=, HDR_GET_PSIZE(hdr));
 		if (psize < asize)
 			bzero((char *)tmp + psize, asize - psize);
@@ -9901,7 +9903,7 @@ l2arc_log_blk_commit(l2arc_dev_t *dev, zio_t *pio, l2arc_write_callback_t *cb)
 	/* try to compress the buffer */
 	list_insert_tail(&cb->l2wcb_abd_list, abd_buf);
 	psize = zio_compress_data(ZIO_COMPRESS_LZ4,
-	    abd_buf->abd, tmpbuf, sizeof (*lb));
+	    abd_buf->abd, tmpbuf, sizeof (*lb), dev->l2ad_spa->spa_max_ashift);
 
 	/* a log block is never entirely zero */
 	ASSERT(psize != 0);

diff --git a/module/zfs/zio.c b/module/zfs/zio.c
@@ -23,6 +23,7 @@
  * Copyright (c) 2011, 2019 by Delphix. All rights reserved.
  * Copyright (c) 2011 Nexenta Systems, Inc. All rights reserved.
  * Copyright (c) 2017, Intel Corporation.
+ * Copyright (c) 2020 by George Melikov. All rights reserved.
  */
 
 #include <sys/sysmacros.h>
@@ -1676,7 +1677,8 @@ zio_write_compress(zio_t *zio)
 	if (compress != ZIO_COMPRESS_OFF &&
 	    !(zio->io_flags & ZIO_FLAG_RAW_COMPRESS)) {
 		void *cbuf = zio_buf_alloc(lsize);
-		psize = zio_compress_data(compress, zio->io_abd, cbuf, lsize);
+		psize = zio_compress_data(compress, zio->io_abd, cbuf, lsize,
+		    spa->spa_max_ashift);
 		if (psize == 0 || psize == lsize) {
 			compress = ZIO_COMPRESS_OFF;
 			zio_buf_free(cbuf, lsize);
@@ -1739,7 +1741,7 @@ zio_write_compress(zio_t *zio)
 		 * to a hole.
 		 */
 		psize = zio_compress_data(ZIO_COMPRESS_EMPTY,
-		    zio->io_abd, NULL, lsize);
+		    zio->io_abd, NULL, lsize, 0);
 		if (psize == 0)
 			compress = ZIO_COMPRESS_OFF;
 	} else {

diff --git a/module/zfs/zio_compress.c b/module/zfs/zio_compress.c
@@ -22,13 +22,9 @@
 /*
  * Copyright 2009 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
- */
-/*
  * Copyright (c) 2013 by Saso Kiselkov. All rights reserved.
- */
-
-/*
  * Copyright (c) 2013, 2018 by Delphix. All rights reserved.
+ * Copyright (c) 2020 by George Melikov. All rights reserved.
  */
 
 #include <sys/zfs_context.h>
@@ -102,7 +98,8 @@ zio_compress_zeroed_cb(void *data, size_t len, void *private)
 }
 
 size_t
-zio_compress_data(enum zio_compress c, abd_t *src, void *dst, size_t s_len)
+zio_compress_data(enum zio_compress c, abd_t *src, void *dst, size_t s_len,
+    int compress_threshold)
 {
 	size_t c_len, d_len;
 	zio_compress_info_t *ci = &zio_compress_table[c];
@@ -120,8 +117,24 @@ zio_compress_data(enum zio_compress c, abd_t *src, void *dst, size_t s_len)
 	if (c == ZIO_COMPRESS_EMPTY)
 		return (s_len);
 
-	/* Compress at least 12.5% */
-	d_len = s_len - (s_len >> 3);
+	/*
+	 * If data was compressed and passed ratio checks before -
+	 * we can skip these checks for reproducible results,
+	 * so set compress_threshold=0 for reproducibility of ARC checks.
+	 */
+	if (compress_threshold > 0) {
+		/* Data is already less or equal to compress threshold */
+		if (s_len <= compress_threshold)
+			return (s_len);
+
+		/*
+		 * Write compressed only if there is at least
+		 * one sector compressed
+		 */
+		d_len = s_len - compress_threshold;
+	} else {
+		d_len = s_len;
+	}
 
 	/* No compression algorithms can read from ABDs directly */
 	void *tmp = abd_borrow_buf_copy(src, s_len);
@@ -131,7 +144,6 @@ zio_compress_data(enum zio_compress c, abd_t *src, void *dst, size_t s_len)
 	if (c_len > d_len)
 		return (s_len);
 
-	ASSERT3U(c_len, <=, d_len);
 	return (c_len);
 }