On Wed, Dec 02, 2009 at 05:11:39PM +1100, Dave Chinner wrote:
> Reference count the per-ag structures to ensure that we keep get/put
> pairs balanced. Assert that the reference counts are zero at unmount
> time to catch leaks. In future, reference counts will enable us to
> safely remove perag structures by allowing us to detect when they
> are no longer in use.
>
> Signed-off-by: Dave Chinner <david@xxxxxxxxxxxxx>
> ---
> fs/xfs/xfs_ag.h | 4 ++--
> fs/xfs/xfs_alloc.c | 2 +-
> fs/xfs/xfs_inode.c | 5 ++++-
> fs/xfs/xfs_mount.c | 1 +
> fs/xfs/xfs_mount.h | 11 +++++++++--
> 5 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/fs/xfs/xfs_ag.h b/fs/xfs/xfs_ag.h
> index a5d54bf..b384a3c 100644
> --- a/fs/xfs/xfs_ag.h
> +++ b/fs/xfs/xfs_ag.h
> index 2d076e2..d1349b1 100644
> --- a/fs/xfs/xfs_alloc.c
> +++ b/fs/xfs/xfs_alloc.c
> @@ -2545,7 +2545,6 @@ xfs_alloc_vextent(
> }
> xfs_perag_put(args->pag);
> }
> - xfs_perag_put(args->pag);
> if (bump_rotor || (type == XFS_ALLOCTYPE_ANY_AG)) {
> if (args->agno == sagno)
> mp->m_agfrotor = (mp->m_agfrotor + 1) %
> @@ -2571,6 +2570,7 @@ xfs_alloc_vextent(
> args->len);
> #endif
> }
> + xfs_perag_put(args->pag);
> return 0;
> error0:
> xfs_perag_put(args->pag);
Should be folded into the earlier patches.
>
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 44a1168..e6c178e 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -2729,7 +2729,7 @@ xfs_iflush_cluster(
> ilist_size = inodes_per_cluster * sizeof(xfs_inode_t *);
> ilist = kmem_alloc(ilist_size, KM_MAYFAIL|KM_NOFS);
> if (!ilist)
> - return 0;
> + goto out_put;
>
> mask = ~(((XFS_INODE_CLUSTER_SIZE(mp) >> mp->m_sb.sb_inodelog)) - 1);
> first_index = XFS_INO_TO_AGINO(mp, ip->i_ino) & mask;
> @@ -2798,6 +2798,8 @@ xfs_iflush_cluster(
> out_free:
> read_unlock(&pag->pag_ici_lock);
> kmem_free(ilist);
> +out_put:
> + xfs_perag_put(pag);
> return 0;
>
>
> @@ -2841,6 +2843,7 @@ cluster_corrupt_out:
> */
> xfs_iflush_abort(iq);
> kmem_free(ilist);
> + xfs_perag_put(pag);
> return XFS_ERROR(EFSCORRUPTED);
Same here.
> static inline xfs_perag_t *
> xfs_perag_get(struct xfs_mount *mp, xfs_agnumber_t agno)
> @@ -393,6 +393,12 @@ xfs_perag_get(struct xfs_mount *mp, xfs_agnumber_t agno)
>
> spin_lock(&mp->m_perag_lock);
> pag = radix_tree_lookup(&mp->m_perag_tree, agno);
> + if (pag) {
> + ASSERT(atomic_read(&pag->pag_ref) >= 0);
> + /* catch leaks in the positive direction during testing */
> + ASSERT(atomic_read(&pag->pag_ref) < 1000);
Is there any good reason why we should not be able to hit this number?
Patch looks good, although I'm a bit worried about the performance
overhead.
|