> On Aug 14, 2015, at 3:47 AM, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
>
> My static check complains because we have:
>
> if (!*bh)
> return -ENOMEM;
> if (*bh) {
>
> The second check is unnecessary.
>
> I've simplified this code by moving the "if (!*bh)" checks around. Also
> Andreas Dilger says we should probably print a warning if sb_getblk()
> fails.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Reviewed-by: Andreas Dilger <adilger@xxxxxxxxx>
> ---
> v2: move the code around even more, add a warning message
>
> diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
> index 8313ca3..0880ec9 100644
> --- a/fs/ext4/mmp.c
> +++ b/fs/ext4/mmp.c
> @@ -69,6 +69,7 @@ static int read_mmp_block(struct super_block *sb, struct buffer_head **bh,
> ext4_fsblk_t mmp_block)
> {
> struct mmp_struct *mmp;
> + int ret;
>
> if (*bh)
> clear_buffer_uptodate(*bh);
> @@ -76,25 +77,24 @@ static int read_mmp_block(struct super_block *sb, struct buffer_head **bh,
> /* This would be sb_bread(sb, mmp_block), except we need to be sure
> * that the MD RAID device cache has been bypassed, and that the read
> * is not blocked in the elevator. */
> - if (!*bh)
> + if (!*bh) {
> *bh = sb_getblk(sb, mmp_block);
> - if (!*bh)
> - return -ENOMEM;
> - if (*bh) {
> - get_bh(*bh);
> - lock_buffer(*bh);
> - (*bh)->b_end_io = end_buffer_read_sync;
> - submit_bh(READ_SYNC | REQ_META | REQ_PRIO, *bh);
> - wait_on_buffer(*bh);
> - if (!buffer_uptodate(*bh)) {
> - brelse(*bh);
> - *bh = NULL;
> + if (!*bh) {
> + ret = -ENOMEM;
> + goto warn_exit;
> }
> }
> - if (unlikely(!*bh)) {
> - ext4_warning(sb, "Error while reading MMP block %llu",
> - mmp_block);
> - return -EIO;
> +
> + get_bh(*bh);
> + lock_buffer(*bh);
> + (*bh)->b_end_io = end_buffer_read_sync;
> + submit_bh(READ_SYNC | REQ_META | REQ_PRIO, *bh);
> + wait_on_buffer(*bh);
> + if (!buffer_uptodate(*bh)) {
> + brelse(*bh);
> + *bh = NULL;
> + ret = -EIO;
> + goto warn_exit;
> }
>
> mmp = (struct mmp_struct *)((*bh)->b_data);
> @@ -103,6 +103,10 @@ static int read_mmp_block(struct super_block *sb, struct buffer_head **bh,
> return -EINVAL;
>
> return 0;
> +
> +warn_exit:
> + ext4_warning(sb, "Error while reading MMP block %llu", mmp_block);
It wouldn't be terrible to include the error in this message, but
since it will also be returned to userspace it isn't critical.
> + return ret;
> }
>
> /*
Cheers, Andreas
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
