[PATCH 0/4] Minor compression cleanups

[PATCH 0/4] Minor compression cleanups

[David Sterba]

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(-)

-- 
2.15.0

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

[David Sterba]

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

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/compression.c | 15 +++++++++++++++
 fs/btrfs/compression.h |  2 ++
 2 files changed, 17 insertions(+)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 5982c8a71f02..a1a5958ff328 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -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,
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index 6b692903a23c..677fa4aa0bd7 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -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
-- 
2.15.0

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

[David Sterba]

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ioctl.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index e8adebc8c1b0..c41c259d9283 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -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)
-- 
2.15.0

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

[David Sterba]

Use the helper for conversion, keep the semantics.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/props.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
index c39a940d0c75..b30a056963ab 100644
--- a/fs/btrfs/props.c
+++ b/fs/btrfs/props.c
@@ -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;
-- 
2.15.0

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

[David Sterba]

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

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/super.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 3d0813b17e9c..febe67a37f8a 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -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
-- 
2.15.0

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

[Anand Jain]

On 12/01/2017 12:09 AM, David Sterba wrote:
> There are several places opencoding this conversion, add a helper now
> that we have 3 compression algorithms.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>   fs/btrfs/compression.c | 15 +++++++++++++++
>   fs/btrfs/compression.h |  2 ++
>   2 files changed, 17 insertions(+)
> 
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index 5982c8a71f02..a1a5958ff328 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -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


> +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,
> diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
> index 6b692903a23c..677fa4aa0bd7 100644
> --- a/fs/btrfs/compression.h
> +++ b/fs/btrfs/compression.h
> @@ -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
> 

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

[Anand Jain]

On 12/01/2017 12:09 AM, David Sterba wrote:
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>   fs/btrfs/ioctl.c | 10 ++++------
>   1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index e8adebc8c1b0..c41c259d9283 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -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


> +		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)
> 

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

[David Sterba]

On Fri, Dec 01, 2017 at 06:08:53AM +0800, Anand Jain wrote:
> 
> 
> On 12/01/2017 12:09 AM, David Sterba wrote:
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> >   fs/btrfs/ioctl.c | 10 ++++------
> >   1 file changed, 4 insertions(+), 6 deletions(-)
> > 
> > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> > index e8adebc8c1b0..c41c259d9283 100644
> > --- a/fs/btrfs/ioctl.c
> > +++ b/fs/btrfs/ioctl.c
> > @@ -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 ?

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/

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

[David Sterba]

On Fri, Dec 01, 2017 at 05:55:56AM +0800, Anand Jain wrote:
> 
> 
> On 12/01/2017 12:09 AM, David Sterba wrote:
> > There are several places opencoding this conversion, add a helper now
> > that we have 3 compression algorithms.
> > 
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> >   fs/btrfs/compression.c | 15 +++++++++++++++
> >   fs/btrfs/compression.h |  2 ++
> >   2 files changed, 17 insertions(+)
> > 
> > diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> > index 5982c8a71f02..a1a5958ff328 100644
> > --- a/fs/btrfs/compression.c
> > +++ b/fs/btrfs/compression.c
> > @@ -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,

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