xfs
[Top] [All Lists]

Re: [RFC PATCH 2/4] xfs: create function to scan and clear EOFBLOCKS ino

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [RFC PATCH 2/4] xfs: create function to scan and clear EOFBLOCKS inodes
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 5 Sep 2012 16:42:02 +1000
Cc: xfs@xxxxxxxxxxx
In-reply-to: <50460BB8.1060701@xxxxxxxxxx>
References: <1346097111-4476-1-git-send-email-bfoster@xxxxxxxxxx> <1346097111-4476-3-git-send-email-bfoster@xxxxxxxxxx> <20120903050612.GR15292@dastard> <50460BB8.1060701@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
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

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