On Fri, Dec 17, 2010 at 06:22:54AM -0500, Christoph Hellwig wrote: > Looks good, > > Reviewed-by: Christoph Hellwig <hch@xxxxxx> > > Some minor comments below: > > > +STATIC void > > +__xfs_efi_release( > > + struct xfs_efi_log_item *efip) > > +{ > > + struct xfs_ail *ailp = efip->efi_item.li_ailp; > > + > > + if (!test_and_clear_bit(XFS_EFI_COMMITTED, &efip->efi_flags)) { > > + spin_lock(&ailp->xa_lock); > > + /* xfs_trans_ail_delete() drops the AIL lock. */ > > + xfs_trans_ail_delete(ailp, (xfs_log_item_t *)efip); > > The second argument should be &efip->efi_item to preserve ty;e safety. > > > void > > xfs_efi_release(xfs_efi_log_item_t *efip, > > uint nextents) > > { > > + ASSERT(atomic_read(&efip->efi_next_extent) >= nextents); > > + if (!atomic_sub_and_test(nextents, &efip->efi_next_extent)) > > + return; > > > > + __xfs_efi_release(efip); > > Why not just: > > if (atomic_sub_and_test(nextents, &efip->efi_next_extent)) > __xfs_efi_release(efip); > > ? Good idea. Fixed up. Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs