On 07/18/2017 11:01 PM, David Sterba wrote:
On Tue, Jul 18, 2017 at 05:37:47PM +0800, Anand Jain wrote:
Its better to have the policy enforcement going through a function,
so that we have better control and visibility of the decision logic.
Signed-off-by: Anand Jain <anand.jain@xxxxxxxxxx>
---
fs/btrfs/inode.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 06dea7c89bbd..d0cc3de120b7 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -1189,11 +1189,10 @@ static int cow_file_range_async(struct inode *inode, struct page *locked_page,
async_cow->locked_page = locked_page;
async_cow->start = start;
- if (BTRFS_I(inode)->flags & BTRFS_INODE_NOCOMPRESS &&
- !btrfs_test_opt(fs_info, FORCE_COMPRESS))
- cur_end = end;
- else
+ if (inode_need_compress(inode))
cur_end = min(end, start + SZ_512K - 1);
+ else
+ cur_end = end;
The opencoded test should be cleaned up, however cow_file_range_async is
called from run_delalloc_range if the inode_need_compress passes the
'inode_need_compress' condition. So at minimum, checking again here is
redundant,
David, no its not redundant. If the preceding SZ_512K block is not
compressible, then we would have already set the BTRFS_INODE_NOCOMPRESS
flag, which then we would straightaway go up to the end.
but we still might need to know if the compression is
desired.
Uh ?
The check does no depend on anything inside the loop, so it should be
moved out of it.
No, it should not be, if you move check outside then it will act as if
compress-force is set even if you don't set it.
If I understand the logic correctly, we get to
cow_file_range_async and always want to compress, so the test should be
dropped entirely.
sorry. its wrong. as above.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
