linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC 0/3]: settable compression level for zstd
@ 2017-09-15 15:34 Adam Borowski
  2017-09-15 15:36 ` [RFC PATCH 1/3] btrfs: allow to set compression level for zlib Adam Borowski
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Adam Borowski @ 2017-09-15 15:34 UTC (permalink / raw)
  To: linux-btrfs, Nick Terrell, David Sterba

Hi!
Here's a patch set that allows changing the compression level for zstd,
currently at mount time only.  I've played with it for a month, so despite
being a quick hack, it's reasonably well tested.  Tested on 4.13 +
btrfs-for-4.14 only, though -- I've booted 11th-day-of-merge-window only an
hour ago on one machine, no explosions yet.

As a quick hack, it doesn't conserve memory as it should: all workspace
allocations assume level 15 and waste space otherwise.

Because of an (easily changeable) quirk of compression level encoding, the
max is set at 15, but I guess higher levels are pointless for 128KB blocks. 
Nick and co can tell us more -- for me zstd is mostly a black box so it's
you who knows anything about tuning it.

There are three patches:
* [David Sterba] btrfs: allow to set compression level for zlib
  Unmodified version of the patch from Jul 24, I'm re-sending it for
  convenience.
* btrfs: allow setting zlib compression level via :9
  Some bikeshedding: it looks like Chris Mason also favours zlib:9 over
  zlib9 as the former is more readable.  If you disagree... well, it's up
  to you to decide anyway.  This patch accepts both syntaxes.
* btrfs: allow setting zstd level


Meow!
-- 
⢀⣴⠾⠻⢶⣦⠀ I've read an article about how lively happy music boosts
⣾⠁⢰⠒⠀⣿⡁ productivity.  You can read it, too, you just need the
⢿⡄⠘⠷⠚⠋⠀ right music while doing so.  I recommend Skepticism
⠈⠳⣄⠀⠀⠀⠀ (funeral doom metal).

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [RFC PATCH 1/3] btrfs: allow to set compression level for zlib
  2017-09-15 15:34 [RFC 0/3]: settable compression level for zstd Adam Borowski
@ 2017-09-15 15:36 ` Adam Borowski
  2017-09-15 15:36   ` [RFC PATCH 2/3] btrfs: allow setting zlib compression level via :9 Adam Borowski
  2017-09-15 15:36   ` [RFC PATCH 3/3] btrfs: allow setting zstd level Adam Borowski
  2017-09-15 17:14 ` [RFC 0/3]: settable compression level for zstd Austin S. Hemmelgarn
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 7+ messages in thread
From: Adam Borowski @ 2017-09-15 15:36 UTC (permalink / raw)
  To: linux-btrfs, Nick Terrell, David Sterba; +Cc: David Sterba

From: David Sterba <dsterba@suse.com>

Preliminary support for setting compression level for zlib, the
following works:

$ mount -o compess=zlib                 # default
$ mount -o compess=zlib0                # same
$ mount -o compess=zlib9                # level 9, slower sync, less data
$ mount -o compess=zlib1                # level 1, faster sync, more data
$ mount -o remount,compress=zlib3	# level set by remount

The level is visible in the same format in /proc/mounts. Level set via
file property does not work yet.

Required patch: "btrfs: prepare for extensions in compression options"

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/compression.c | 20 +++++++++++++++++++-
 fs/btrfs/compression.h |  6 +++++-
 fs/btrfs/ctree.h       |  1 +
 fs/btrfs/inode.c       |  5 ++++-
 fs/btrfs/lzo.c         |  5 +++++
 fs/btrfs/super.c       |  7 +++++--
 fs/btrfs/zlib.c        | 12 +++++++++++-
 7 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index b51d23f5cafa..70a50194fcf5 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -867,6 +867,11 @@ static void free_workspaces(void)
  * Given an address space and start and length, compress the bytes into @pages
  * that are allocated on demand.
  *
+ * @type_level is encoded algorithm and level, where level 0 means whatever
+ * default the algorithm chooses and is opaque here;
+ * - compression algo are 0-3
+ * - the level are bits 4-7
+ *
  * @out_pages is an in/out parameter, holds maximum number of pages to allocate
  * and returns number of actually allocated pages
  *
@@ -881,7 +886,7 @@ static void free_workspaces(void)
  * @max_out tells us the max number of bytes that we're allowed to
  * stuff into pages
  */
-int btrfs_compress_pages(int type, struct address_space *mapping,
+int btrfs_compress_pages(unsigned int type_level, struct address_space *mapping,
 			 u64 start, struct page **pages,
 			 unsigned long *out_pages,
 			 unsigned long *total_in,
@@ -889,9 +894,11 @@ int btrfs_compress_pages(int type, struct address_space *mapping,
 {
 	struct list_head *workspace;
 	int ret;
+	int type = type_level & 0xF;
 
 	workspace = find_workspace(type);
 
+	btrfs_compress_op[type - 1]->set_level(workspace, type_level);
 	ret = btrfs_compress_op[type-1]->compress_pages(workspace, mapping,
 						      start, pages,
 						      out_pages,
@@ -1081,3 +1088,14 @@ int btrfs_compress_heuristic(struct inode *inode, u64 start, u64 end)
 
 	return ret;
 }
+
+unsigned int btrfs_compress_str2level(const char *str)
+{
+	if (strncmp(str, "zlib", 4) != 0)
+		return 0;
+
+	if ('1' <= str[4] && str[4] <= '9' )
+		return str[4] - '0';
+
+	return 0;
+}
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index d2781ff8f994..da20755ebf21 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -76,7 +76,7 @@ struct compressed_bio {
 void btrfs_init_compress(void);
 void btrfs_exit_compress(void);
 
-int btrfs_compress_pages(int type, struct address_space *mapping,
+int btrfs_compress_pages(unsigned int type_level, struct address_space *mapping,
 			 u64 start, struct page **pages,
 			 unsigned long *out_pages,
 			 unsigned long *total_in,
@@ -95,6 +95,8 @@ blk_status_t btrfs_submit_compressed_write(struct inode *inode, u64 start,
 blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
 				 int mirror_num, unsigned long bio_flags);
 
+unsigned btrfs_compress_str2level(const char *str);
+
 enum btrfs_compression_type {
 	BTRFS_COMPRESS_NONE  = 0,
 	BTRFS_COMPRESS_ZLIB  = 1,
@@ -124,6 +126,8 @@ struct btrfs_compress_op {
 			  struct page *dest_page,
 			  unsigned long start_byte,
 			  size_t srclen, size_t destlen);
+
+	void (*set_level)(struct list_head *ws, unsigned int type);
 };
 
 extern const struct btrfs_compress_op btrfs_zlib_compress;
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 5a8933da39a7..dd07a7ef234c 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -791,6 +791,7 @@ struct btrfs_fs_info {
 	 */
 	unsigned long pending_changes;
 	unsigned long compress_type:4;
+	unsigned int compress_level;
 	int commit_interval;
 	/*
 	 * It is a suggestive number, the read side is safe even it gets a
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 128f3e58634f..28201b924575 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -530,7 +530,10 @@ static noinline void compress_file_range(struct inode *inode,
 		 */
 		extent_range_clear_dirty_for_io(inode, start, end);
 		redirty = 1;
-		ret = btrfs_compress_pages(compress_type,
+
+		/* Compression level is applied here and only here */
+		ret = btrfs_compress_pages(
+			compress_type | (fs_info->compress_level << 4),
 					   inode->i_mapping, start,
 					   pages,
 					   &nr_pages,
diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c
index d433e75d489a..6c7f18cd3b61 100644
--- a/fs/btrfs/lzo.c
+++ b/fs/btrfs/lzo.c
@@ -430,10 +430,15 @@ static int lzo_decompress(struct list_head *ws, unsigned char *data_in,
 	return ret;
 }
 
+static void lzo_set_level(struct list_head *ws, unsigned int type)
+{
+}
+
 const struct btrfs_compress_op btrfs_lzo_compress = {
 	.alloc_workspace	= lzo_alloc_workspace,
 	.free_workspace		= lzo_free_workspace,
 	.compress_pages		= lzo_compress_pages,
 	.decompress_bio		= lzo_decompress_bio,
 	.decompress		= lzo_decompress,
+	.set_level		= lzo_set_level,
 };
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 35a128acfbd1..5467187701ef 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -502,6 +502,7 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
 			    strncmp(args[0].from, "zlib", 4) == 0) {
 				compress_type = "zlib";
 				info->compress_type = BTRFS_COMPRESS_ZLIB;
+				info->compress_level = btrfs_compress_str2level(args[0].from);
 				btrfs_set_opt(info->mount_opt, COMPRESS);
 				btrfs_clear_opt(info->mount_opt, NODATACOW);
 				btrfs_clear_opt(info->mount_opt, NODATASUM);
@@ -549,9 +550,9 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
 			      compress_force != saved_compress_force)) ||
 			    (!btrfs_test_opt(info, COMPRESS) &&
 			     no_compress == 1)) {
-				btrfs_info(info, "%s %s compression",
+				btrfs_info(info, "%s %s compression, level %d",
 					   (compress_force) ? "force" : "use",
-					   compress_type);
+					   compress_type, info->compress_level);
 			}
 			compress_force = false;
 			break;
@@ -1246,6 +1247,8 @@ static int btrfs_show_options(struct seq_file *seq, struct dentry *dentry)
 			seq_printf(seq, ",compress-force=%s", compress_type);
 		else
 			seq_printf(seq, ",compress=%s", compress_type);
+		if (info->compress_level)
+			seq_printf(seq, "%d", info->compress_level);
 	}
 	if (btrfs_test_opt(info, NOSSD))
 		seq_puts(seq, ",nossd");
diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
index c248f9286366..c890032e680f 100644
--- a/fs/btrfs/zlib.c
+++ b/fs/btrfs/zlib.c
@@ -37,6 +37,7 @@ struct workspace {
 	z_stream strm;
 	char *buf;
 	struct list_head list;
+	int level;
 };
 
 static void zlib_free_workspace(struct list_head *ws)
@@ -96,7 +97,7 @@ static int zlib_compress_pages(struct list_head *ws,
 	*total_out = 0;
 	*total_in = 0;
 
-	if (Z_OK != zlib_deflateInit(&workspace->strm, 3)) {
+	if (Z_OK != zlib_deflateInit(&workspace->strm, workspace->level)) {
 		pr_warn("BTRFS: deflateInit failed\n");
 		ret = -EIO;
 		goto out;
@@ -402,10 +403,19 @@ static int zlib_decompress(struct list_head *ws, unsigned char *data_in,
 	return ret;
 }
 
+static void zlib_set_level(struct list_head *ws, unsigned int type)
+{
+	struct workspace *workspace = list_entry(ws, struct workspace, list);
+	unsigned level = (type & 0xF0) >> 4;
+
+	workspace->level = level > 0 ? level : 3;
+}
+
 const struct btrfs_compress_op btrfs_zlib_compress = {
 	.alloc_workspace	= zlib_alloc_workspace,
 	.free_workspace		= zlib_free_workspace,
 	.compress_pages		= zlib_compress_pages,
 	.decompress_bio		= zlib_decompress_bio,
 	.decompress		= zlib_decompress,
+	.set_level              = zlib_set_level,
 };
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [RFC PATCH 2/3] btrfs: allow setting zlib compression level via :9
  2017-09-15 15:36 ` [RFC PATCH 1/3] btrfs: allow to set compression level for zlib Adam Borowski
