Re: [PATCH 1/4] btrfs: Use IS_ERR() instead of checking folio for NULL

From: Boris Burkov <boris@bur.io>
To: Goldwyn Rodrigues <rgoldwyn@suse.de>
Cc: linux-btrfs@vger.kernel.org, Goldwyn Rodrigues <rgoldwyn@suse.com>
Subject: Re: [PATCH 1/4] btrfs: Use IS_ERR() instead of checking folio for NULL
Date: Thu, 18 Jan 2024 13:31:36 -0800	[thread overview]
Message-ID: <20240118213136.GA1356080@zen.localdomain> (raw)
In-Reply-To: <e4df9a1068c81f3edeee9bbb4e63d1d453be569b.1705605787.git.rgoldwyn@suse.com>

On Thu, Jan 18, 2024 at 01:46:37PM -0600, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
> 
> __filemap_get_folio() returns an error instead of a NULL pointer. Use
> IS_ERR() to check if folio is not returned.
> 
> As we are fixing this, use set_folio_extent_mapped() instead of
> set_page_extent_mapped().

nit:
I would change the commit message to something like:
"btrfs_truncate_block folio vs page fixes"

So that it is exactly what it says on the tin, and the second change
isn't a "sneaky" one.

Reviewed-by: Boris Burkov <boris@bur.io>

> 
> Fixes: f8809b1f6a3e btrfs: page to folio conversion in btrfs_truncate_block()
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> ---
>  fs/btrfs/inode.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 7199670599d9..25090d23834b 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -4714,7 +4714,7 @@ int btrfs_truncate_block(struct btrfs_inode *inode, loff_t from, loff_t len,
>  again:
>  	folio = __filemap_get_folio(mapping, index,
>  				    FGP_LOCK | FGP_ACCESSED | FGP_CREAT, mask);
> -	if (!folio) {
> +	if (IS_ERR(folio)) {
>  		btrfs_delalloc_release_space(inode, data_reserved, block_start,
>  					     blocksize, true);
>  		btrfs_delalloc_release_extents(inode, blocksize);
> @@ -4742,7 +4742,7 @@ int btrfs_truncate_block(struct btrfs_inode *inode, loff_t from, loff_t len,
>  	 * folio private, but left the page in the mapping.  Set the page mapped
>  	 * here to make sure it's properly set for the subpage stuff.
>  	 */
> -	ret = set_page_extent_mapped(&folio->page);
> +	ret = set_folio_extent_mapped(folio);
>  	if (ret < 0)
>  		goto out_unlock;
>  
> -- 
> 2.43.0
> 