On 14.05.2018 10:02, Qu Wenruo wrote:
> As btrfs(5) specified:
>
> Note
> If nodatacow or nodatasum are enabled, compression is disabled.
>
> If NODATASUM or NODATACOW set, we should not compress the extent.
>
> And in fact, we have bug report about corrupted compressed extent
> leading to memory corruption in mail list.
> Although it's mostly buggy lzo implementation causing the problem, btrfs
What does it mean "it's mostly buggy lzo implementation"? If we have bug
in the LZO implementation that btrfs uses shouldn't it be fixed as well ?
> still needs to be fixed to meet the specification.
>
> Reported-by: James Harvey <jamespharve...@gmail.com>
> Signed-off-by: Qu Wenruo <w...@suse.com>
> ---
> fs/btrfs/inode.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index d241285a0d2a..dbef3f404559 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -396,6 +396,14 @@ static inline int inode_need_compress(struct inode
> *inode, u64 start, u64 end)
> {
> struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
>
> + /*
> + * Btrfs doesn't support compression without csum or CoW.
> + * This should have the highest priority.
> + */
> + if (BTRFS_I(inode)->flags & BTRFS_INODE_NODATACOW ||
> + BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM)
> + return 0;
> +
> /* force compress */
> if (btrfs_test_opt(fs_info, FORCE_COMPRESS))
> return 1;
But if we have mounted the fs with FORCE_COMPRESS shouldn't we disregard
the inode flags, presumably the admin knows what he is doing? Won't it
be better if somewhere further in the call chain we check if
FORCE_COMPRESS is set and if so override any inode flags. This can be
implemented by _always_ calling inode_need_compress to decide if we
should compress or not?
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
