[PATCH] Btrfs: add "nocompress" mount option

[PATCH] Btrfs: add "nocompress" mount option

[Andrei Popa]

In btrfs if we mount with "compress" we have no way to disable
compressing by remounting 
(mount -o remount /mnt/btrfs), only by unmounting and mounting without
"compress".
This patch adds "nocompress" mount option which can be used to remount
the filesystem without compression:
# mount -o remount,nocompress /mnt/btrfs
This option is usefull in cases when we have a filesystem mounted with
"compress" and we want to disable compression but we don't want to
umount and mount it.

Signed-off-by: Andrei Popa <ierdnah@gmail.com>
---
 fs/btrfs/ctree.h |    1 +
 fs/btrfs/super.c |   11 ++++++++++-
 2 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 0236d03..c6e050a 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1567,6 +1567,7 @@ struct btrfs_ioctl_defrag_range_args {
 #define BTRFS_MOUNT_CHECK_INTEGRITY	(1 << 20)
 #define BTRFS_MOUNT_CHECK_INTEGRITY_INCLUDING_EXTENT_DATA (1 << 21)
 #define BTRFS_MOUNT_PANIC_ON_FATAL_ERROR	(1 << 22)
+#define BTRFS_MOUNT_NOCOMPRESS	(1 << 23)
 
 #define btrfs_clear_opt(o, opt)		((o) &= ~BTRFS_MOUNT_##opt)
 #define btrfs_set_opt(o, opt)		((o) |= BTRFS_MOUNT_##opt)
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 0874dba..a78a6cf 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -280,7 +280,7 @@ enum {
 	Opt_enospc_debug, Opt_subvolrootid, Opt_defrag, Opt_inode_cache,
 	Opt_no_space_cache, Opt_recovery, Opt_skip_balance,
 	Opt_check_integrity, Opt_check_integrity_including_extent_data,
-	Opt_check_integrity_print_mask, Opt_fatal_errors,
+	Opt_check_integrity_print_mask, Opt_fatal_errors, Opt_nocompress
 	Opt_err,
 };
 
@@ -299,6 +299,7 @@ static match_table_t tokens = {
 	{Opt_compress_type, "compress=%s"},
 	{Opt_compress_force, "compress-force"},
 	{Opt_compress_force_type, "compress-force=%s"},
+	{Opt_nocompress, "nocompress"},
 	{Opt_ssd, "ssd"},
 	{Opt_ssd_spread, "ssd_spread"},
 	{Opt_nossd, "nossd"},
@@ -404,6 +405,7 @@ int btrfs_parse_options(struct btrfs_root *root,
char *options)
 				goto out;
 			}
 
+			btrfs_clear_opt(info->mount_opt, NOCOMPRESS);
 			btrfs_set_opt(info->mount_opt, COMPRESS);
 			if (compress_force) {
 				btrfs_set_opt(info->mount_opt, FORCE_COMPRESS);
@@ -413,6 +415,11 @@ int btrfs_parse_options(struct btrfs_root *root,
char *options)
 				pr_info("btrfs: use %s compression\n",
 					compress_type);
 			break;
+		case Opt_nocompress:
+			btrfs_clear_opt(info->mount_opt, FORCE_COMPRESS);
+			btrfs_clear_opt(info->mount_opt, COMPRESS);
+			btrfs_set_opt(info->mount_opt, NOCOMPRESS);
+			break;
 		case Opt_ssd:
 			printk(KERN_INFO "btrfs: use ssd allocation scheme\n");
 			btrfs_set_opt(info->mount_opt, SSD);
@@ -856,6 +863,8 @@ static int btrfs_show_options(struct seq_file *seq,
struct dentry *dentry)
 		else
 			seq_printf(seq, ",compress=%s", compress_type);
 	}
+	if (btrfs_test_opt(root, NOCOMPRESS))
+		seq_puts(seq, ",nocompress");
 	if (btrfs_test_opt(root, NOSSD))
 		seq_puts(seq, ",nossd");
 	if (btrfs_test_opt(root, SSD_SPREAD))
-- 
1.7.3.4

Re: [PATCH] Btrfs: add "nocompress" mount option

[David Sterba]

On Fri, Jun 15, 2012 at 12:56:49PM +0300, Andrei Popa wrote:
> In btrfs if we mount with "compress" we have no way to disable
> compressing by remounting 
> (mount -o remount /mnt/btrfs), only by unmounting and mounting without
> "compress".
> This patch adds "nocompress" mount option which can be used to remount
> the filesystem without compression:
> # mount -o remount,nocompress /mnt/btrfs
> This option is usefull in cases when we have a filesystem mounted with
> "compress" and we want to disable compression but we don't want to
> umount and mount it.


Arnd Hannemann sent a patch for this
http://lkml.indiana.edu/hypermail/linux/kernel/1204.2/00231.html

that takes a different approach: setting compression to '=no' disables
compression. I prefer this over adding an extra option to disable. This
way there's no confusion if the compression is on or off.

  mount -o compress,nocompress /dev /mnt

Is it on or off? Yeah we can document that nocompress has higher
priority. But then I can't turn a nocompress option back to compress via
remount. This is possible with Arnd's patch.


david

Re: [PATCH] Btrfs: add "nocompress" mount option

[Andrei Popa]

[-- Attachment #1: Type: text/plain, Size: 633 bytes --]

On Fri, 2012-06-15 at 12:50 +0200, David Sterba wrote: 
> that takes a different approach: setting compression to '=no' disables
> compression. I prefer this over adding an extra option to disable. This
> way there's no confusion if the compression is on or off.
> 
>   mount -o compress,nocompress /dev /mnt
> 
> Is it on or off? Yeah we can document that nocompress has higher
> priority. But then I can't turn a nocompress option back to compress via
> remount. This is possible with Arnd's patch.

Ok, how can we get his patch merged ?

Thanks 
-- 
Andrei Popa
NOC Manager - Nextgen Communications
0760 683 280

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 4553 bytes --]

Re: [PATCH] Btrfs: add "nocompress" mount option

[Andrei Popa]

On Fri, 2012-06-15 at 12:50 +0200, David Sterba wrote:
>   mount -o compress,nocompress /dev /mnt
> 
> Is it on or off? Yeah we can document that nocompress has higher
> priority. But then I can't turn a nocompress option back to compress via
> remount. This is possible with Arnd's patch.

This happens for space_cache mount option:
ierdnac-hp ~ # mount -o remount,space_cache,nospace_cache /mnt/btrfs/

Also there are mount option that can't be enabled/disabled by remount
like:
nobarrier - > "barrier" mount option doesn't exists
ssd -> "nossd" doesn't exists
noacl, user_subvol_rm_allowed, autodefrag, inode_cache, check_int,
check_int_data.

For some of these(ex: nossd,barrier) I think it would be usefull to have
the reverse action.
What do you think ?

Andrei

Re: [PATCH] Btrfs: add "nocompress" mount option

[Goffredo Baroncelli]

On 06/15/2012 12:50 PM, David Sterba wrote:
> I prefer this over adding an extra option to disable. This
> way there's no confusion if the compression is on or off.
> 
>   mount -o compress,nocompress /dev /mnt

The confusion still exists if an user does:

	mount -o compress,compress=no /dev /mnt

However I prefer the form "compress=" because already exists the option
"compress=lzo|zlib".

BR
G.Baroncelli

> 
> Is it on or off? Yeah we can document that nocompress has higher
> priority. But then I can't turn a nocompress option back to compress via
> remount. This is possible with Arnd's patch.
> 
> 
> david

Re: [PATCH] Btrfs: add "nocompress" mount option

[David Sterba]

On Fri, Jun 15, 2012 at 06:56:35PM +0200, Goffredo Baroncelli wrote:
> On 06/15/2012 12:50 PM, David Sterba wrote:
> > I prefer this over adding an extra option to disable. This
> > way there's no confusion if the compression is on or off.
> > 
> >   mount -o compress,nocompress /dev /mnt
> 
> The confusion still exists if an user does:
> 
> 	mount -o compress,compress=no /dev /mnt

Ah right. The precedence is simply given by order on the options line,
and this is same in case of 'nocompress'.

> However I prefer the form "compress=" because already exists the option
> "compress=lzo|zlib".

My comment about precedence is not valid now and the (rather small)
benefit of having a single option is to have the code handling this in a
single location.


david

Re: [PATCH] Btrfs: add "nocompress" mount option

[David Sterba]

On Fri, Jun 15, 2012 at 05:27:49PM +0300, Andrei Popa wrote:
> On Fri, 2012-06-15 at 12:50 +0200, David Sterba wrote:
> >   mount -o compress,nocompress /dev /mnt
> > 
> > Is it on or off? Yeah we can document that nocompress has higher
> > priority. But then I can't turn a nocompress option back to compress via
> > remount. This is possible with Arnd's patch.
> 
> This happens for space_cache mount option:
> ierdnac-hp ~ # mount -o remount,space_cache,nospace_cache /mnt/btrfs/
> 
> Also there are mount option that can't be enabled/disabled by remount
> like:
> nobarrier - > "barrier" mount option doesn't exists
> ssd -> "nossd" doesn't exists
> noacl, user_subvol_rm_allowed, autodefrag, inode_cache, check_int,
> check_int_data.

I'm not against having options with a simple 'no' prefix, but it seems
more fit for a "on/off" type of options, like ssd/nossd. The compression
takes more values.

> For some of these(ex: nossd,barrier) I think it would be usefull to have
> the reverse action.
> What do you think ?

The style of mount options seems to

* have sane defaults
* not listed in mount options
* have reverting counterparts (when applicable)

These are more recommendations than some hard rules, as eg. space_cache
is default, but listed in mount options, and I don't find this harmful.


david