Re: [PATCH 2/2] xfs: recovery of swap extents operations for CRC filesystems

Mark Tinguely
2013-09-09T20:38:02Z

On 08/29/13 19:23, Dave Chinner wrote:
> From: Dave Chinner<dchinner@redhat.com>
>
> This is the recovery side of the btree block owner change operation
> performed by swapext on CRC enabled filesystems. We detect that an
> owner change is needed by the flag that has been placed on the inode
> log format flag field. Because the inode recovery is being replayed
> after the buffers that make up the BMBT in the given checkpoint, we
> can walk all the buffers and directly modify them when we see the
> flag set on an inode.
>
> Because the inode can be relogged and hence present in multiple
> chekpoints with the "change owner" flag set, we could do multiple
> passes across the inode to do this change. While this isn't optimal,
> we can't directly ignore the flag as there may be multiple
> independent swap extent operations being replayed on the same inode
> in different checkpoints so we can't ignore them.
>
> Further, because the owner change operation uses ordered buffers, we
> might have buffers that are newer on disk than the current
> checkpoint and so already have the owner changed in them. Hence we
> cannot just peek at a buffer in the tree and check that it has the
> correct owner and assume that the change was completed.
>
> So, for the moment just brute force the owner change every time we
> see an inode with the flag set. Note that we have to be careful here
> because the owner of the buffers may point to either the old owner
> or the new owner. Currently the verifier can't verify the owner
> directly, so there is no failure case here right now. If we verify
> the owner exactly in future, then we'll have to take this into
> account.
>

...

Recovery of swap extent makes sense. Variable name, XFS_ILOG_DOWNER,
gave me a chuckle but it is consistent with the other names.

Really could use some recovery test for this. Did inode create recovery
get a test?

Reviewed-by: Mark Tinguely <tinguely@sgi.com>