Re: [PATCH 2/9] xfs: Pull EFI/EFD handling out from under the AIL lock

[Dave Chinner <david@xxxxxxxxxxxxx>]

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