[20/28] xfs: report iunlink recovery failure upwards
diff mbox series

Message ID 158864116132.182683.16387605365627894770.stgit@magnolia
State New
Headers show
Series
  • xfs: refactor log recovery
Related show

Commit Message

Darrick J. Wong May 5, 2020, 1:12 a.m. UTC
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.

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(-)

Comments

Chandan Babu R May 5, 2020, 1:43 p.m. UTC | #1
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;
>  }
> 
>
Christoph Hellwig May 6, 2020, 3:27 p.m. UTC | #2
On Mon, May 04, 2020 at 06:12:41PM -0700, 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.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Patch
diff mbox series

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;
 }