Date:   Thu, 9 Sep 2021 15:39:15 +0200
From:   Greg KH <greg@kroah.com>
To:     David Sterba <dsterba@suse.com>
Cc:     stable@vger.kernel.org
Subject: Re: [PATCH stable 5.4, 4.19, 4.14] Revert "btrfs: compression: don't
 try to compress if we don't have enough pages"
Message-ID: <YToOg6MhWOb3Iw/f@kroah.com>
References: <20210909124750.29238-1-dsterba@suse.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210909124750.29238-1-dsterba@suse.com>
Precedence: bulk

On Thu, Sep 09, 2021 at 02:47:50PM +0200, David Sterba wrote:
> From: Qu Wenruo <wqu@suse.com>
> 
> commit 4e9655763b82a91e4c341835bb504a2b1590f984 upstream.
> 
> This reverts commit f2165627319ffd33a6217275e5690b1ab5c45763.
> 
> [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 and the test lacks the
>   condition to properly recognize a non-inline extent.
> 
> - 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.
> 
> For those reasons, the requirement for the data to be more than one page
> is not correct, and is already causing regression for compressed inline
> data writeback.  The idea of skipping one page to avoid wasting CPU time
> could be revisited in the future.
> 
> [FIX]
> Fix it by reverting the offending commit.
> 
> Reported-by: Zygo Blaxell <ce3g8jdj@umail.furryterror.org>
> Link: https://lore.kernel.org/linux-btrfs/afa2742.c084f5d6.17b6b08dffc@tnonline.net
> Fixes: f2165627319f ("btrfs: compression: don't try to compress if we don't have enough pages")
> CC: stable@vger.kernel.org # 4.4+
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> Reviewed-by: David Sterba <dsterba@suse.com>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/inode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 29552d4f6845..33b8fedab6c6 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -543,7 +543,7 @@ static noinline int compress_file_range(struct async_chunk *async_chunk)
>  	 * inode has not been flagged as nocompress.  This flag can
>  	 * change at any time if we discover bad compression ratios.
>  	 */
> -	if (nr_pages > 1 && inode_need_compress(inode, start, end)) {
> +	if (inode_need_compress(inode, start, end)) {
>  		WARN_ON(pages);
>  		pages = kcalloc(nr_pages, sizeof(struct page *), GFP_NOFS);
>  		if (!pages) {
> -- 
> 2.33.0
> 

All now queued up, thanks.

greg k-h