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.
