Skip to contentSkip to conversation

Minor compression cleanups

PATCH In review David Sterba 3 participants 9 messages Nov 30, 2017 – Dec 4, 2017 Btrfs file system

lore
Share & tools
lore

David Sterba

author maintainer
#0 lore raw

cover Minor compression cleanups

Factor opencoded switches for compression algos to helpers.

David Sterba (4):
btrfs: compression: add helper for type to string conversion
btrfs: SETFLAGS ioctl: use helper for compression type conversion
btrfs: prop: use common helper for type to string conversion
btrfs: show options: use helper to convert compression type string

 fs/btrfs/compression.c | 15 +++++++++++++++
 fs/btrfs/compression.h |  2 ++
 fs/btrfs/ioctl.c       | 10 ++++------
 fs/btrfs/props.c       |  6 +++---
 fs/btrfs/super.c       |  9 ++-------
 5 files changed, 26 insertions(+), 16 deletions(-)
Signature · 1 line
2.15.0
author maintainer
#1 lore raw

PATCH 1/4 btrfs: compression: add helper for type to string conversion

There are several places opencoding this conversion, add a helper now that we have 3 compression algorithms.

Signed-off-by David Sterba <[email protected]>
---
 fs/btrfs/compression.c | 15 +++++++++++++++
 fs/btrfs/compression.h |  2 ++
 2 files changed, 17 insertions(+)
fs/btrfs/compression.c +15 -0
index 5982c8a71f02..a1a5958ff328 100644
@@ -45,6 +45,21 @@
 #include "extent_io.h"
 #include "extent_map.h"
 
