xfs
[Top] [All Lists]

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

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 2/9] xfs: Pull EFI/EFD handling out from under the AIL lock
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Fri, 17 Dec 2010 06:22:54 -0500
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1292214743-18073-3-git-send-email-david@xxxxxxxxxxxxx>
References: <1292214743-18073-1-git-send-email-david@xxxxxxxxxxxxx> <1292214743-18073-3-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
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);

?

<Prev in Thread] Current Thread [Next in Thread>