xfs
[Top] [All Lists]

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

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 6/6] [XFS] Reference count per-ag structures
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 11 Dec 2009 11:50:04 +1100
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20091210234836.GA4774@xxxxxxxxxxxxx>
References: <1259734299-20306-1-git-send-email-david@xxxxxxxxxxxxx> <1259734299-20306-7-git-send-email-david@xxxxxxxxxxxxx> <20091210234836.GA4774@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.18 (2008-05-17)
On Thu, Dec 10, 2009 at 06:48:36PM -0500, Christoph Hellwig wrote:
> On Wed, Dec 02, 2009 at 05:11:39PM +1100, Dave Chinner wrote:
> > -           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.

Will do.

> >  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?

that would require 1000 parallel accesses to the perag structure.
Most accesses are serialised by the AGF/AGI buffer locks, and there
are only a handful of traversal references that can be made, so I
don't think that a reference count of more than 10 is going to be
common in real life.

> Patch looks good, although I'm a bit worried about the performance
> overhead.

None that I've been able to see so far - effectively we are
replacing a rwsem with a spin lock and an atomic op, so I think
the overhead is comparable, perhaps even lower than before.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>