@ 2017-09-15 15:36   ` Adam Borowski
  2017-09-15 15:36   ` [RFC PATCH 3/3] btrfs: allow setting zstd level Adam Borowski
  1 sibling, 0 replies; 7+ messages in thread
From: Adam Borowski @ 2017-09-15 15:36 UTC (permalink / raw)
  To: linux-btrfs, Nick Terrell, David Sterba; +Cc: Adam Borowski

This is bikeshedding, but it seems people are drastically more likely to
understand "zlib:9" as compression level rather than an algorithm version
compared to "zlib9".

Signed-off-by: Adam Borowski <kilobyte@angband.pl>
---
 fs/btrfs/compression.c | 2 ++
 fs/btrfs/super.c       | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 70a50194fcf5..71782ec976c7 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -1096,6 +1096,8 @@ unsigned int btrfs_compress_str2level(const char *str)
 
 	if ('1' <= str[4] && str[4] <= '9' )
 		return str[4] - '0';
+	if (str[4] == ':' && '1' <= str[5] && str[5] <= '9')
+		return str[5] - '0';
 
 	return 0;
 }
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 5467187701ef..537e04120457 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1248,7 +1248,7 @@ static int btrfs_show_options(struct seq_file *seq, struct dentry *dentry)
 		else
 			seq_printf(seq, ",compress=%s", compress_type);
 		if (info->compress_level)
