Re: [PATCH v2] btrfs: inode: Don't compress if NODATASUM or NODATACOW set

[David Sterba <dsterba@suse.cz>]

On Thu, Jul 04, 2019 at 11:51:38PM +0000, WenRuo Qu wrote:
> >> @@ -1630,7 +1655,8 @@ int btrfs_run_delalloc_range(struct inode *inode, struct page *locked_page,
> >>  	} else if (BTRFS_I(inode)->flags & BTRFS_INODE_PREALLOC && !force_cow) {
> >>  		ret = run_delalloc_nocow(inode, locked_page, start, end,
> >>  					 page_started, 0, nr_written);
> >> -	} else if (!inode_need_compress(inode, start, end)) {
> >> +	} else if (!inode_can_compress(inode) ||
> >> +		   !inode_need_compress(inode, start, end)) {
> > 
> > Well, that's not excatly what I expected, but because this is an
> > important fix I won't object now and add it to 5.3 queue.
> > 
> 
> I know what you expect, single inode_can_compress().

Yeah, because need_compress calls the heuristic, and then again inside
compress_file_range. I found a few problems in the compression decision
logic that will address that but for now that's how things have been so
the fix is not making it worse.

> But still, we want to avoid hitting the compression routine, thus here
> we do extra inode_need_compress() check other than exiting in
> compress_file_extent().

That's right, some of the compression decisions can be made out of
compress_file_range, like NOCOMPRESS. Even the heuristics can let it
skip compression completely but when this does not reach
compress_file_range the NOCOMPRESS flag has no chance to be set.
And there are more issues.