Re: [PATCH] Revert "btrfs: compression: don't try to compress if we don't have enough pages"

[Qu Wenruo <wqu@suse.com>]

On 2021/8/25 下午7:55, David Sterba wrote:
> On Wed, Aug 25, 2021 at 01:41:42PM +0800, Qu Wenruo wrote:
>> This reverts commit f2165627319ffd33a6217275e5690b1ab5c45763.
> 
> At this point the revert is the simplest way to restore the inline
> extent compression so that's what I'll probably do. However.
>>
>> [BUG]
>> It's no longer possible to create compressed inline extent after commit
>> f2165627319f ("btrfs: compression: don't try to compress if we don't
>> have enough pages").
>>
>> [CAUSE]
>> For compression code, there are several possible reasons we have a range
>> that needs to be compressed while it's no more than one page.
>>
>> - Compressed inline write
>>    The data is always smaller than one sector.
> 
> The missing logic was for the true inline extent. The patch was supposed
> to skip compression for single pages other than inline extents, due to
> efficiency. So I wonder if we want to do that or just don't bother as
> it's probably a negligible amount of wasted time.

Yeah, I guess that's the case.

We may be able to do such check in the future, but at that time, we need 
to take inline extents into consideration.

Thus it won't be just a simple @nr_pages check, but with extra 
@start/@len check.

>>
>> - Compressed subpage write
>>    For the incoming subpage compressed write support, we require page
>>    alignment of the delalloc range.
>>    And for 64K page size, we can compress just one page into smaller
>>    sectors.
> 
> Oh so the logic would have to be updated to distinguish sectors and
> pages, but to simplify the code for subpage compression support it would
> be easier to revert it and start from there.
> 
For subpage support, there will be no conflicts inside the that function.

Just a reminder that we need to change our mindset to distinguish page 
and sector more in the future.

Anyway, if we really want to do the same check in the future, it will 
take into @start, @len, and sectorsize into consider, and maybe more 
factors to be consider.

Thanks,
Qu