-			seq_printf(seq, "%d", info->compress_level);
+			seq_printf(seq, ":%d", info->compress_level);
 	}
 	if (btrfs_test_opt(info, NOSSD))
 		seq_puts(seq, ",nossd");
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [RFC PATCH 3/3] btrfs: allow setting zstd level
  2017-09-15 15:36 ` [RFC PATCH 1/3] btrfs: allow to set compression level for zlib Adam Borowski
  2017-09-15 15:36   ` [RFC PATCH 2/3] btrfs: allow setting zlib compression level via :9 Adam Borowski
@ 2017-09-15 15:36   ` Adam Borowski
  1 sibling, 0 replies; 7+ messages in thread
From: Adam Borowski @ 2017-09-15 15:36 UTC (permalink / raw)
  To: linux-btrfs, Nick Terrell, David Sterba; +Cc: Adam Borowski

Capped at 15 because of currently used encoding, which is also a reasonable
limit because highest levels shine only on blocks much bigger than btrfs'
128KB.

Memory is allocated for the biggest supported level rather than for
what is actually used.

Signed-off-by: Adam Borowski <kilobyte@angband.pl>
---
 fs/btrfs/compression.c | 21 +++++++++++++++------
 fs/btrfs/props.c       |  2 +-
 fs/btrfs/super.c       |  3 ++-
 fs/btrfs/zstd.c        | 24 +++++++++++++++++++-----
 4 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 71782ec976c7..2d4337756fef 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -1091,13 +1091,22 @@ int btrfs_compress_heuristic(struct inode *inode, u64 start, u64 end)
 
 unsigned int btrfs_compress_str2level(const char *str)
 {
-	if (strncmp(str, "zlib", 4) != 0)
+	long level;
+	int max;
+
+	if (strncmp(str, "zlib", 4) == 0)
+		max = 9;
+	else if (strncmp(str, "zstd", 4) == 0)
+		max = 15; // encoded on 4 bits, real max is 22
+	else
 		return 0;
 
-	if ('1' <= str[4] && str[4] <= '9' )
-		return str[4] - '0';
-	if (str[4] == ':' && '1' <= str[5] && str[5] <= '9')
-		return str[5] - '0';
+	str += 4;
+	if (*str == ':')
+		str++;
 
-	return 0;
+	if (kstrtoul(str, 10, &level))
+		return 0;
+
+	return (level > max) ? 0 : level;
 }
diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
index f6a05f836629..2e35aa2b2d79 100644
--- a/fs/btrfs/props.c
+++ b/fs/btrfs/props.c
@@ -414,7 +414,7 @@ static int prop_compression_apply(struct inode *inode,
 		type = BTRFS_COMPRESS_LZO;
 	else if (!strncmp("zlib", value, 4))
 		type = BTRFS_COMPRESS_ZLIB;
-	else if (!strncmp("zstd", value, len))
+	else if (!strncmp("zstd", value, 4))
 		type = BTRFS_COMPRESS_ZSTD;
 	else
 		return -EINVAL;
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 537e04120457..f9d4522336db 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -515,9 +515,10 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
 				btrfs_clear_opt(info->mount_opt, NODATASUM);
 				btrfs_set_fs_incompat(info, COMPRESS_LZO);
 				no_compress = 0;
-			} else if (strcmp(args[0].from, "zstd") == 0) {
+			} else if (strncmp(args[0].from, "zstd", 4) == 0) {
 				compress_type = "zstd";
 				info->compress_type = BTRFS_COMPRESS_ZSTD;
+				info->compress_level = btrfs_compress_str2level(args[0].from);
 				btrfs_set_opt(info->mount_opt, COMPRESS);
 				btrfs_clear_opt(info->mount_opt, NODATACOW);
 				btrfs_clear_opt(info->mount_opt, NODATASUM);
diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c
index 607ce47b483a..99e11cb2d60e 100644
--- a/fs/btrfs/zstd.c
+++ b/fs/btrfs/zstd.c
@@ -26,11 +26,14 @@
 #define ZSTD_BTRFS_MAX_WINDOWLOG 17
 #define ZSTD_BTRFS_MAX_INPUT (1 << ZSTD_BTRFS_MAX_WINDOWLOG)
 #define ZSTD_BTRFS_DEFAULT_LEVEL 3
+// Max supported by the algorithm is 22, but gains for small blocks (128KB)
+// are limited, thus we cap at 15.
+#define ZSTD_BTRFS_MAX_LEVEL 15
 
-static ZSTD_parameters zstd_get_btrfs_parameters(size_t src_len)
+static ZSTD_parameters zstd_get_btrfs_parameters(size_t src_len, int level)
 {
-	ZSTD_parameters params = ZSTD_getParams(ZSTD_BTRFS_DEFAULT_LEVEL,
-						src_len, 0);
+	ZSTD_parameters params = ZSTD_getParams(level, src_len, 0);
+	BUG_ON(level > ZSTD_maxCLevel());
 
 	if (params.cParams.windowLog > ZSTD_BTRFS_MAX_WINDOWLOG)
 		params.cParams.windowLog = ZSTD_BTRFS_MAX_WINDOWLOG;
@@ -43,6 +46,7 @@ struct workspace {
 	size_t size;
 	char *buf;
 	struct list_head list;
+	int level;
 };
 
 static void zstd_free_workspace(struct list_head *ws)
@@ -57,7 +61,8 @@ static void zstd_free_workspace(struct list_head *ws)
 static struct list_head *zstd_alloc_workspace(void)
 {
 	ZSTD_parameters params =
-			zstd_get_btrfs_parameters(ZSTD_BTRFS_MAX_INPUT);
+			zstd_get_btrfs_parameters(ZSTD_BTRFS_MAX_INPUT,
+						  ZSTD_BTRFS_MAX_LEVEL);
 	struct workspace *workspace;
 
 	workspace = kzalloc(sizeof(*workspace), GFP_KERNEL);
@@ -101,7 +106,7 @@ static int zstd_compress_pages(struct list_head *ws,
 	unsigned long len = *total_out;
 	const unsigned long nr_dest_pages = *out_pages;
 	unsigned long max_out = nr_dest_pages * PAGE_SIZE;
-	ZSTD_parameters params = zstd_get_btrfs_parameters(len);
+	ZSTD_parameters params = zstd_get_btrfs_parameters(len, workspace->level);
 
 	*out_pages = 0;
 	*total_out = 0;
@@ -423,10 +428,19 @@ static int zstd_decompress(struct list_head *ws, unsigned char *data_in,
 	return ret;
 }
 
+static void zstd_set_level(struct list_head *ws, unsigned int type)
+{
+	struct workspace *workspace = list_entry(ws, struct workspace, list);
+	unsigned level = (type & 0xF0) >> 4;
+
+	workspace->level = level > 0 ? level : ZSTD_BTRFS_DEFAULT_LEVEL;
+}
+
 const struct btrfs_compress_op btrfs_zstd_compress = {
 	.alloc_workspace = zstd_alloc_workspace,
 	.free_workspace = zstd_free_workspace,
 	.compress_pages = zstd_compress_pages,
 	.decompress_bio = zstd_decompress_bio,
 	.decompress = zstd_decompress,
+	.set_level = zstd_set_level,
 };
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [RFC 0/3]: settable compression level for zstd
  2017-09-15 15:34 [RFC 0/3]: settable compression level for zstd Adam Borowski
  2017-09-15 15:36 ` [RFC PATCH 1/3] btrfs: allow to set compression level for zlib Adam Borowski
