[4/4] btrfs: allow defrag compress to override NOCOMPRESS attribute
diff mbox

Message ID afb29965ae24123bf6b5aa4d9390f42292a1796c.1500317040.git.dsterba@suse.com
State New
Headers show

Commit Message

David Sterba July 17, 2017, 6:46 p.m. UTC
Currently, the BTRFS_INODE_NOCOMPRESS will prevent any compression on a
given file, except when the mount is force-compress. As users have
reported on IRC, this will also prevent compression when requested by
defrag (btrfs fi defrag -c file).

The nocompress flag is set automatically by filesystem when the ratios
are bad and the user would have to manually drop the bit in order to
make defrag -c work. This is not good from the usability perspective.

This patch will raise priority for the defrag -c over nocompress, ie.
any file with NOCOMPRESS bit set will get defragmented. The bit will
remain untouched.

Alternate option was to also drop the nocompress bit and keep the
decision logic as is, but I think this is not the right solution.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/inode.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Anand Jain July 20, 2017, 11:02 a.m. UTC | #1
On 07/18/2017 02:46 AM, David Sterba wrote:
> Currently, the BTRFS_INODE_NOCOMPRESS will prevent any compression on a
> given file, except when the mount is force-compress. As users have
> reported on IRC, this will also prevent compression when requested by
> defrag (btrfs fi defrag -c file).

  There is a hidden workaround... even with the existing
  inode_need_compression().

  BTRFS_INODE_NOCOMPRESS gets reset [1] so ..

    (btrfs prop set /btrfs/sv1 compression "")
    btrfs prop set /btrfs/sv1 compression lzo
    A normal defrag or with -c will try to compress again.

----------------
static int prop_compression_apply(struct inode *inode,
                                   const char *value,
                                   size_t len)
{
         int type;

         if (len == 0) {
                 BTRFS_I(inode)->flags |= BTRFS_INODE_NOCOMPRESS;
                 BTRFS_I(inode)->flags &= ~BTRFS_INODE_COMPRESS;
                 BTRFS_I(inode)->force_compress = BTRFS_COMPRESS_NONE;

                 return 0;
         }

         if (!strncmp("lzo", value, len))
                 type = BTRFS_COMPRESS_LZO;
         else if (!strncmp("zlib", value, len))
                 type = BTRFS_COMPRESS_ZLIB;
         else
                 return -EINVAL;

         BTRFS_I(inode)->flags &= ~BTRFS_INODE_NOCOMPRESS;   <---- [1]
         BTRFS_I(inode)->flags |= BTRFS_INODE_COMPRESS;
         BTRFS_I(inode)->force_compress = type;

         return 0;
}
---------------

  So what's missing is btrfs prop set /btrfs/sv1 compress-force
  and rest cleanup as discussed in the other email.

Thanks, Anand


--
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

Patch
diff mbox

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 14677535610b..d28441831ac6 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -399,12 +399,12 @@  static inline int inode_need_compress(struct inode *inode)
 	/* force compress */
 	if (btrfs_test_opt(fs_info, FORCE_COMPRESS))
 		return 1;
-	/* bad compression ratios */
-	if (BTRFS_I(inode)->flags & BTRFS_INODE_NOCOMPRESS)
-		return 0;
 	/* defrag ioctl */
 	if (BTRFS_I(inode)->defrag_compress)
 		return 1;
+	/* bad compression ratios */
+	if (BTRFS_I(inode)->flags & BTRFS_INODE_NOCOMPRESS)
+		return 0;
 	if (btrfs_test_opt(fs_info, COMPRESS) ||
 	    BTRFS_I(inode)->flags & BTRFS_INODE_COMPRESS ||
 	    BTRFS_I(inode)->prop_compress)