All of lore.kernel.org
 help / color / mirror / Atom feed
From: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
To: "linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>
Subject: [PATCH 5/5] btrfs: rework compression related options processing
Date: Thu, 18 Sep 2014 17:43:37 +0900	[thread overview]
Message-ID: <541A9B39.7040202@jp.fujitsu.com> (raw)
In-Reply-To: <541A9717.5000003@jp.fujitsu.com>

From: Naohiro Aota <naota@elisp.net>

There are mutual exclusive mount options corresponding to
compression options. If we set these options at once, we get
improper messages and states, and a verbose message.

 * Improper states
  - We can't disable "compress forcing" with compress={zlib,lzo}
    mount option.

    ====
    # mount -o remount,compress-force=zlib,compress=lzo
    [58852.210209] BTRFS info (device vda2): force zlib compression
    (We should see "use lzo compression" here)
    # mount | grep btrfs
    /dev/vda2 on / type btrfs (rw,relatime,seclabel,compress-force=lzo,space_cache)
    (We should also change "compress-force=lzo" to "compress=lzo")
    ====

 * Improper messages
  - We don't see compression enabled message when we enables it
    after disabling it.

    ====
    # mount -o remount,compress=no,compress=zlib; dmesg -c
    [ 4077.130625] BTRFS info (device vda2): btrfs: use no compression
    (We should see "use zlib compression" here)
    ====

  - We don't see any message when we change compression type.

    ====
    # mount -o remount,compress=zlib,compress=lzo
    (Since "BTRFS info" is at the beginning of this message, "btrfs: " is verbose
     and we don't need it.)
    ====

  - We don't see datacow and/or datasum enabling message when we
    enable compression.

    ====
    # mount -o remount,nodatacow,nodatasum,compress
    [58913.704536] BTRFS info (device vda2): setting nodatacow
    (We should see "use zlib compression, setting datacow and datasum" here)
    ====

    ====
    # mount -o remount,nodatasum,compress
    [58950.529392] BTRFS info (device vda2): setting nodatasum
    (We should see "use zlib compression, setting datasum" here)
    ====

 * A verbose message
  - We see verbose "btrfs: " prefix on disabling compression.

    ====
    # mount -o remount,compress=zlib,compress=no
    [58790.727788] BTRFS info (device vda2): btrfs: use no compression
    ("btrfs: " is verbose here. We don't need it, since we already
    state this is btrfs with "BTRFS info")
    ====

This commit solves all these problems.

I separated the code in the following two parts:

  a) Parse the options.
  b) Set and clear flags of mount_opt and show proper messages.

I've confirmed that this patch doesn't cause any regression
by running the following script, which tests all combinations
of corresponding mount options (compress, compress-force,
datacow, and datasum).

=====================
#!/bin/bash

BTRFS_ROOT=/

states=(
  compress=zlib,datacow,datasum
  compress=lzo,datacow,datasum
  compress-force=zlib,datacow,datasum
  compress-force=lzo,datacow,datasum
  compress=no,nodatacow,nodatasum
  compress=no,datacow,nodatasum
  compress=no,datacow,datasum
)

options=(
  compress
  compress=zlib
  compress=lzo
  compress=no
  compress=badopt
  compress-force
  compress-force=zlib
  compress-force=lzo
  compress-force=no
  compress-force=badopt
  nodatacow
  datacow
  nodatasum
  datasum
)

for s in ${states[@]}; do
  for o in ${options[@]}; do
    # set initial state
    echo initial: mount -o remount,$s
    mount -o remount,$s ${BTRFS_ROOT}
    # mount with each option
    echo mount -o remount,$o
    dmesg -C
    mount -o remount,$o ${BTRFS_ROOT}
    echo dmesg:
    dmesg -t | perl -ne 'if(/^BTRFS info \(device .+?\): (.+)$/){print "\t$1\n"}'
    echo -n 'mount options: '
    grep -v rootfs /proc/mounts | \
      awk "{if(\$2==\"${BTRFS_ROOT}\"){print \$4}}" | \
      perl -ne '@a=split(/,/); for(@a){print "$_ " if(/^compress(-force)?=|nodata(cow|sum)$/)}'
    echo
    echo
  done
done
=====================

