Re: [PATCH 15/17] gfs2: use bdev_rw_virt in gfs2_read_super

[Damien Le Moal <dlemoal@kernel.org>]

On 4/22/25 23:26, Christoph Hellwig wrote:
> Switch gfs2_read_super to allocate the superblock buffer using kmalloc
> which falls back to the page allocator for PAGE_SIZE allocation but
> gives us a kernel virtual address and then use bdev_rw_virt to perform
> the synchronous read into it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

One nit below.

> ---
>  fs/gfs2/ops_fstype.c | 24 +++++++++---------------
>  1 file changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
> index e83d293c3614..7c1014ba7ac7 100644
> --- a/fs/gfs2/ops_fstype.c
> +++ b/fs/gfs2/ops_fstype.c
> @@ -226,28 +226,22 @@ static void gfs2_sb_in(struct gfs2_sbd *sdp, const struct gfs2_sb *str)
>  
>  static int gfs2_read_super(struct gfs2_sbd *sdp, sector_t sector, int silent)
>  {
> -	struct super_block *sb = sdp->sd_vfs;
> -	struct page *page;
> -	struct bio_vec bvec;
> -	struct bio bio;
> +	struct gfs2_sb *sb;
>  	int err;
>  
> -	page = alloc_page(GFP_KERNEL);
> -	if (unlikely(!page))
> +	sb = kmalloc(PAGE_SIZE, GFP_KERNEL);
> +	if (unlikely(!sb))
>  		return -ENOMEM;
> -
> -	bio_init(&bio, sb->s_bdev, &bvec, 1, REQ_OP_READ | REQ_META);
> -	bio.bi_iter.bi_sector = sector * (sb->s_blocksize >> 9);
> -	__bio_add_page(&bio, page, PAGE_SIZE, 0);
> -
> -	err = submit_bio_wait(&bio);
> +	err = bdev_rw_virt(sdp->sd_vfs->s_bdev,
> +			sector * (sdp->sd_vfs->s_blocksize >> 9), sb, PAGE_SIZE,

While at it, use SECTOR_SHIFT here ?

> +			REQ_OP_READ | REQ_META);
>  	if (err) {
>  		pr_warn("error %d reading superblock\n", err);
> -		__free_page(page);
> +		kfree(sb);
>  		return err;
>  	}
> -	gfs2_sb_in(sdp, page_address(page));
> -	__free_page(page);
> +	gfs2_sb_in(sdp, sb);
> +	kfree(sb);
>  	return gfs2_check_sb(sdp, silent);
>  }
>  


-- 
Damien Le Moal
Western Digital Research