+static const char* const btrfs_compress_types[] = { "", "lzo", "zlib", "zstd" };
+
+const char* btrfs_compress_type2str(enum btrfs_compression_type type)
+{
+	switch (type) {
+	case BTRFS_COMPRESS_LZO:
+	case BTRFS_COMPRESS_ZLIB:
+	case BTRFS_COMPRESS_ZSTD:
+	case BTRFS_COMPRESS_NONE:
+		return btrfs_compress_types[type];
+	}
+
+	return NULL;
+}
+
 static int btrfs_decompress_bio(struct compressed_bio *cb);
 
 static inline int compressed_bio_size(struct btrfs_fs_info *fs_info,
fs/btrfs/compression.h +2 -0
index 6b692903a23c..677fa4aa0bd7 100644
@@ -137,6 +137,8 @@ extern const struct btrfs_compress_op btrfs_zlib_compress;
 extern const struct btrfs_compress_op btrfs_lzo_compress;
 extern const struct btrfs_compress_op btrfs_zstd_compress;
 
+const char* btrfs_compress_type2str(enum btrfs_compression_type type);
+
 int btrfs_compress_heuristic(struct inode *inode, u64 start, u64 end);
 
 #endif
Signature · 1 line
2.15.0
author maintainer
#4 lore raw

PATCH 2/4 btrfs: SETFLAGS ioctl: use helper for compression type conversion

In reply to David Sterba
Signed-off-by David Sterba <[email protected]>
---
 fs/btrfs/ioctl.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
fs/btrfs/ioctl.c +4 -6
index e8adebc8c1b0..c41c259d9283 100644
@@ -307,12 +307,10 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
 		ip->flags |= BTRFS_INODE_COMPRESS;
 		ip->flags &= ~BTRFS_INODE_NOCOMPRESS;
 
-		if (fs_info->compress_type == BTRFS_COMPRESS_LZO)
-			comp = "lzo";
-		else if (fs_info->compress_type == BTRFS_COMPRESS_ZLIB)
-			comp = "zlib";
-		else
-			comp = "zstd";
+		comp = btrfs_compress_type2str(fs_info->compress_type);
+		if (!comp || comp[0] == 0)
+			comp = btrfs_compress_type2str(BTRFS_COMPRESS_ZLIB);
+
 		ret = btrfs_set_prop(inode, "btrfs.compression",
 				     comp, strlen(comp), 0);
 		if (ret)
Signature · 1 line
2.15.0
author maintainer
#7 lore raw

PATCH 3/4 btrfs: prop: use common helper for type to string conversion

In reply to David Sterba

Use the helper for conversion, keep the semantics.

Signed-off-by David Sterba <[email protected]>
---
 fs/btrfs/props.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
fs/btrfs/props.c +3 -3
index c39a940d0c75..b30a056963ab 100644
@@ -423,11 +423,11 @@ static const char *prop_compression_extract(struct inode *inode)
 {
 	switch (BTRFS_I(inode)->prop_compress) {
 	case BTRFS_COMPRESS_ZLIB:
-		return "zlib";
 	case BTRFS_COMPRESS_LZO:
-		return "lzo";
 	case BTRFS_COMPRESS_ZSTD:
-		return "zstd";
+		return btrfs_compress_type2str(BTRFS_I(inode)->prop_compress);
+	default:
+		break;
 	}
 
 	return NULL;
Signature · 1 line
2.15.0
author maintainer
#8 lore raw

PATCH 4/4 btrfs: show options: use helper to convert compression type string

In reply to David Sterba

Use the helper, if the COMPRESS option is set, the result is always defined and not empty.

Signed-off-by David Sterba <[email protected]>
---
 fs/btrfs/super.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)
fs/btrfs/super.c +2 -7
index 3d0813b17e9c..febe67a37f8a 100644
@@ -1243,7 +1243,7 @@ int btrfs_sync_fs(struct super_block *sb, int wait)
 static int btrfs_show_options(struct seq_file *seq, struct dentry *dentry)
 {
 	struct btrfs_fs_info *info = btrfs_sb(dentry->d_sb);
-	char *compress_type;
+	const char *compress_type;
 
 	if (btrfs_test_opt(info, DEGRADED))
 		seq_puts(seq, ",degraded");

@@ -1259,12 +1259,7 @@ static int btrfs_show_options(struct seq_file *seq, struct dentry *dentry)
 					     num_online_cpus() + 2, 8))
 		seq_printf(seq, ",thread_pool=%d", info->thread_pool_size);
 	if (btrfs_test_opt(info, COMPRESS)) {
-		if (info->compress_type == BTRFS_COMPRESS_ZLIB)
-			compress_type = "zlib";
-		else if (info->compress_type == BTRFS_COMPRESS_LZO)
-			compress_type = "lzo";
-		else
-			compress_type = "zstd";
+		compress_type = btrfs_compress_type2str(info->compress_type);
 		if (btrfs_test_opt(info, FORCE_COMPRESS))
 			seq_printf(seq, ",compress-force=%s", compress_type);
 		else
Signature · 1 line
2.15.0
#2 lore raw

Re: [PATCH 1/4] btrfs: compression: add helper for type to string conversion

In reply to David Sterba
Quoted 14 lines · David Sterba

There are several places opencoding this conversion, add a helper now that we have 3 compression algorithms.

Signed-off-by David Sterba <[email protected]>
---
  fs/btrfs/compression.c | 15 +++++++++++++++
  fs/btrfs/compression.h |  2 ++
  2 files changed, 17 insertions(+)
fs/btrfs/compression.c +0 -0
index 5982c8a71f02..a1a5958ff328 100644

fs/btrfs/compression.c line 48

Quoted 4 lines
fs/btrfs/compression.c +1 -0
@@ -45,6 +45,21 @@
  #include "extent_io.h"
  #include "extent_map.h"
 
+static const char* const btrfs_compress_types[] = { "", "lzo", "zlib", "zstd" };

This is wrong order.

compression.h:	BTRFS_COMPRESS_NONE  = 0,
compression.h:	BTRFS_COMPRESS_ZLIB  = 1,
compression.h:	BTRFS_COMPRESS_LZO   = 2,
compression.h:	BTRFS_COMPRESS_ZSTD  = 3,

compression.h: BTRFS_COMPRESS_TYPES = 3,

Thanks, Anand

Quoted 30 lines
fs/btrfs/compression.c +13 -0
+const char* btrfs_compress_type2str(enum btrfs_compression_type type)
+{
+	switch (type) {
+	case BTRFS_COMPRESS_LZO:
+	case BTRFS_COMPRESS_ZLIB:
+	case BTRFS_COMPRESS_ZSTD:
+	case BTRFS_COMPRESS_NONE:
+		return btrfs_compress_types[type];
+	}
+
+	return NULL;
+}
+
  static int btrfs_decompress_bio(struct compressed_bio *cb);
 
  static inline int compressed_bio_size(struct btrfs_fs_info *fs_info,
fs/btrfs/compression.h +2 -0
index 6b692903a23c..677fa4aa0bd7 100644
@@ -137,6 +137,8 @@ extern const struct btrfs_compress_op btrfs_zlib_compress;
  extern const struct btrfs_compress_op btrfs_lzo_compress;
  extern const struct btrfs_compress_op btrfs_zstd_compress;
 
+const char* btrfs_compress_type2str(enum btrfs_compression_type type);
+
  int btrfs_compress_heuristic(struct inode *inode, u64 start, u64 end);
 
  #endif
#5 lore raw

Re: [PATCH 2/4] btrfs: SETFLAGS ioctl: use helper for compression type conversion

In reply to David Sterba
Quoted 10 lines · David Sterba
Signed-off-by David Sterba <[email protected]>
---
  fs/btrfs/ioctl.c | 10 ++++------
  1 file changed, 4 insertions(+), 6 deletions(-)
fs/btrfs/ioctl.c +0 -0
index e8adebc8c1b0..c41c259d9283 100644

fs/btrfs/ioctl.c static int btrfs_ioctl_setflags(struct file *file, void __user *arg) · line 315

Quoted 9 lines
fs/btrfs/ioctl.c +0 -6
@@ -307,12 +307,10 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
  		ip->flags |= BTRFS_INODE_COMPRESS;
  		ip->flags &= ~BTRFS_INODE_NOCOMPRESS;
 
-		if (fs_info->compress_type == BTRFS_COMPRESS_LZO)
-			comp = "lzo";
-		else if (fs_info->compress_type == BTRFS_COMPRESS_ZLIB)
-			comp = "zlib";
-		else
-			comp = "zstd";

Bit deviating from the stuff here. Looks like zstd changed the default compression algo to zstd (from zlib). I remember seeing your comment on that too which I thought is fixed, but now I can't locate. Shouldn't this be a bug ?

Thanks, Anand

Quoted 8 lines
fs/btrfs/ioctl.c +4 -0
+		comp = btrfs_compress_type2str(fs_info->compress_type);
+		if (!comp || comp[0] == 0)
+			comp = btrfs_compress_type2str(BTRFS_COMPRESS_ZLIB);
+
  		ret = btrfs_set_prop(inode, "btrfs.compression",
  				     comp, strlen(comp), 0);
  		if (ret)
#6 lore raw

Re: [PATCH 2/4] btrfs: SETFLAGS ioctl: use helper for compression type conversion

Quoted 16 lines · Anand Jain
Quoted 13 lines · David Sterba
Signed-off-by David Sterba <[email protected]>
---
  fs/btrfs/ioctl.c | 10 ++++------
  1 file changed, 4 insertions(+), 6 deletions(-)
fs/btrfs/ioctl.c +0 -0
index e8adebc8c1b0..c41c259d9283 100644
@@ -307,12 +307,10 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
  		ip->flags |= BTRFS_INODE_COMPRESS;
  		ip->flags &= ~BTRFS_INODE_NOCOMPRESS;
Quoted 11 lines

fs/btrfs/ioctl.c static int btrfs_ioctl_setflags(struct file *file, void __user *arg) · line 315

Quoted 6 lines
fs/btrfs/ioctl.c +0 -6
-		if (fs_info->compress_type == BTRFS_COMPRESS_LZO)
-			comp = "lzo";
-		else if (fs_info->compress_type == BTRFS_COMPRESS_ZLIB)
-			comp = "zlib";
-		else
-			comp = "zstd";

Bit deviating from the stuff here. Looks like zstd changed the default compression algo to zstd (from zlib). I remember seeing your comment on that too which I thought is fixed, but now I can't locate. Shouldn't this be a bug ?

It's not a bug, zlib is still the default, but proving that needs more code for the context.

https://patchwork.kernel.org/patch/10034957/

#3 lore raw

Re: [PATCH 1/4] btrfs: compression: add helper for type to string conversion

In reply to Anand Jain
Quoted 24 lines · Anand Jain
Quoted 14 lines · David Sterba

There are several places opencoding this conversion, add a helper now that we have 3 compression algorithms.

Signed-off-by David Sterba <[email protected]>
---
  fs/btrfs/compression.c | 15 +++++++++++++++
  fs/btrfs/compression.h |  2 ++
  2 files changed, 17 insertions(+)
fs/btrfs/compression.c +0 -0
index 5982c8a71f02..a1a5958ff328 100644

fs/btrfs/compression.c line 48

Quoted 4 lines
fs/btrfs/compression.c +1 -0
@@ -45,6 +45,21 @@
  #include "extent_io.h"
  #include "extent_map.h"
 
+static const char* const btrfs_compress_types[] = { "", "lzo", "zlib", "zstd" };

This is wrong order.

Quoted 5 lines
compression.h:	BTRFS_COMPRESS_NONE  = 0,
compression.h:	BTRFS_COMPRESS_ZLIB  = 1,
compression.h:	BTRFS_COMPRESS_LZO   = 2,
compression.h:	BTRFS_COMPRESS_ZSTD  = 3,

compression.h: BTRFS_COMPRESS_TYPES = 3,

This was unintentionally left as an exercise for reviewers. Will fix, thanks.