diff mbox series

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

Message ID e4df9a1068c81f3edeee9bbb4e63d1d453be569b.1705605787.git.rgoldwyn@suse.com (mailing list archive)
State New, archived
Headers show
Series [1/4] btrfs: Use IS_ERR() instead of checking folio for NULL | expand

Commit Message

Goldwyn Rodrigues Jan. 18, 2024, 7:46 p.m. UTC 
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().

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(-)

Comments

Boris Burkov Jan. 18, 2024, 9:31 p.m. UTC | #1 
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
>
David Sterba Jan. 19, 2024, 2:53 p.m. UTC | #2 
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().
> 
> Fixes: f8809b1f6a3e btrfs: page to folio conversion in btrfs_truncate_block()

This is still in for-next so the fixup should be squashed there.
Goldwyn Rodrigues Jan. 19, 2024, 2:58 p.m. UTC | #3 
On 15:53 19/01, David Sterba wrote:
> 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().
> > 
> > Fixes: f8809b1f6a3e btrfs: page to folio conversion in btrfs_truncate_block()
> 
> This is still in for-next so the fixup should be squashed there.

Oh cool. However, this would have to be put after Matthew's patch of
changing set_page_extent_mapped().
diff mbox series

Patch

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;