Message ID | 158864116132.182683.16387605365627894770.stgit@magnolia (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | xfs: refactor log recovery | expand |
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; > } > >
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>
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; }