On Fri, Mar 16, 2012 at 01:55:45PM -0400, Christoph Hellwig wrote:
> Now that we write back all metadata either synchronously or through the AIL
> we can simply implement metadata freezing in terms of emptying the AIL.
>
> The implementation for this is fairly simply and straight-forward: A new
> routine is added that increments a counter that tells xfsaild to not stop
> until the AIL is empty and then waits on a wakeup from
> xfs_trans_ail_delete_bulk to signal that the AIL is empty.
>
> As usual the devil is in the details, in this case the filesystem shutdown
> code. Currently we are a bit sloppy there and do not continue ail pushing
> in that case, and thus never reach the code in the log item implementations
> that can unwind in case of a shutdown filesystem. Also the code to
> abort inode and dquot flushes was rather sloppy before and did not remove
> the log items from the AIL, which had to be fixed as well.
>
> Also treat unmount the same way as freeze now, except that we still keep a
> synchronous inode reclaim pass to make sure we reclaim all clean inodes, too.
>
> As an upside we can now remove the radix tree based inode writeback and
> xfs_unmountfs_writesb.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
.....
> @@ -397,6 +396,15 @@ xfsaild_push(
> XFS_STATS_INC(xs_push_ail);
>
> /*
> + * If we are draining the AIL push all items, not just the current
> + * threshold.
> + */
> + if (atomic_read(&ailp->xa_wait_empty))
> + target = xfs_ail_max(ailp)->li_lsn;
I don't think this is safe - we may have finished pushing the AIL to
empty, but the waiter that decrements xa_wait_empty may not have run
yet and we can race with that. In that case, xfs_ail_max() will
return NULL as the AIL is empty.
.....
> @@ -611,6 +614,34 @@ xfs_ail_push_all(
> }
>
> /*
> + * Push out all items in the AIL immediately and wait until the AIL is empty.
> + */
> +void
> +xfs_ail_push_all_sync(
> + struct xfs_ail *ailp)
> +{
> + DEFINE_WAIT(wait);
> +
> + /*
> + * We use a counter instead of a flag here to support multiple
> + * processes calling into sync at the same time.
> + */
> + atomic_inc(&ailp->xa_wait_empty);
> + do {
> + prepare_to_wait(&ailp->xa_empty, &wait, TASK_KILLABLE);
Why a killable wait? We need to wait until the push is complete
before returning because we can't return an error and we don't want
ctrl-c to break out of this loop while writeback it still taking
place on a non-shutdown based unmount (e.g. got thousands of inodes
to write on a slow RAID5 device doing RMW cycles for every inode).
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
