ext4: add ext4_sb_bread_nofail() helper function for ext4_free_branches()

[PATCH] ext4: add ext4_sb_bread_nofail() helper function for ext4_free_branches()

Posted by libaokun@huaweicloud.com 1 month, 1 week ago

From: Baokun Li <libaokun1@huawei.com>

The implicit __GFP_NOFAIL flag in ext4_sb_bread() was removed in commit
8a83ac54940d ("ext4: call bdev_getblk() from sb_getblk_gfp()"), meaning
the function can now fail under memory pressure.

Most callers of ext4_sb_bread() propagate the error to userspace and do not
remount the filesystem read-only. However, ext4_free_branches() handles
ext4_sb_bread() failure by remounting the filesystem read-only.

This implies that an ext3 filesystem (mounted via the ext4 driver) could be
forcibly remounted read-only due to a transient page allocation failure,
which is unacceptable.

To mitigate this, introduce a new helper function, ext4_sb_bread_nofail(),
which explicitly uses __GFP_NOFAIL, and use it in ext4_free_branches().

Fixes: 8a83ac54940d ("ext4: call bdev_getblk() from sb_getblk_gfp()")
Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
 fs/ext4/ext4.h     | 2 ++
 fs/ext4/indirect.c | 2 +-
 fs/ext4/super.c    | 9 +++++++++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 106484739932..32315e51639e 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3134,6 +3134,8 @@ extern struct buffer_head *ext4_sb_bread(struct super_block *sb,
 					 sector_t block, blk_opf_t op_flags);
 extern struct buffer_head *ext4_sb_bread_unmovable(struct super_block *sb,
 						   sector_t block);
