[PATCH] Btrfs: allow mount -o remount,compress=no

[PATCH] Btrfs: allow mount -o remount,compress=no

[Arnd Hannemann]

Btrfs allows to turn on compression on a mounted and used filesystem
by issuing mount -o remount,compress=lzo.
This patch allows to turn compression off again
while the filesystem is mounted.

Signed-off-by: Arnd Hannemann <arnd@arndnet.de>
---
 fs/btrfs/super.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 8d5d380..f1fb6c0 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -394,15 +394,20 @@ int btrfs_parse_options(struct btrfs_root *root, char *options)
 			    strcmp(args[0].from, "zlib") == 0) {
 				compress_type = "zlib";
 				info->compress_type = BTRFS_COMPRESS_ZLIB;
+				btrfs_set_opt(info->mount_opt, COMPRESS);
 			} else if (strcmp(args[0].from, "lzo") == 0) {
 				compress_type = "lzo";
 				info->compress_type = BTRFS_COMPRESS_LZO;
+				btrfs_set_opt(info->mount_opt, COMPRESS);
+			} else if (strncmp(args[0].from, "no", 2) == 0) {
+				compress_type = "no";
+				info->compress_type = BTRFS_COMPRESS_NONE;
+				btrfs_clear_opt(info->mount_opt, COMPRESS);
 			} else {
 				ret = -EINVAL;
 				goto out;
 			}
 
