| Message ID | 20171021164901.11529-1-kilobyte@angband.pl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Oct 21, 2017 at 06:49:01PM +0200, Adam Borowski wrote: > Many compressors do assign a meaning to level 0: either null compression or > the lowest possible level. This differs from our "unset thus default". > Thus, let's not unnecessarily confuse users. I agree 'level 0' confusing, however I'd like to keep the level mentioned in the message. We could add #define BTRFS_COMPRESSION_ZLIB_DEFAULT 3 and use it in btrfs_compress_str2level. > > Signed-off-by: Adam Borowski <kilobyte@angband.pl> > --- > fs/btrfs/super.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c > index f9d4522336db..144fabfbd246 100644 > --- a/fs/btrfs/super.c > +++ b/fs/btrfs/super.c > @@ -551,7 +551,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, level %d", > + btrfs_printk(info, info->compress_level ? > + KERN_INFO"%s %s compression, level %d" : > + KERN_INFO"%s %s compression", Please keep using btrfs_info, the KERN_INFO prefix would not work here. btrfs_printk prepends the filesystem description and the message level must be at the beginning. > (compress_force) ? "force" : "use", > compress_type, info->compress_level); > } -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 25, 2017 at 03:23:11PM +0200, David Sterba wrote: > On Sat, Oct 21, 2017 at 06:49:01PM +0200, Adam Borowski wrote: > > Many compressors do assign a meaning to level 0: either null compression or > > the lowest possible level. This differs from our "unset thus default". > > Thus, let's not unnecessarily confuse users. > > I agree 'level 0' confusing, however I'd like to keep the level > mentioned in the message. > > We could add > > #define BTRFS_COMPRESSION_ZLIB_DEFAULT 3 > > and use it in btrfs_compress_str2level. I considered this but every algorithm has a different default, thus we'd need separate cases for zlib vs zstd, while lzo has no settable level at all. Still, this is just some extra lines of code, thus doable. > > Signed-off-by: Adam Borowski <kilobyte@angband.pl> > > --- > > fs/btrfs/super.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c > > index f9d4522336db..144fabfbd246 100644 > > --- a/fs/btrfs/super.c > > +++ b/fs/btrfs/super.c > > @@ -551,7 +551,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, level %d", > > + btrfs_printk(info, info->compress_level ? > > + KERN_INFO"%s %s compression, level %d" : > > + KERN_INFO"%s %s compression", > > Please keep using btrfs_info, the KERN_INFO prefix would not work here. > btrfs_printk prepends the filesystem description and the message level > must be at the beginning. Seems to work for me: [ 14.072575] BTRFS info (device sda1): use lzo compression with identical colors as other info messages next to it. But if we're to expand this code, ternary operators would get too hairy, thus this can go at least for clarity. > > (compress_force) ? "force" : "use", > > compress_type, info->compress_level); > > } >
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index f9d4522336db..144fabfbd246 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -551,7 +551,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, level %d", + btrfs_printk(info, info->compress_level ? + KERN_INFO"%s %s compression, level %d" : + KERN_INFO"%s %s compression", (compress_force) ? "force" : "use", compress_type, info->compress_level); }
Many compressors do assign a meaning to level 0: either null compression or the lowest possible level. This differs from our "unset thus default". Thus, let's not unnecessarily confuse users. Signed-off-by: Adam Borowski <kilobyte@angband.pl> --- fs/btrfs/super.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)