+extern struct buffer_head *ext4_sb_bread_nofail(struct super_block *sb,
+						sector_t block);
 extern void ext4_read_bh_nowait(struct buffer_head *bh, blk_opf_t op_flags,
 				bh_end_io_t *end_io, bool simu_fail);
 extern int ext4_read_bh(struct buffer_head *bh, blk_opf_t op_flags,
diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
index 7de327fa7b1c..39350a18e919 100644
--- a/fs/ext4/indirect.c
+++ b/fs/ext4/indirect.c
@@ -1025,7 +1025,7 @@ static void ext4_free_branches(handle_t *handle, struct inode *inode,
 			}
 
 			/* Go read the buffer for the next level down */
-			bh = ext4_sb_bread(inode->i_sb, nr, 0);
+			bh = ext4_sb_bread_nofail(inode->i_sb, nr);
 
 			/*
 			 * A read failure? Report error and clear slot
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index c7d39da7e733..a0e2b396aa76 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -265,6 +265,15 @@ struct buffer_head *ext4_sb_bread_unmovable(struct super_block *sb,
 	return __ext4_sb_bread_gfp(sb, block, 0, gfp);
 }
 
+struct buffer_head *ext4_sb_bread_nofail(struct super_block *sb,
+					 sector_t block)
+{
+	gfp_t gfp = mapping_gfp_constraint(sb->s_bdev->bd_mapping,
+			~__GFP_FS) | __GFP_MOVABLE | __GFP_NOFAIL;
+
+	return __ext4_sb_bread_gfp(sb, block, 0, gfp);
+}
+
 void ext4_sb_breadahead_unmovable(struct super_block *sb, sector_t block)
 {
 	struct buffer_head *bh = bdev_getblk(sb->s_bdev, block,
-- 
2.39.2

Re: [PATCH] ext4: add ext4_sb_bread_nofail() helper function for ext4_free_branches()

Posted by Theodore Ts'o 1 week ago

On Thu, 21 Aug 2025 21:38:57 +0800, libaokun@huaweicloud.com wrote:
> The implicit __GFP_NOFAIL flag in ext4_sb_bread() was removed in commit
> 8a83ac54940d ("ext4: call bdev_getblk() from sb_getblk_gfp()"), meaning
> the function can now fail under memory pressure.
> 
> Most callers of ext4_sb_bread() propagate the error to userspace and do not
> remount the filesystem read-only. However, ext4_free_branches() handles
> ext4_sb_bread() failure by remounting the filesystem read-only.
> 
> [...]

Applied, thanks!

[1/1] ext4: add ext4_sb_bread_nofail() helper function for ext4_free_branches()
      commit: d8b90e6387a74bcb1714c8d1e6a782ff709de9a9

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>

Re: [PATCH] ext4: add ext4_sb_bread_nofail() helper function for ext4_free_branches()

Posted by Jan Kara 1 month, 1 week ago

On Thu 21-08-25 21:38:57, libaokun@huaweicloud.com wrote:
> From: Baokun Li <libaokun1@huawei.com>
> 
> The implicit __GFP_NOFAIL flag in ext4_sb_bread() was removed in commit
> 8a83ac54940d ("ext4: call bdev_getblk() from sb_getblk_gfp()"), meaning
> the function can now fail under memory pressure.
> 
> Most callers of ext4_sb_bread() propagate the error to userspace and do not
> remount the filesystem read-only. However, ext4_free_branches() handles
> ext4_sb_bread() failure by remounting the filesystem read-only.
> 
> This implies that an ext3 filesystem (mounted via the ext4 driver) could be
> forcibly remounted read-only due to a transient page allocation failure,
> which is unacceptable.
> 
> To mitigate this, introduce a new helper function, ext4_sb_bread_nofail(),
> which explicitly uses __GFP_NOFAIL, and use it in ext4_free_branches().
> 
> Fixes: 8a83ac54940d ("ext4: call bdev_getblk() from sb_getblk_gfp()")
> Signed-off-by: Baokun Li <libaokun1@huawei.com>

Looks sane. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/ext4.h     | 2 ++
>  fs/ext4/indirect.c | 2 +-
>  fs/ext4/super.c    | 9 +++++++++
>  3 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 106484739932..32315e51639e 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3134,6 +3134,8 @@ extern struct buffer_head *ext4_sb_bread(struct super_block *sb,
>  					 sector_t block, blk_opf_t op_flags);
>  extern struct buffer_head *ext4_sb_bread_unmovable(struct super_block *sb,
>  						   sector_t block);
> +extern struct buffer_head *ext4_sb_bread_nofail(struct super_block *sb,
> +						sector_t block);
>  extern void ext4_read_bh_nowait(struct buffer_head *bh, blk_opf_t op_flags,
>  				bh_end_io_t *end_io, bool simu_fail);
>  extern int ext4_read_bh(struct buffer_head *bh, blk_opf_t op_flags,
> diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
> index 7de327fa7b1c..39350a18e919 100644
> --- a/fs/ext4/indirect.c
> +++ b/fs/ext4/indirect.c
> @@ -1025,7 +1025,7 @@ static void ext4_free_branches(handle_t *handle, struct inode *inode,
>  			}
>  
>  			/* Go read the buffer for the next level down */
> -			bh = ext4_sb_bread(inode->i_sb, nr, 0);
> +			bh = ext4_sb_bread_nofail(inode->i_sb, nr);
>  
>  			/*
>  			 * A read failure? Report error and clear slot
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index c7d39da7e733..a0e2b396aa76 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -265,6 +265,15 @@ struct buffer_head *ext4_sb_bread_unmovable(struct super_block *sb,
>  	return __ext4_sb_bread_gfp(sb, block, 0, gfp);
>  }
>  
> +struct buffer_head *ext4_sb_bread_nofail(struct super_block *sb,
> +					 sector_t block)
> +{
> +	gfp_t gfp = mapping_gfp_constraint(sb->s_bdev->bd_mapping,
> +			~__GFP_FS) | __GFP_MOVABLE | __GFP_NOFAIL;
> +
> +	return __ext4_sb_bread_gfp(sb, block, 0, gfp);
> +}
> +
>  void ext4_sb_breadahead_unmovable(struct super_block *sb, sector_t block)
>  {
>  	struct buffer_head *bh = bdev_getblk(sb->s_bdev, block,
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR