On Tue, Jan 25, 2011 at 07:50:38PM +1100, Dave Chinner wrote:
> diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
> index 75f2ef6..effbb41 100644
> --- a/fs/xfs/xfs_extfree_item.c
> +++ b/fs/xfs/xfs_extfree_item.c
> @@ -138,7 +138,6 @@ xfs_efi_item_unpin(
>
> if (remove) {
> ASSERT(!(lip->li_flags & XFS_LI_IN_AIL));
> - xfs_trans_del_item(lip);
> xfs_efi_item_free(efip);
> return;
> }
> STATIC void
> xfs_trans_uncommit(
> struct xfs_trans *tp,
> uint flags)
> {
> - struct xfs_log_item_desc *lidp;
> + struct xfs_log_item_desc *lidp, *n;
>
> - list_for_each_entry(lidp, &tp->t_items, lid_trans) {
> + list_for_each_entry_safe(lidp, n, &tp->t_items, lid_trans) {
> /*
> * Unpin all but those that aren't dirty.
> */
> - if (lidp->lid_flags & XFS_LID_DIRTY)
> + if (lidp->lid_flags & XFS_LID_DIRTY) {
> + xfs_trans_del_item(lidp->lid_item);
> IOP_UNPIN(lidp->lid_item, 1);
> + }
This part of the patch isn't explained in the changelog, and I suspect
it should be a separate patch from the addition of the IOP_UNPIN with
remove call to the CIL error path.
Moving the xfs_trans_del_item from the IOP_UNPIN implementation into
the caller seems sane to me, but:
- why is the call to xfs_trans_del_item left in xfs_buf_item_unpin
- why did xfs_buf_item_unpin get away only calling it for the stale
case, and the other log item implementations completely without
it
I suspect the answer lies in xfs_trans_free_items opencoding the
call to xfs_trans_del_item, thus avoiding any leak of log item
descriptors or log items on the transaction list. By adding the
call of xfs_trans_del_item to xfs_trans_uncommit we thus skip
the calls to IOP_UNLOCK for dirty log items, which is a large
change in behaviour for the existing log items that didn't have
the xfs_trans_del_item calls, and at least for the dquot and
inode items seems incorrect.
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
