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

Brian Foster
2015-08-07T12:10:00Z

On Fri, Aug 07, 2015 at 11:51:33AM +1000, Dave Chinner wrote:
> 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.
>

Ok.

> ....
>
> > @@ -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.
>

It drops and reacquires the lock, hence the removal of the comment about
xfs_trans_ail_delete() dropping it. ;)

Brian

> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs