| Message ID | 201601060803.AA00000@WIN-5MHF4RKU941.jp.fujitsu.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Wed, Jan 06, 2016 at 05:03:40PM +0900, Tsutomu Itoh wrote: > } else if (strncmp(args[0].from, "no", 2) == 0) { > compress_type = "no"; > btrfs_clear_opt(info->mount_opt, COMPRESS); > btrfs_clear_opt(info->mount_opt, FORCE_COMPRESS); > compress_force = false; > + no_compress++; > } else { ... > + if ((btrfs_test_opt(root, COMPRESS) && > + (info->compress_type != saved_compress_type || > + compress_force != saved_compress_force)) || > + (!btrfs_test_opt(root, COMPRESS) && > + no_compress == 1)) { If there are more than one 'compress=no' then the message won't be printed. I don't see a reason for doing no_compress++ above. > + btrfs_info(root->fs_info, > + "%s %s compression", > + (compress_force) ? "force" : "use", > + compress_type); > + } > + compress_force = false; > break; > case Opt_ssd: > btrfs_set_and_info(root, SSD, -- 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 2016/01/13 21:33, David Sterba wrote: > On Wed, Jan 06, 2016 at 05:03:40PM +0900, Tsutomu Itoh wrote: >> } else if (strncmp(args[0].from, "no", 2) == 0) { >> compress_type = "no"; >> btrfs_clear_opt(info->mount_opt, COMPRESS); >> btrfs_clear_opt(info->mount_opt, FORCE_COMPRESS); >> compress_force = false; >> + no_compress++; >> } else { > ... >> + if ((btrfs_test_opt(root, COMPRESS) && >> + (info->compress_type != saved_compress_type || >> + compress_force != saved_compress_force)) || >> + (!btrfs_test_opt(root, COMPRESS) && >> + no_compress == 1)) { > > If there are more than one 'compress=no' then the message won't be > printed. I don't see a reason for doing no_compress++ above. I want to output message as follows. Therefore, no_compress++ is necessary. # mount -o compress,compress,compress=no,compress=no,compress,compress=no <dev> <path> [ 162.048033] BTRFS: device fsid a7f6e96e-653e-42d0-8469-13025396caa2 devid 1 transid 3 <dev> [ 185.349034] BTRFS info (device <dev>): use zlib compression [ 185.349041] BTRFS info (device <dev>): use no compression [ 185.349045] BTRFS info (device <dev>): use zlib compression [ 185.349048] BTRFS info (device <dev>): use no compression [ 185.349050] BTRFS info (device <dev>): disk space caching is enabled Thanks, Tsutomu > >> + btrfs_info(root->fs_info, >> + "%s %s compression", >> + (compress_force) ? "force" : "use", >> + compress_type); >> + } >> + compress_force = false; >> break; >> case Opt_ssd: >> btrfs_set_and_info(root, SSD, > -- > 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 > -- 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 Thu, Jan 14, 2016 at 09:09:14AM +0900, Tsutomu Itoh wrote: > On 2016/01/13 21:33, David Sterba wrote: > > On Wed, Jan 06, 2016 at 05:03:40PM +0900, Tsutomu Itoh wrote: > >> } else if (strncmp(args[0].from, "no", 2) == 0) { > >> compress_type = "no"; > >> btrfs_clear_opt(info->mount_opt, COMPRESS); > >> btrfs_clear_opt(info->mount_opt, FORCE_COMPRESS); > >> compress_force = false; > >> + no_compress++; > >> } else { > > ... > >> + if ((btrfs_test_opt(root, COMPRESS) && > >> + (info->compress_type != saved_compress_type || > >> + compress_force != saved_compress_force)) || > >> + (!btrfs_test_opt(root, COMPRESS) && > >> + no_compress == 1)) { > > > > If there are more than one 'compress=no' then the message won't be > > printed. I don't see a reason for doing no_compress++ above. > > I want to output message as follows. Therefore, no_compress++ is necessary. > > # mount -o compress,compress,compress=no,compress=no,compress,compress=no <dev> <path> > > [ 162.048033] BTRFS: device fsid a7f6e96e-653e-42d0-8469-13025396caa2 devid 1 transid 3 <dev> > [ 185.349034] BTRFS info (device <dev>): use zlib compression > [ 185.349041] BTRFS info (device <dev>): use no compression > [ 185.349045] BTRFS info (device <dev>): use zlib compression > [ 185.349048] BTRFS info (device <dev>): use no compression > [ 185.349050] BTRFS info (device <dev>): disk space caching is enabled I don't think it's necessary ty print all the options. Only the last one is applied so I'd expect to see only "... use no compression". -- 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 Thu, Jan 14, 2016 at 03:20:43PM +0100, David Sterba wrote: > On Thu, Jan 14, 2016 at 09:09:14AM +0900, Tsutomu Itoh wrote: > > On 2016/01/13 21:33, David Sterba wrote: > > > On Wed, Jan 06, 2016 at 05:03:40PM +0900, Tsutomu Itoh wrote: > > >> } else if (strncmp(args[0].from, "no", 2) == 0) { > > >> compress_type = "no"; > > >> btrfs_clear_opt(info->mount_opt, COMPRESS); > > >> btrfs_clear_opt(info->mount_opt, FORCE_COMPRESS); > > >> compress_force = false; > > >> + no_compress++; > > >> } else { > > > ... > > >> + if ((btrfs_test_opt(root, COMPRESS) && > > >> + (info->compress_type != saved_compress_type || > > >> + compress_force != saved_compress_force)) || > > >> + (!btrfs_test_opt(root, COMPRESS) && > > >> + no_compress == 1)) { > > > > > > If there are more than one 'compress=no' then the message won't be > > > printed. I don't see a reason for doing no_compress++ above. > > > > I want to output message as follows. Therefore, no_compress++ is necessary. > > > > # mount -o compress,compress,compress=no,compress=no,compress,compress=no <dev> <path> > > > > [ 162.048033] BTRFS: device fsid a7f6e96e-653e-42d0-8469-13025396caa2 devid 1 transid 3 <dev> > > [ 185.349034] BTRFS info (device <dev>): use zlib compression > > [ 185.349041] BTRFS info (device <dev>): use no compression > > [ 185.349045] BTRFS info (device <dev>): use zlib compression > > [ 185.349048] BTRFS info (device <dev>): use no compression > > [ 185.349050] BTRFS info (device <dev>): disk space caching is enabled > > I don't think it's necessary ty print all the options. Only the last one > is applied so I'd expect to see only "... use no compression". ... but we do print all the options as we find them already, never mind. -- 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 2016/01/14 23:20, David Sterba wrote: > On Thu, Jan 14, 2016 at 09:09:14AM +0900, Tsutomu Itoh wrote: >> On 2016/01/13 21:33, David Sterba wrote: >>> On Wed, Jan 06, 2016 at 05:03:40PM +0900, Tsutomu Itoh wrote: >>>> } else if (strncmp(args[0].from, "no", 2) == 0) { >>>> compress_type = "no"; >>>> btrfs_clear_opt(info->mount_opt, COMPRESS); >>>> btrfs_clear_opt(info->mount_opt, FORCE_COMPRESS); >>>> compress_force = false; >>>> + no_compress++; >>>> } else { >>> ... >>>> + if ((btrfs_test_opt(root, COMPRESS) && >>>> + (info->compress_type != saved_compress_type || >>>> + compress_force != saved_compress_force)) || >>>> + (!btrfs_test_opt(root, COMPRESS) && >>>> + no_compress == 1)) { >>> >>> If there are more than one 'compress=no' then the message won't be >>> printed. I don't see a reason for doing no_compress++ above. >> >> I want to output message as follows. Therefore, no_compress++ is necessary. >> >> # mount -o compress,compress,compress=no,compress=no,compress,compress=no <dev> <path> >> >> [ 162.048033] BTRFS: device fsid a7f6e96e-653e-42d0-8469-13025396caa2 devid 1 transid 3 <dev> >> [ 185.349034] BTRFS info (device <dev>): use zlib compression >> [ 185.349041] BTRFS info (device <dev>): use no compression >> [ 185.349045] BTRFS info (device <dev>): use zlib compression >> [ 185.349048] BTRFS info (device <dev>): use no compression >> [ 185.349050] BTRFS info (device <dev>): disk space caching is enabled > > I don't think it's necessary ty print all the options. Only the last one > is applied so I'd expect to see only "... use no compression". I see. However, the output of other options are also the same as the above-mentioned. e.g. # mount -o discard,nodiscard,discard <dev> <path> [56714.781948] BTRFS: device fsid 22f3af7e-0f1c-40fe-8eb1-091df7bd1da2 devid 1 transid 3 <dev> [56736.296779] BTRFS info (device <dev>): turning on discard [56736.296785] BTRFS info (device <dev>): turning off discard [56736.296789] BTRFS info (device <dev>): turning on discard [56736.296792] BTRFS info (device <dev>): disk space caching is enabled Thanks, Tsutomu -- 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
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 24154e4..12d04c9 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -381,6 +381,9 @@ int btrfs_parse_options(struct btrfs_root *root, char *options) int ret = 0; char *compress_type; bool compress_force = false; + enum btrfs_compression_type saved_compress_type; + bool saved_compress_force; + int no_compress = 0; cache_gen = btrfs_super_cache_generation(root->fs_info->super_copy); if (cache_gen) @@ -458,6 +461,10 @@ int btrfs_parse_options(struct btrfs_root *root, char *options) /* Fallthrough */ case Opt_compress: case Opt_compress_type: + saved_compress_type = btrfs_test_opt(root, COMPRESS) ? + info->compress_type : BTRFS_COMPRESS_NONE; + saved_compress_force = + btrfs_test_opt(root, FORCE_COMPRESS); if (token == Opt_compress || token == Opt_compress_force || strcmp(args[0].from, "zlib") == 0) { @@ -466,6 +473,7 @@ int btrfs_parse_options(struct btrfs_root *root, char *options) btrfs_set_opt(info->mount_opt, COMPRESS); btrfs_clear_opt(info->mount_opt, NODATACOW); btrfs_clear_opt(info->mount_opt, NODATASUM); + no_compress = 0; } else if (strcmp(args[0].from, "lzo") == 0) { compress_type = "lzo"; info->compress_type = BTRFS_COMPRESS_LZO; @@ -473,25 +481,21 @@ int btrfs_parse_options(struct btrfs_root *root, char *options) btrfs_clear_opt(info->mount_opt, NODATACOW); btrfs_clear_opt(info->mount_opt, NODATASUM); btrfs_set_fs_incompat(info, COMPRESS_LZO); + no_compress = 0; } else if (strncmp(args[0].from, "no", 2) == 0) { compress_type = "no"; btrfs_clear_opt(info->mount_opt, COMPRESS); btrfs_clear_opt(info->mount_opt, FORCE_COMPRESS); compress_force = false; + no_compress++; } else { ret = -EINVAL; goto out; } if (compress_force) { - btrfs_set_and_info(root, FORCE_COMPRESS, - "force %s compression", - compress_type); + btrfs_set_opt(info->mount_opt, FORCE_COMPRESS); } else { - if (!btrfs_test_opt(root, COMPRESS)) - btrfs_info(root->fs_info, - "btrfs: use %s compression", - compress_type); /* * If we remount from compress-force=xxx to * compress=xxx, we need clear FORCE_COMPRESS @@ -500,6 +504,17 @@ int btrfs_parse_options(struct btrfs_root *root, char *options) */ btrfs_clear_opt(info->mount_opt, FORCE_COMPRESS); } + if ((btrfs_test_opt(root, COMPRESS) && + (info->compress_type != saved_compress_type || + compress_force != saved_compress_force)) || + (!btrfs_test_opt(root, COMPRESS) && + no_compress == 1)) { + btrfs_info(root->fs_info, + "%s %s compression", + (compress_force) ? "force" : "use", + compress_type); + } + compress_force = false; break; case Opt_ssd: btrfs_set_and_info(root, SSD,
The compression message might not be correctly output. Fix it. [[before fix]] # mount -o compress /dev/sdb3 /test3 [ 996.874264] BTRFS info (device sdb3): disk space caching is enabled [ 996.874268] BTRFS: has skinny extents # mount | grep /test3 /dev/sdb3 on /test3 type btrfs (rw,relatime,compress=zlib,space_cache,subvolid=5,subvol=/) # mount -o remount,compress-force /dev/sdb3 /test3 [ 1035.075017] BTRFS info (device sdb3): force zlib compression [ 1035.075021] BTRFS info (device sdb3): disk space caching is enabled # mount | grep /test3 /dev/sdb3 on /test3 type btrfs (rw,relatime,compress-force=zlib,space_cache,subvolid=5,subvol=/) # mount -o remount,compress /dev/sdb3 /test3 [ 1053.679092] BTRFS info (device sdb3): disk space caching is enabled # mount | grep /test3 /dev/sdb3 on /test3 type btrfs (rw,relatime,compress=zlib,space_cache,subvolid=5,subvol=/) [[after fix]] # mount -o compress /dev/sdb3 /test3 [ 401.021753] BTRFS info (device sdb3): use zlib compression [ 401.021758] BTRFS info (device sdb3): disk space caching is enabled [ 401.021760] BTRFS: has skinny extents # mount | grep /test3 /dev/sdb3 on /test3 type btrfs (rw,relatime,compress=zlib,space_cache,subvolid=5,subvol=/) # mount -o remount,compress-force /dev/sdb3 /test3 [ 439.824624] BTRFS info (device sdb3): force zlib compression [ 439.824629] BTRFS info (device sdb3): disk space caching is enabled # mount | grep /test3 /dev/sdb3 on /test3 type btrfs (rw,relatime,compress-force=zlib,space_cache,subvolid=5,subvol=/) # mount -o remount,compress /dev/sdb3 /test3 [ 459.918430] BTRFS info (device sdb3): use zlib compression [ 459.918434] BTRFS info (device sdb3): disk space caching is enabled # mount | grep /test3 /dev/sdb3 on /test3 type btrfs (rw,relatime,compress=zlib,space_cache,subvolid=5,subvol=/) Signed-off-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com> --- V1->V2: It is corrected that API doesn't change. fs/btrfs/super.c | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-)