On Thu, Jan 27, 2011 at 12:54:20PM +1100, Dave Chinner wrote:

On Tue, Jan 25, 2011 at 04:52:42AM -0500, Christoph Hellwig wrote:

quoted

On Tue, Jan 25, 2011 at 07:50:44PM +1100, Dave Chinner wrote:

quoted

From: Dave Chinner <redacted>

Commit 368e136 ("xfs: remove duplicate code from dquot reclaim") fails
to unlock the dquot freelist when the number of loop restarts is
exceeded in xfs_qm_dqreclaim_one(). This causes hangs in memory
reclaim. Remove the bogus loop exit check that causes the problem.

The fix looks correct, but it's a bit inconsequential about when
to adhere the retry limit and when not.  Shouldn't we just turn the
exit condition into:

	if (dqout || restarts >= XFS_QM_RECLAIM_MAX_RESTARTS)
		break;

I'm guessing that you are suggesting changing the code increments
and checks restarts to:

	restarts++;
	goto startagain;

otherwise this doesn't make sense as restarts will never go above
XFS_QM_RECLAIM_MAX_RESTARTS at this point in the loop.

Argh, there's one loop case where it does increment restarts without
jumping to startagain. I'm going to change this loop to stack the
loop exit cases rather than do them all individually and making it
difficult to work out what is going on.

So each case will increment restarts itself as necessary, set a
"startagain" flag if a loop restart is required, and set dqpout if a
successful reclaim occurred. That way each case can then jump to the
correctly stacked unlock point and all the loop exit logic is in one
place.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs