On Wed, Oct 25, 2017 at 02:57:05PM -0400, Brian Foster wrote:
> Log recovery of v4 filesystems does not use buffer verifiers because
> log recovery historically can result in transient buffer corruption
> when target buffers might be ahead of the log after a crash. v5
> filesystems work around this problem with metadata LSN ordering.
>
> While this log recovery verifier behavior is necessary on v4 supers,
> it can result in leaving buffers around in the LRU without verifiers
> attached for a significant amount of time. This leads to use of
> unverified buffers while the filesystem is in active use, long after
> recovery has completed.
>
> To address this problem, drain all buffers from the LRU as a final
> step of the log mount sequence. Note that this is done
> unconditionally to provide a consistently clean cache footprint,
> regardless of superblock version or log state. As a side effect,
> this ensures that all cache resident, unverified buffers are
> reclaimed after log recovery and therefore must be recreated with
> verifiers on subsequent use.
>
> Reported-by: Darrick Wong <darrick.wong@xxxxxxxxxx>
> Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
> fs/xfs/xfs_log.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index dc95a49..ab59e78 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -744,6 +744,7 @@ xfs_log_mount_finish(
> {
> int error = 0;
> bool readonly = (mp->m_flags & XFS_MOUNT_RDONLY);
> + bool recovered = mp->m_log->l_flags & XLOG_RECOVERY_NEEDED;
>
> if (mp->m_flags & XFS_MOUNT_NORECOVERY) {
> ASSERT(mp->m_flags & XFS_MOUNT_RDONLY);
> @@ -780,6 +781,21 @@ xfs_log_mount_finish(
> mp->m_super->s_flags &= ~MS_ACTIVE;
> evict_inodes(mp->m_super);
>
> + /*
> + * Drain the buffer LRU after log recovery. This is required for v4
> + * filesystems to avoid leaving around buffers with NULL verifier ops,
> + * but we do it unconditionally to make sure we're always in a clean
> + * cache state after mount.
> + *
> + * 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) {
> + xfs_log_force(mp, XFS_LOG_SYNC);
> + xfs_ail_push_all_sync(mp->m_ail);
> + }
> + xfs_wait_buftarg(mp->m_ddev_targp);
> +
> if (readonly)
> mp->m_flags |= XFS_MOUNT_RDONLY;
>
> --
> 2.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
