On Mon, Dec 05, 2011 at 04:04:28PM +1100, Dave Chinner wrote:
> Ok, so we now mark dquots being freed with a flag, and then avoid
> those inodes during various operations as they dquots are considered
> dead.
>
> That means we can use the fact that nothing new will ever use the
> dquot being freed while it is still active on lists, so we don't
> need to nest locks during reclaim of the dquot to prevent concurrent
> lookups from finding it.
>
> Did i understand the intent correctly?
Yes.
> > + mutex_lock(&xfs_Gqm->qm_dqfrlist_lock);
> > + ASSERT(!list_empty(&dqp->q_freelist));
> > list_del_init(&dqp->q_freelist);
>
> That assert could do with a comment - I had to think hard about why
> that was correct. It's because when the dquot refcount goes to zero it
> is moved onto the free list, so when reclaiming a dquot we should
> always find it on the free list....
Indeed. I'll see if I can come up with a sensible comment.
> > @@ -509,46 +518,27 @@ xfs_qm_dqpurge_int(
> > */
> > xfs_qm_detach_gdquots(mp);
> >
> > - again:
> > - nmisses = 0;
>
> I don't think that nmisses is initialised to zero correctly anymore.
We do a
int nmisses = 0;
at the top of the function. Now that there are no retry loops that's
sufficient.
>
> > - ASSERT(mutex_is_locked(&q->qi_dqlist_lock));
> > /*
> > * Try to get rid of all of the unwanted dquots. The idea is to
> > * get them off mplist and hashlist, but leave them on freelist.
> > */
>
> That comment is no longer correct - they purged dquots don't remain
> on the freelist anymore - they are freed....
I'll fix it up.
> > /*
> > + * Prevent lookups now that we are past the point of no return.
> > */
> > - if (!mutex_trylock(&dqp->q_hash->qh_lock)) {
> > - restarts++;
> > - goto dqfunlock;
> > - }
> > + dqp->dq_flags |= XFS_DQ_FREEING;
> > + xfs_dqunlock(dqp);
>
> Probably worth commenting here that it is safe to access the dquot
> unlocked because we own the XFS_DQ_FREEING flag that guarantees
> nobody else will start using the dquot once we unlock it.
I'll make the comment a bit more verbose.
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
