Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 3 Aug 2023 17:56:53 +0200
From:   Jan Kara <jack@suse.cz>
To:     Zhang Yi <yi.zhang@huaweicloud.com>
Cc:     linux-ext4@vger.kernel.org, tytso@mit.edu,
        adilger.kernel@dilger.ca, jack@suse.cz, yi.zhang@huawei.com,
        chengzhihao1@huawei.com, yukuai3@huawei.com
Subject: Re: [PATCH 10/12] jbd2: jbd2_journal_init_{dev,inode} return proper
 error return value
Message-ID: <20230803155653.tzqcnpia3ksb7xsq@quack3>
References: <20230704134233.110812-1-yi.zhang@huaweicloud.com>
 <20230704134233.110812-11-yi.zhang@huaweicloud.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230704134233.110812-11-yi.zhang@huaweicloud.com>
Precedence: bulk

On Tue 04-07-23 21:42:31, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
> 
> Current jbd2_journal_init_{dev,inode} return NULL if some error
> happens, make them to pass out proper error return value.
> 
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>

Looks good. Feel free to add:

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

								Honza

> ---
>  fs/ext4/super.c    |  4 ++--
>  fs/jbd2/journal.c  | 19 +++++++++----------
>  fs/ocfs2/journal.c |  8 ++++----
>  3 files changed, 15 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index c94ebf704616..ce2e02b139af 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -5827,7 +5827,7 @@ static journal_t *ext4_get_journal(struct super_block *sb,
>  		return NULL;
>  
>  	journal = jbd2_journal_init_inode(journal_inode);
> -	if (!journal) {
> +	if (IS_ERR(journal)) {
>  		ext4_msg(sb, KERN_ERR, "Could not load journal inode");
>  		iput(journal_inode);
>  		return NULL;
> @@ -5906,7 +5906,7 @@ static journal_t *ext4_get_dev_journal(struct super_block *sb,
>  
>  	journal = jbd2_journal_init_dev(bdev, sb->s_bdev,
>  					start, len, blocksize);
> -	if (!journal) {
> +	if (IS_ERR(journal)) {
>  		ext4_msg(sb, KERN_ERR, "failed to create device journal");
>  		goto out_bdev;
>  	}
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index cc344b8d7476..34dd65aa9f61 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -1539,7 +1539,7 @@ static journal_t *journal_init_common(struct block_device *bdev,
>  
>  	journal = kzalloc(sizeof(*journal), GFP_KERNEL);
>  	if (!journal)
> -		return NULL;
> +		return ERR_PTR(-ENOMEM);
>  
>  	journal->j_blocksize = blocksize;
>  	journal->j_dev = bdev;
> @@ -1584,6 +1584,7 @@ static journal_t *journal_init_common(struct block_device *bdev,
>  	 * journal descriptor can store up to n blocks, we need enough
>  	 * buffers to write out full descriptor block.
>  	 */
> +	err = -ENOMEM;
>  	n = journal->j_blocksize / jbd2_min_tag_size();
>  	journal->j_wbufsize = n;
>  	journal->j_fc_wbuf = NULL;
> @@ -1615,7 +1616,7 @@ static journal_t *journal_init_common(struct block_device *bdev,
>  	jbd2_journal_destroy_revoke(journal);
>  	journal_fail_superblock(journal);
>  	kfree(journal);
> -	return NULL;
> +	return ERR_PTR(err);
>  }
>  
>  /* jbd2_journal_init_dev and jbd2_journal_init_inode:
> @@ -1648,8 +1649,8 @@ journal_t *jbd2_journal_init_dev(struct block_device *bdev,
>  	journal_t *journal;
>  
>  	journal = journal_init_common(bdev, fs_dev, start, len, blocksize);
> -	if (!journal)
> -		return NULL;
> +	if (IS_ERR(journal))
> +		return ERR_CAST(journal);
>  
>  	snprintf(journal->j_devname, sizeof(journal->j_devname),
>  		 "%pg", journal->j_dev);
> @@ -1675,11 +1676,9 @@ journal_t *jbd2_journal_init_inode(struct inode *inode)
>  
>  	blocknr = 0;
>  	err = bmap(inode, &blocknr);
> -
>  	if (err || !blocknr) {
> -		pr_err("%s: Cannot locate journal superblock\n",
> -			__func__);
> -		return NULL;
> +		pr_err("%s: Cannot locate journal superblock\n", __func__);
> +		return err ? ERR_PTR(err) : ERR_PTR(-EINVAL);
>  	}
>  
>  	jbd2_debug(1, "JBD2: inode %s/%ld, size %lld, bits %d, blksize %ld\n",
> @@ -1689,8 +1688,8 @@ journal_t *jbd2_journal_init_inode(struct inode *inode)
>  	journal = journal_init_common(inode->i_sb->s_bdev, inode->i_sb->s_bdev,
>  			blocknr, inode->i_size >> inode->i_sb->s_blocksize_bits,
>  			inode->i_sb->s_blocksize);
> -	if (!journal)
> -		return NULL;
> +	if (IS_ERR(journal))
> +		return ERR_CAST(journal);
>  
>  	journal->j_inode = inode;
>  	snprintf(journal->j_devname, sizeof(journal->j_devname),
> diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
> index 25d8072ccfce..f35a1bbf52e2 100644
> --- a/fs/ocfs2/journal.c
> +++ b/fs/ocfs2/journal.c
> @@ -911,9 +911,9 @@ int ocfs2_journal_init(struct ocfs2_super *osb, int *dirty)
>  
>  	/* call the kernels journal init function now */
>  	j_journal = jbd2_journal_init_inode(inode);
> -	if (j_journal == NULL) {
> +	if (IS_ERR(j_journal)) {
>  		mlog(ML_ERROR, "Linux journal layer error\n");
> -		status = -EINVAL;
> +		status = PTR_ERR(journal);
>  		goto done;
>  	}
>  
> @@ -1687,9 +1687,9 @@ static int ocfs2_replay_journal(struct ocfs2_super *osb,
>  	}
>  
>  	journal = jbd2_journal_init_inode(inode);
> -	if (journal == NULL) {
> +	if (IS_ERR(journal)) {
>  		mlog(ML_ERROR, "Linux journal layer error\n");
> -		status = -EIO;
> +		status = PTR_ERR(journal);
>  		goto done;
>  	}
>  
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR