[PATCH 6/6] XFS: Reference count per-ag structures

Alex Elder aelder at sgi.com
Wed Dec 23 16:12:56 CST 2009


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.

Looks good.  Two comments below; I'll fix up if you want.

> Signed-off-by: Dave Chinner <david at fromorbit.com>

Reviewed-by: Alex Elder <aelder at sgi.com>

> ---
. . .
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index 73d61d4..1d00f7f 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -215,6 +215,7 @@ xfs_free_perag(
>  	for (agno = 0; agno < mp->m_sb.sb_agcount; agno++) {
>  		spin_lock(&mp->m_perag_lock);
>  		pag = radix_tree_delete(&mp->m_perag_tree, agno);
> +		ASSERT(atomic_read(&pag->pag_ref) == 0);
>  		spin_unlock(&mp->m_perag_lock);
>  		ASSERT(pag);

This ASSERT(pag) should be moved up before ASSERT(atomic_read(&pag->pag_ref == 0)

>  		kmem_free(pag->pagb_list);
. . .
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index cfa7a5d..16b2212 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
. . .
> @@ -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);

This is good; what makes 1000 a reasonable number (why not
100 or 10000?)

> +		atomic_inc(&pag->pag_ref);
> +	}
>  	spin_unlock(&mp->m_perag_lock);
>  	return pag;
>  }
. . .




More information about the xfs mailing list