On Wed, Apr 08, 2020 at 08:21:19AM -0400, Brian Foster wrote:
> The filesystem freeze sequence in XFS waits on any background
> eofblocks or cowblocks scans to complete before the filesystem is
> quiesced. At this point, the freezer has already stopped the
> transaction subsystem, however, which means a truncate or cowblock
> cancellation in progress is likely blocked in transaction
> allocation. This results in a deadlock between freeze and the
> associated scanner.
>
> Fix this problem by holding superblock write protection across calls
> into the block reapers. Since protection for background scans is
> acquired from the workqueue task context, trylock to avoid a similar
> deadlock between freeze and blocking on the write lock.
>
> Fixes: d6b636ebb1c9f ("xfs: halt auto-reclamation activities while rebuilding rmap")
> Reported-by: Paul Furtado <paulfurtado91@xxxxxxxxx>
> Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
Looks ok, will test and probably queue for 5.7-rc2...
Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
--D
> ---
>
> Note that this has the opposite tradeoff as the approach I originally
> posited [1], specifically that the eofblocks ioctl() now always blocks
> on a frozen fs rather than return -EAGAIN. It's worth pointing out that
> the eofb control structure has a sync flag (that is not used for
> background scans), so yet another approach could be to tie the trylock
> to that.
>
> Brian
>
> [1] https://lore.kernel.org/linux-xfs/20200407163739.GG28936@bfoster/
>
> fs/xfs/xfs_icache.c | 10 ++++++++++
> fs/xfs/xfs_ioctl.c | 5 ++++-
> 2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index a7be7a9e5c1a..8bf1d15be3f6 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -911,7 +911,12 @@ xfs_eofblocks_worker(
> {
> struct xfs_mount *mp = container_of(to_delayed_work(work),
> struct xfs_mount, m_eofblocks_work);
> +
> + if (!sb_start_write_trylock(mp->m_super))
> + return;
> xfs_icache_free_eofblocks(mp, NULL);
> + sb_end_write(mp->m_super);
> +
> xfs_queue_eofblocks(mp);
> }
>
> @@ -938,7 +943,12 @@ xfs_cowblocks_worker(
> {
> struct xfs_mount *mp = container_of(to_delayed_work(work),
> struct xfs_mount, m_cowblocks_work);
> +
> + if (!sb_start_write_trylock(mp->m_super))
> + return;
> xfs_icache_free_cowblocks(mp, NULL);
> + sb_end_write(mp->m_super);
> +
> xfs_queue_cowblocks(mp);
> }
>
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index cdfb3cd9a25b..309958186d33 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -2363,7 +2363,10 @@ xfs_file_ioctl(
> if (error)
> return error;
>
> - return xfs_icache_free_eofblocks(mp, &keofb);
> + sb_start_write(mp->m_super);
> + error = xfs_icache_free_eofblocks(mp, &keofb);
> + sb_end_write(mp->m_super);
> + return error;
> }
>
> default:
> --
> 2.21.1
>
