Re: [PATCH 10/12] jbd2: jbd2_journal_init_{dev,inode} return proper error return value

Jan Kara
2023-08-03T15:56:59Z

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