-			btrfs_set_opt(info->mount_opt, COMPRESS);
 			if (compress_force) {
 				btrfs_set_opt(info->mount_opt, FORCE_COMPRESS);
 				pr_info("btrfs: force %s compression\n",
-- 
1.7.9.1

Re: [PATCH] Btrfs: allow mount -o remount,compress=no

[David Sterba]

On Wed, Apr 04, 2012 at 09:46:17PM +0200, Arnd Hannemann wrote:
> Btrfs allows to turn on compression on a mounted and used filesystem
> by issuing mount -o remount,compress=lzo.
> This patch allows to turn compression off again
> while the filesystem is mounted.

I agree it makes sense to allow for turning off the compression via
remount. Currently the mount flag FORCE_COMPRESS implies COMPRESS and
your code may lead to FORCE_COMPRESS without COMPRESS. To fix that
clear the force option as well.

For completeness, there should be also possibility of

  -o remount,compress-force=no

doing exactly the same, just a different syntax. Proposed fix below.

> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -394,15 +394,20 @@ int btrfs_parse_options(struct btrfs_root *root, char *options)
>  			    strcmp(args[0].from, "zlib") == 0) {
>  				compress_type = "zlib";
>  				info->compress_type = BTRFS_COMPRESS_ZLIB;
> +				btrfs_set_opt(info->mount_opt, COMPRESS);
>  			} else if (strcmp(args[0].from, "lzo") == 0) {
>  				compress_type = "lzo";
>  				info->compress_type = BTRFS_COMPRESS_LZO;
> +				btrfs_set_opt(info->mount_opt, COMPRESS);
> +			} else if (strncmp(args[0].from, "no", 2) == 0) {
> +				compress_type = "no";
> +				info->compress_type = BTRFS_COMPRESS_NONE;
> +				btrfs_clear_opt(info->mount_opt, COMPRESS);

				btrfs_clear_opt(info->mount_opt, FORCE_COMPRESS);
				compress_force = false;

>  			} else {
>  				ret = -EINVAL;
>  				goto out;
>  			}
>  
> -			btrfs_set_opt(info->mount_opt, COMPRESS);
>  			if (compress_force) {
>  				btrfs_set_opt(info->mount_opt, FORCE_COMPRESS);
>  				pr_info("btrfs: force %s compression\n",

[PATCH v2] Btrfs: allow mount -o remount,compress=no

[Arnd Hannemann]

Btrfs allows to turn on compression on a mounted and used filesystem
by issuing mount -o remount,compress=lzo.
This patch allows to turn compression off again
while the filesystem is mounted. As suggested by David Sterba
if the compress-force option was set, it is implicitly cleared
if compression is turned off.

Signed-off-by: Arnd Hannemann <arnd@arndnet.de>
---
 fs/btrfs/super.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 8d5d380..79a2ca5 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -394,15 +394,22 @@ int btrfs_parse_options(struct btrfs_root *root, char *options)
 			    strcmp(args[0].from, "zlib") == 0) {
 				compress_type = "zlib";
 				info->compress_type = BTRFS_COMPRESS_ZLIB;
+				btrfs_set_opt(info->mount_opt, COMPRESS);
 			} else if (strcmp(args[0].from, "lzo") == 0) {
 				compress_type = "lzo";
 				info->compress_type = BTRFS_COMPRESS_LZO;
+				btrfs_set_opt(info->mount_opt, COMPRESS);
+			} else if (strncmp(args[0].from, "no", 2) == 0) {
+				compress_type = "no";
+				info->compress_type = BTRFS_COMPRESS_NONE;
+				btrfs_clear_opt(info->mount_opt, COMPRESS);
+				btrfs_clear_opt(info->mount_opt, FORCE_COMPRESS);
+				compress_force = false;
 			} else {
 				ret = -EINVAL;
 				goto out;
 			}
 
-			btrfs_set_opt(info->mount_opt, COMPRESS);
 			if (compress_force) {
 				btrfs_set_opt(info->mount_opt, FORCE_COMPRESS);
 				pr_info("btrfs: force %s compression\n",
-- 
1.7.9.5

Re: [PATCH v2] Btrfs: allow mount -o remount,compress=no

[David Sterba]

On Mon, Apr 16, 2012 at 03:27:51PM +0200, Arnd Hannemann wrote:
> Btrfs allows to turn on compression on a mounted and used filesystem
> by issuing mount -o remount,compress=lzo.
> This patch allows to turn compression off again
> while the filesystem is mounted. As suggested by David Sterba
> if the compress-force option was set, it is implicitly cleared
> if compression is turned off.
> 
> Signed-off-by: Arnd Hannemann <arnd@arndnet.de>

Tested-by: David Sterba <dsterba@suse.cz>

worked perfectly, remounting back and forth with compress=lzo =no
-force=lzo etc, checked output via 'mount'.


david

Re: [PATCH v2] Btrfs: allow mount -o remount,compress=no

[Arnd Hannemann]

Hi Chris,

Am 16.04.2012 16:42, schrieb David Sterba:
> On Mon, Apr 16, 2012 at 03:27:51PM +0200, Arnd Hannemann wrote:
>> Btrfs allows to turn on compression on a mounted and used filesystem
>> by issuing mount -o remount,compress=lzo.
>> This patch allows to turn compression off again
>> while the filesystem is mounted. As suggested by David Sterba
>> if the compress-force option was set, it is implicitly cleared
>> if compression is turned off.
>>
>> Signed-off-by: Arnd Hannemann <arnd@arndnet.de>
> 
> Tested-by: David Sterba <dsterba@suse.cz>
> 
> worked perfectly, remounting back and forth with compress=lzo =no
> -force=lzo etc, checked output via 'mount'.

How show should we proceed to get above mentioned patch
(or the similar patch from Andrei Popa) merged?

Best regards,
Arnd

Re: [PATCH v2] Btrfs: allow mount -o remount,compress=no

[David Sterba]

On Tue, Jun 26, 2012 at 08:48:37AM +0200, Arnd Hannemann wrote:
> How show should we proceed to get above mentioned patch
> (or the similar patch from Andrei Popa) merged?

Josef picked the patch into btrfs-next, I see not problem to include it
in next merge window patchset.


david

Re: [PATCH v2] Btrfs: allow mount -o remount,compress=no

[Mitch Harder]

On Thu, Jun 28, 2012 at 10:40 AM, David Sterba <dave@jikos.cz> wrote:
> On Tue, Jun 26, 2012 at 08:48:37AM +0200, Arnd Hannemann wrote:
>> How show should we proceed to get above mentioned patch
>> (or the similar patch from Andrei Popa) merged?
>
> Josef picked the patch into btrfs-next, I see not problem to include it
> in next merge window patchset.
>

I was testing the lz4(hc) patches, and I found the the compression
INCOMPAT flags are not being updated using the method in this patch.

The compression INCOMPAT flags are generally checked and updated in
the open_ctree() function.

But, on remount, open_ctree() is not called.

I was going to test a patch to update the INCOMPAT flags similar to
the way lzo INCOMPAT is updated when specifying the compress method in
defragmentation.

http://kerneltrap.org/mailarchive/linux-btrfs/2010/11/18/6886194

But, let me know if it is preferred to just return -EINVAL when trying
to remount with a compression method that has an INCOMPAT not yet seen
by that volume.

Re: [PATCH v2] Btrfs: allow mount -o remount,compress=no

[David Sterba]

On Fri, Jul 13, 2012 at 10:19:14AM -0500, Mitch Harder wrote:
> I was testing the lz4(hc) patches, and I found the the compression
> INCOMPAT flags are not being updated using the method in this patch.
> 
> The compression INCOMPAT flags are generally checked and updated in
> the open_ctree() function.
> 
> But, on remount, open_ctree() is not called.

This currently happens with lzo as well, right?

* mount without any compression
* remount -o compress=lzo
* write data
* umount
* => filesystem has lzo data without incompat bit set

> I was going to test a patch to update the INCOMPAT flags similar to
> the way lzo INCOMPAT is updated when specifying the compress method in
> defragmentation.
> 
> http://kerneltrap.org/mailarchive/linux-btrfs/2010/11/18/6886194

This is clear that the incompatibility should be set, because the user
wants it so and the lz4 patches should do the same. I see that the hc
incompatibility does not though, that has to be fixed.

> But, let me know if it is preferred to just return -EINVAL when trying
> to remount with a compression method that has an INCOMPAT not yet seen
> by that volume.

Let's say it returns EINVAL upon remount, then I need to do umount/mount
with the desired option. Remount is usually not done by accident, so
similar to the defrag, I'd expect the operation to succeed, but I as a
user may not know that it brings a backward incompatibility. Getting rid
of an incompat is not straightfoward at all, so I understand the
caution.

My preference is to let remount succeed and set the incompat bit,
possibly with a KERN_INFO message to syslog in case the bit is yet
unseen by the volume.


david

Re: [PATCH v2] Btrfs: allow mount -o remount,compress=no

[Mitch Harder]

On Wed, Jul 18, 2012 at 8:28 PM, David Sterba <dave@jikos.cz> wrote:
> On Fri, Jul 13, 2012 at 10:19:14AM -0500, Mitch Harder wrote:
>> I was testing the lz4(hc) patches, and I found the the compression
>> INCOMPAT flags are not being updated using the method in this patch.
>>
>> The compression INCOMPAT flags are generally checked and updated in
>> the open_ctree() function.
>>
>> But, on remount, open_ctree() is not called.
>
> This currently happens with lzo as well, right?
>

Yes, this will happen with lzo as implemented in the patch at the head
of this thread.

>
> My preference is to let remount succeed and set the incompat bit,
> possibly with a KERN_INFO message to syslog in case the bit is yet
> unseen by the volume.
>

Great.

I've put together a patch that does just that, and I've been testing
it to make sure it works as intended.

I'll finish it up and send it to the list tomorrow.

This patch will only address the lzo INCOMPAT from the remount
capabilities provided by the patch at the head of the thread.

A similar modification will be needed for lz4 patches that allow for remount.