Signed-off-by: Naohiro Aota <naota@elisp.net>
Signed-off-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
---
 fs/btrfs/super.c | 57 +++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 40 insertions(+), 17 deletions(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 03131c5..83433e15 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -401,6 +401,7 @@ int btrfs_parse_options(struct btrfs_root *root, char *options)
 	char *compress_type;
 	bool compress_force = false;
 	bool compress = false;
+	unsigned long old_compress_type;
 
 	cache_gen = btrfs_super_cache_generation(root->fs_info->super_copy);
 	if (cache_gen)
@@ -486,40 +487,62 @@ int btrfs_parse_options(struct btrfs_root *root, char *options)
 		case Opt_compress:
 		case Opt_compress_type:
 			compress = true;
+			old_compress_type = info->compress_type;
 			if (token == Opt_compress ||
 			    token == Opt_compress_force ||
 			    strcmp(args[0].from, "zlib") == 0) {
 				compress_type = "zlib";
 				info->compress_type = BTRFS_COMPRESS_ZLIB;
-				btrfs_set_opt(info->mount_opt, COMPRESS);
-				btrfs_clear_opt(info->mount_opt, NODATACOW);
-				btrfs_clear_opt(info->mount_opt, NODATASUM);
 			} else if (strcmp(args[0].from, "lzo") == 0) {
 				compress_type = "lzo";
 				info->compress_type = BTRFS_COMPRESS_LZO;
-				btrfs_set_opt(info->mount_opt, COMPRESS);
-				btrfs_clear_opt(info->mount_opt, NODATACOW);
-				btrfs_clear_opt(info->mount_opt, NODATASUM);
 				btrfs_set_fs_incompat(info, COMPRESS_LZO);
 			} 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;
+				compress = compress_force = false;
 			} else {
 				ret = -EINVAL;
 				goto out;
 			}
 
-			if (compress_force) {
-				btrfs_set_and_info(root, FORCE_COMPRESS,
-						   "force %s compression",
-						   compress_type);
-			} else if (compress) {
-				if (!btrfs_test_opt(root, COMPRESS))
+			if (compress_force || compress) {
+				const char *method = "", *setting = "";
+
+				if ((info->compress_type != old_compress_type) ||
+				    (compress_force &&
+				     !btrfs_test_opt(root, FORCE_COMPRESS)) ||
+				    (!compress_force &&
+				     btrfs_test_opt(root, FORCE_COMPRESS)) ||
+				    (compress && !btrfs_test_opt(root, COMPRESS))) {
+					method = compress_force?"force":"use";
+				}
+
+				if (btrfs_test_opt(root, NODATACOW))
+					setting = ", setting datacow and datasum";
+				else if (btrfs_test_opt(root, NODATASUM))
+					setting = ", setting datasum";
+
+				if (strcmp(method, "") != 0) {
 					btrfs_info(root->fs_info,
-						   "btrfs: use %s compression",
-						   compress_type);
+						   "%s %s compression%s",
+						   method, compress_type,
+						   setting);
+				}
+				if (compress_force) {
+					btrfs_set_opt(info->mount_opt,
+						      FORCE_COMPRESS);
+				} else {
+					btrfs_clear_opt(info->mount_opt,
+							FORCE_COMPRESS);
+				}
+				btrfs_set_opt(info->mount_opt, COMPRESS);
+				btrfs_clear_opt(info->mount_opt, NODATACOW);
+				btrfs_clear_opt(info->mount_opt, NODATASUM);
+			} else {
+				btrfs_clear_and_info(root, COMPRESS,
+						     "use no compression");
+				btrfs_clear_opt(info->mount_opt,
+						FORCE_COMPRESS);
 			}
 			break;
 		case Opt_ssd:
-- 1.8.3.1 


      parent reply	other threads:[~2014-09-18  8:43 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-18  8:25 [PATCH 1/5] btrfs: nodatasum drop compress Satoru Takeuchi
2014-09-18  8:28 ` [PATCH 2/5] btrfs: correct a message on setting nodatacow Satoru Takeuchi
2014-09-19  2:03   ` Qu Wenruo
2014-09-19  6:45     ` Satoru Takeuchi
2014-09-19  7:01       ` Qu Wenruo
2014-09-18  8:30 ` [PATCH 3/5] btrfs: notice nodatasum is also enabled with nodatacow Satoru Takeuchi
2014-09-18  8:32 ` [PATCH 4/5] btrfs: suppress a verbose message about enabling space cache on remounting Satoru Takeuchi
2014-09-18  8:43 ` Satoru Takeuchi [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=541A9B39.7040202@jp.fujitsu.com \
    --to=takeuchi_satoru@jp.fujitsu.com \
    --cc=linux-btrfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.