@ 2017-09-15 17:14 ` Austin S. Hemmelgarn
  2017-09-25 16:22 ` David Sterba
  2017-10-19 14:05 ` David Sterba
  3 siblings, 0 replies; 7+ messages in thread
From: Austin S. Hemmelgarn @ 2017-09-15 17:14 UTC (permalink / raw)
  To: Adam Borowski, linux-btrfs, Nick Terrell, David Sterba

On 2017-09-15 11:34, Adam Borowski wrote:
> Hi!
> Here's a patch set that allows changing the compression level for zstd,
> currently at mount time only.  I've played with it for a month, so despite
> being a quick hack, it's reasonably well tested.  Tested on 4.13 +
> btrfs-for-4.14 only, though -- I've booted 11th-day-of-merge-window only an
> hour ago on one machine, no explosions yet.
> 
> As a quick hack, it doesn't conserve memory as it should: all workspace
> allocations assume level 15 and waste space otherwise.
> 
> Because of an (easily changeable) quirk of compression level encoding, the
> max is set at 15, but I guess higher levels are pointless for 128KB blocks.
> Nick and co can tell us more -- for me zstd is mostly a black box so it's
> you who knows anything about tuning it.
I've got limited knowledge of the zstandard algorithm, but based on my 
(probably incomplete and possibly inaccurate) analysis of the code in 
the kernel, I think this assessment is correct.  At a minimum, higher 
levels are likely to provide no benefit considering how slow things get 
(even the speed of level 15 means it's probably not going to be used much).
> 
> There are three patches:
> * [David Sterba] btrfs: allow to set compression level for zlib
>    Unmodified version of the patch from Jul 24, I'm re-sending it for
>    convenience.
> * btrfs: allow setting zlib compression level via :9
>    Some bikeshedding: it looks like Chris Mason also favours zlib:9 over
>    zlib9 as the former is more readable.  If you disagree... well, it's up
>    to you to decide anyway.  This patch accepts both syntaxes.
On this in particular, I think there _might_ be a potential issue with 
option parsing, I'd advocate something less likely to be a metacharacter 
(like '-') instead of ':', but I do agree that it's a bit more concise 
when the algorithm and level are separate.
> * btrfs: allow setting zstd level


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC 0/3]: settable compression level for zstd
  2017-09-15 15:34 [RFC 0/3]: settable compression level for zstd Adam Borowski
  2017-09-15 15:36 ` [RFC PATCH 1/3] btrfs: allow to set compression level for zlib Adam Borowski
  2017-09-15 17:14 ` [RFC 0/3]: settable compression level for zstd Austin S. Hemmelgarn
@ 2017-09-25 16:22 ` David Sterba
  2017-10-19 14:05 ` David Sterba
  3 siblings, 0 replies; 7+ messages in thread
From: David Sterba @ 2017-09-25 16:22 UTC (permalink / raw)
  To: Adam Borowski; +Cc: linux-btrfs, Nick Terrell, David Sterba

On Fri, Sep 15, 2017 at 05:34:57PM +0200, Adam Borowski wrote:
> Hi!
> Here's a patch set that allows changing the compression level for zstd,
> currently at mount time only.  I've played with it for a month, so despite
> being a quick hack, it's reasonably well tested.  Tested on 4.13 +
> btrfs-for-4.14 only, though -- I've booted 11th-day-of-merge-window only an
> hour ago on one machine, no explosions yet.
> 
> As a quick hack, it doesn't conserve memory as it should: all workspace
> allocations assume level 15 and waste space otherwise.
> 
> Because of an (easily changeable) quirk of compression level encoding, the
> max is set at 15, but I guess higher levels are pointless for 128KB blocks. 
> Nick and co can tell us more -- for me zstd is mostly a black box so it's
> you who knows anything about tuning it.
> 
> There are three patches:
> * [David Sterba] btrfs: allow to set compression level for zlib
>   Unmodified version of the patch from Jul 24, I'm re-sending it for
>   convenience.
> * btrfs: allow setting zlib compression level via :9
>   Some bikeshedding: it looks like Chris Mason also favours zlib:9 over
>   zlib9 as the former is more readable.  If you disagree... well, it's up
>   to you to decide anyway.  This patch accepts both syntaxes.
> * btrfs: allow setting zstd level

I'll add the patches to for-next as it does not affect default behaviour
and the changes are contained in compression. Further updates are fine,
though I think we'll have to resolve the workspace waste before the
patches can be merged to master.

I tend agree to add the separator and slightly prefer ':' over '-'. In
order to utilize the zstd at higher levels, we can enhance the
compressed chunk size and then we could use another ':' to separate that
from the rest.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC 0/3]: settable compression level for zstd
  2017-09-15 15:34 [RFC 0/3]: settable compression level for zstd Adam Borowski
                   ` (2 preceding siblings ...)
  2017-09-25 16:22 ` David Sterba
@ 2017-10-19 14:05 ` David Sterba
  3 siblings, 0 replies; 7+ messages in thread
