On Thu, Dec 10, 2020 at 09:46:07AM -0500, Brian Foster wrote:
> xfs_buftarg_drain() is called from xfs_log_quiesce() to ensure the
> buffer cache is reclaimed during unmount. xfs_log_quiesce() is also
> called from xfs_quiesce_attr(), however, which means that cache
> state is completely drained for filesystem freeze and read-only
> remount. While technically harmless, this is unnecessarily
> heavyweight. Both freeze and read-only mounts allow reads and thus
> allow population of the buffer cache. Therefore, the transitional
> sequence in either case really only needs to quiesce outstanding
> writes to return the filesystem in a generally read-only state.
>
> Additionally, some users have reported that attempts to freeze a
> filesystem concurrent with a read-heavy workload causes the freeze
> process to stall for a significant amount of time. This occurs
> because, as mentioned above, the read workload repopulates the
> buffer LRU while the freeze task attempts to drain it.
>
> To improve this situation, replace the drain in xfs_log_quiesce()
> with a buffer I/O quiesce and lift the drain into the unmount path.
> This removes buffer LRU reclaim from freeze and read-only [re]mount,
> but ensures the LRU is still drained before the filesystem unmounts.
>
> Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
Looks good to me,
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> ---
> fs/xfs/xfs_buf.c | 20 +++++++++++++++-----
> fs/xfs/xfs_buf.h | 1 +
> fs/xfs/xfs_log.c | 6 ++++--
> 3 files changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index db918ed20c40..d3fce3129f6e 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -1815,14 +1815,13 @@ xfs_buftarg_drain_rele(
> return LRU_REMOVED;
> }
>
> +/*
> + * Wait for outstanding I/O on the buftarg to complete.
> + */
> void
> -xfs_buftarg_drain(
> +xfs_buftarg_wait(
> struct xfs_buftarg *btp)
> {
> - LIST_HEAD(dispose);
> - int loop = 0;
> - bool write_fail = false;
> -
> /*
> * First wait on the buftarg I/O count for all in-flight buffers to be
> * released. This is critical as new buffers do not make the LRU until
> @@ -1838,6 +1837,17 @@ xfs_buftarg_drain(
> while (percpu_counter_sum(&btp->bt_io_count))
> delay(100);
> flush_workqueue(btp->bt_mount->m_buf_workqueue);
> +}
> +
> +void
> +xfs_buftarg_drain(
> + struct xfs_buftarg *btp)
> +{
> + LIST_HEAD(dispose);
> + int loop = 0;
> + bool write_fail = false;
> +
> + xfs_buftarg_wait(btp);
>
> /* loop until there is nothing left on the lru list. */
> while (list_lru_count(&btp->bt_lru)) {
> diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
> index ea32369f8f77..96c6b478e26e 100644
> --- a/fs/xfs/xfs_buf.h
> +++ b/fs/xfs/xfs_buf.h
> @@ -347,6 +347,7 @@ xfs_buf_update_cksum(struct xfs_buf *bp, unsigned long cksum_offset)
> extern struct xfs_buftarg *xfs_alloc_buftarg(struct xfs_mount *,
> struct block_device *, struct dax_device *);
> extern void xfs_free_buftarg(struct xfs_buftarg *);
> +extern void xfs_buftarg_wait(struct xfs_buftarg *);
> extern void xfs_buftarg_drain(struct xfs_buftarg *);
> extern int xfs_setsize_buftarg(struct xfs_buftarg *, unsigned int);
>
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 5ad4d5e78019..46ea4017fcec 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -936,13 +936,13 @@ xfs_log_quiesce(
>
> /*
> * The superblock buffer is uncached and while xfs_ail_push_all_sync()
> - * will push it, xfs_buftarg_drain() will not wait for it. Further,
> + * will push it, xfs_buftarg_wait() will not wait for it. Further,
> * xfs_buf_iowait() cannot be used because it was pushed with the
> * XBF_ASYNC flag set, so we need to use a lock/unlock pair to wait for
> * the IO to complete.
> */
> xfs_ail_push_all_sync(mp->m_ail);
> - xfs_buftarg_drain(mp->m_ddev_targp);
> + xfs_buftarg_wait(mp->m_ddev_targp);
> xfs_buf_lock(mp->m_sb_bp);
> xfs_buf_unlock(mp->m_sb_bp);
>
> @@ -962,6 +962,8 @@ xfs_log_unmount(
> {
> xfs_log_quiesce(mp);
>
> + xfs_buftarg_drain(mp->m_ddev_targp);
> +
> xfs_trans_ail_destroy(mp);
>
> xfs_sysfs_del(&mp->m_log->l_kobj);
> --
> 2.26.2
>
