On Tue, Sep 22, 2020 at 11:42:49AM +0800, Gao Xiang wrote:
> From: Gao Xiang <hsiangkao@xxxxxxxxxx>
>
> Since commit 1c1c6ebcf52 ("xfs: Replace per-ag array with a radix
> tree"), there is no m_peraglock anymore, so it's hard to understand
> the described situation since per-ag is no longer an array and no
> need to reallocate, call xfs_filestream_flush() in growfs.
>
> In addition, the race condition for shrink feature is quite confusing
> to me currently as well. Get rid of it instead.
>
> Signed-off-by: Gao Xiang <hsiangkao@xxxxxxxxxx>
I'm convinced by the discussion, so:
Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
--D
> ---
> fs/xfs/xfs_filestream.c | 34 +---------------------------------
> 1 file changed, 1 insertion(+), 33 deletions(-)
>
> diff --git a/fs/xfs/xfs_filestream.c b/fs/xfs/xfs_filestream.c
> index 1a88025e68a3..db23e455eb91 100644
> --- a/fs/xfs/xfs_filestream.c
> +++ b/fs/xfs/xfs_filestream.c
> @@ -33,39 +33,7 @@ enum xfs_fstrm_alloc {
> /*
> * Allocation group filestream associations are tracked with per-ag atomic
> * counters. These counters allow xfs_filestream_pick_ag() to tell whether a
> - * particular AG already has active filestreams associated with it. The mount
> - * point's m_peraglock is used to protect these counters from per-ag array
> - * re-allocation during a growfs operation. When xfs_growfs_data_private() is
> - * about to reallocate the array, it calls xfs_filestream_flush() with the
> - * m_peraglock held in write mode.
> - *
> - * Since xfs_mru_cache_flush() guarantees that all the free functions for all
> - * the cache elements have finished executing before it returns, it's safe for
> - * the free functions to use the atomic counters without m_peraglock protection.
> - * This allows the implementation of xfs_fstrm_free_func() to be agnostic about
> - * whether it was called with the m_peraglock held in read mode, write mode or
> - * not held at all. The race condition this addresses is the following:
> - *
> - * - The work queue scheduler fires and pulls a filestream directory cache
> - * element off the LRU end of the cache for deletion, then gets pre-empted.
> - * - A growfs operation grabs the m_peraglock in write mode, flushes all the
> - * remaining items from the cache and reallocates the mount point's per-ag
> - * array, resetting all the counters to zero.
> - * - The work queue thread resumes and calls the free function for the element
> - * it started cleaning up earlier. In the process it decrements the
> - * filestreams counter for an AG that now has no references.
> - *
> - * With a shrinkfs feature, the above scenario could panic the system.
> - *
> - * All other uses of the following macros should be protected by either the
> - * m_peraglock held in read mode, or the cache's internal locking exposed by the
> - * interval between a call to xfs_mru_cache_lookup() and a call to
> - * xfs_mru_cache_done(). In addition, the m_peraglock must be held in read mode
> - * when new elements are added to the cache.
> - *
> - * Combined, these locking rules ensure that no associations will ever exist in
> - * the cache that reference per-ag array elements that have since been
> - * reallocated.
> + * particular AG already has active filestreams associated with it.
> */
> int
> xfs_filestream_peek_ag(
> --
> 2.24.0
>