From: David Sterba @ 2017-10-19 14:05 UTC (permalink / raw)
  To: Adam Borowski; +Cc: linux-btrfs, Nick Terrell, David Sterba

On Fri, Sep 15, 2017 at 05:34:57PM +0200, Adam Borowski wrote:
> Hi!
> Here's a patch set that allows changing the compression level for zstd,
> currently at mount time only.  I've played with it for a month, so despite
> being a quick hack, it's reasonably well tested.  Tested on 4.13 +
> btrfs-for-4.14 only, though -- I've booted 11th-day-of-merge-window only an
> hour ago on one machine, no explosions yet.
> 
> As a quick hack, it doesn't conserve memory as it should: all workspace
> allocations assume level 15 and waste space otherwise.
> 
> Because of an (easily changeable) quirk of compression level encoding, the
> max is set at 15, but I guess higher levels are pointless for 128KB blocks. 
> Nick and co can tell us more -- for me zstd is mostly a black box so it's
> you who knows anything about tuning it.
> 
> There are three patches:
> * [David Sterba] btrfs: allow to set compression level for zlib
>   Unmodified version of the patch from Jul 24, I'm re-sending it for
>   convenience.
> * btrfs: allow setting zlib compression level via :9
>   Some bikeshedding: it looks like Chris Mason also favours zlib:9 over
>   zlib9 as the former is more readable.  If you disagree... well, it's up
>   to you to decide anyway.  This patch accepts both syntaxes.

FYI, I'm going to add the patches 1 and 2 to 4.15 pull, ie. there will
be support for zlib levels.

Final syntax of the level specification is with ":". I've update the
patches, dropping the "zlib9" way, plus some more changelog updates.

> * btrfs: allow setting zstd level

Before the zstd levels are supported, we'll have to update the workspace
allocation.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-10-19 14:07 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-15 15:34 [RFC 0/3]: settable compression level for zstd Adam Borowski
2017-09-15 15:36 ` [RFC PATCH 1/3] btrfs: allow to set compression level for zlib Adam Borowski
2017-09-15 15:36   ` [RFC PATCH 2/3] btrfs: allow setting zlib compression level via :9 Adam Borowski
2017-09-15 15:36   ` [RFC PATCH 3/3] btrfs: allow setting zstd level Adam Borowski
2017-09-15 17:14 ` [RFC 0/3]: settable compression level for zstd Austin S. Hemmelgarn
2017-09-25 16:22 ` David Sterba
2017-10-19 14:05 ` David Sterba

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).