On Tue, Sep 04, 2012 at 10:10:00AM -0400, Brian Foster wrote:
> On 09/03/2012 01:06 AM, Dave Chinner wrote:
> > On Mon, Aug 27, 2012 at 03:51:49PM -0400, Brian Foster wrote:
> ...
> >> +/*
> >> + * Handle an EOFBLOCKS tagged inode. If this is a forced scan, we wait on the
> >> + * iolock ourselves rather than rely on the trylock in xfs_free_eofblocks().
> >> + *
> >> + * We rely on the output parameter from xfs_free_eofblocks() to determine
> >> + * whether we should clear the tag because in the trylock case, it could have
> >> + * skipped the inode due to lock contention.
> >> + */
> >> +STATIC int
> >> +xfs_inode_free_eofblocks(
> >> + struct xfs_inode *ip,
> >> + int flags)
> >> +{
> >> + int ret = 0;
> >> + bool freed = false;
> >> + bool wait_iolock = (flags & EOFBLOCKS_WAIT) ? true : false;
> >> +
> >> + if (wait_iolock)
> >> + xfs_ilock(ip, XFS_IOLOCK_EXCL);
> >
> > Why do we need the IO lock here? xfs_free_eofblocks() does all the
> > necessary locking....
> >
>
> This was for the wait case (e.g., xfs_free_eofblocks() does a trylock
> on the IO lock and we want to wait for the lock in this case).
Does it really matter if we can't get the iolock and skip the inode?
Note that xfs_inode_ag_walk() will keep a skipped count if you
return EAGAIN, and re-run the walk on the AG if any inodes are
skipped. This is done to prevent blocking on locks slowing down the
scan - instead of blocking we move on to the next inode, and revist
the blocked inodes again in a later pass.
IOWs, if you convert the code to use the generic walk and make
xfs_free_eofblocks() return EAGAIN when it can't get the io lock,
you get the retry for free...
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
