Re: [PATCH 05/11] xfs: use EFI refcount consistently in log recovery

Dave Chinner
2015-08-07T01:52:16Z

On Thu, Aug 06, 2015 at 01:44:26PM -0400, Brian Foster wrote:
> @@ -2933,16 +2933,16 @@ xlog_recover_efi_pass2(
> struct xlog_recover_item *item,
> xfs_lsn_t lsn)
> {
> - int error;
> - xfs_mount_t *mp = log->l_mp;
> - xfs_efi_log_item_t *efip;
> - xfs_efi_log_format_t *efi_formatp;
> + int error;
> + struct xfs_mount *mp = log->l_mp;
> + struct xfs_efi_log_item *efip;
> + struct xfs_efi_log_format *efi_formatp;
>
> efi_formatp = item->ri_buf[0].i_addr;
>
> efip = xfs_efi_init(mp, efi_formatp->efi_nextents);
> - if ((error = xfs_efi_copy_format(&(item->ri_buf[0]),
> - &(efip->efi_format)))) {
> + error = xfs_efi_copy_format(&(item->ri_buf[0]), &(efip->efi_format));
> + if (error) {

You can kill the extra (..) around the variables here.

....

> @@ -2995,18 +2998,18 @@ xlog_recover_efd_pass2(
> efip = (xfs_efi_log_item_t *)lip;
> if (efip->efi_format.efi_id == efi_id) {
> /*
> - * xfs_trans_ail_delete() drops the
> - * AIL lock.
> + * Drop the EFD reference to the EFI. This
> + * removes the EFI from the AIL and frees it.
> */
> - xfs_trans_ail_delete(ailp, lip,
> - SHUTDOWN_CORRUPT_INCORE);
> - xfs_efi_item_free(efip);
> + spin_unlock(&ailp->xa_lock);
> + xfs_efi_release(efip);
> spin_lock(&ailp->xa_lock);

Need to call xfs_efi_release() outside the ailp->xa_lock as
xfs_efi_release() now does the removal of the log item from the AIL
and so can deadlock when taking the ailp->xa_lock.

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com