Re: [PATCH 20/28] xfs: report iunlink recovery failure upwards

Chandan Babu R
2020-05-05T13:43:27Z

On Tuesday 5 May 2020 6:42:41 AM IST Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> If we fail to recover unlinked inodes due to corruption or whatnot, we
> should report this upwards and fail the mount instead of continuing on
> like nothing's wrong. Eventually the user will trip over the busted
> AGI anyway.

The changes look good to me.

Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>

>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_log_recover.h | 2 +-
> fs/xfs/xfs_log.c | 4 +++-
> fs/xfs/xfs_log_recover.c | 7 ++++++-
> fs/xfs/xfs_unlink_recover.c | 4 +++-
> 4 files changed, 13 insertions(+), 4 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_log_recover.h b/fs/xfs/libxfs/xfs_log_recover.h
> index 33c14dd22b77..d4d6d4f84fda 100644
> --- a/fs/xfs/libxfs/xfs_log_recover.h
> +++ b/fs/xfs/libxfs/xfs_log_recover.h
> @@ -124,6 +124,6 @@ bool xlog_add_buffer_cancelled(struct xlog *log, xfs_daddr_t blkno, uint len);
> bool xlog_is_buffer_cancelled(struct xlog *log, xfs_daddr_t blkno, uint len);
> bool xlog_put_buffer_cancelled(struct xlog *log, xfs_daddr_t blkno, uint len);
> void xlog_recover_iodone(struct xfs_buf *bp);
> -void xlog_recover_process_unlinked(struct xlog *log);
> +int xlog_recover_process_unlinked(struct xlog *log);
>
> #endif /* __XFS_LOG_RECOVER_H__ */
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 00fda2e8e738..8203b9b0fd08 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -727,6 +727,8 @@ xfs_log_mount_finish(
> xfs_log_work_queue(mp);
> mp->m_super->s_flags &= ~SB_ACTIVE;
> evict_inodes(mp->m_super);
> + if (error)
> + return error;
>
> /*
> * Drain the buffer LRU after log recovery. This is required for v4
> @@ -737,7 +739,7 @@ xfs_log_mount_finish(
> * Don't push in the error case because the AIL may have pending intents
> * that aren't removed until recovery is cancelled.
> */
> - if (!error && recovered) {
> + if (recovered) {
> xfs_log_force(mp, XFS_LOG_SYNC);
> xfs_ail_push_all_sync(mp->m_ail);
> }
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 362296b34490..0ccc09c004f1 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -3399,7 +3399,12 @@ xlog_recover_finish(
> */
> xfs_log_force(log->l_mp, XFS_LOG_SYNC);
>
> - xlog_recover_process_unlinked(log);
> + error = xlog_recover_process_unlinked(log);
> + if (error) {
> + xfs_alert(log->l_mp,
> + "Failed to recover unlinked metadata");
> + return error;
> + }
>
> xlog_recover_check_summary(log);
>
> diff --git a/fs/xfs/xfs_unlink_recover.c b/fs/xfs/xfs_unlink_recover.c
> index 413b34085640..fe7fa3d623f2 100644
> --- a/fs/xfs/xfs_unlink_recover.c
> +++ b/fs/xfs/xfs_unlink_recover.c
> @@ -195,7 +195,7 @@ xlog_recover_process_iunlinked(
> return 0;
> }
>
> -void
> +int
> xlog_recover_process_unlinked(
> struct xlog *log)
> {
> @@ -208,4 +208,6 @@ xlog_recover_process_unlinked(
> if (error)
> break;
> }
> +
> + return error;
> }
>
>

--
chandan