Re: [PATCH v2 3/4] btrfs: add per-inode compression levels in xattrs

["koraynilay" <koray.fra@gmail.com>]

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

> I'd prefer to have a dedicated patch to set compress_level to the 
> default value 0, as a proper bug fix as the first patch of the series.
>
> As you mentioned in the cover-letter, this is in fact fixing a bug in 
> the old behavior (mismatched algo and level).
>
> So it's definitely worth a dedicated fix, so that we can backport the 
> fix without pulling in the full series for older kernels.

Ah yes of course, although, would it be better to have a single patch
that looks like this[2], which would also need to add
prop_compress_level's initialization to 0 as per [1] (that would
probably be good anyway) or simply a:

 compress_type = inode->prop_compress;
+compress_level = 0; //level in xattr isn't supported yet

in the 2 relevant places?

Thanks.

Best,
koraynilay

P.S. I also noticed that defrag_compress_level too doesn't get
initialized to 0, so if we choose that approach a separate patch that
adds it would be in order IMO.

[1]:
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index e057aceaa68d..aec977f47cfb 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -7896,7 +7896,8 @@ struct inode *btrfs_alloc_inode(struct super_block *sb)
                                              BTRFS_BLOCK_RSV_DELALLOC);
        ei->runtime_flags = 0;
        ei->prop_compress = BTRFS_COMPRESS_NONE;
+       ei->prop_compress_level = 0;
        ei->defrag_compress = BTRFS_COMPRESS_NONE;
 
        ei->delayed_node = NULL;

[2]:
diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index 1082fa92c145..0a4e567d0109 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -130,6 +130,7 @@ struct btrfs_inode {
  
  	/* Cached value of inode property 'compression'. */
  	u8 prop_compress;
+	s8 prop_compress_level;
  
  	/*
  	 * Force compression on the file using the defrag ioctl, could be
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 2534cd9284d5..cff4b3e97559 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -938,6 +938,7 @@ static void compress_file_range(struct btrfs_work *work)
  		compress_level = inode->defrag_compress_level;
  	} else if (inode->prop_compress) {
  		compress_type = inode->prop_compress;
+		compress_level = inode->prop_compress_level;
  	}
  
  	/* Compression level is applied here. */
@@ -2326,6 +2327,7 @@ static int run_delalloc_inline(struct btrfs_inode *inode, struct folio *locked_f
  			compress_level = inode->defrag_compress_level;
  		} else if (inode->prop_compress) {
  			compress_type = inode->prop_compress;
+			compress_level = inode->prop_compress_level;
  		}
  		cb = btrfs_compress_bio(inode, 0, blocksize, compress_type, compress_level, 0);
  		if (IS_ERR(cb)) {

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]