From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay.sgi.com (relay3.corp.sgi.com [198.149.34.15]) by oss.sgi.com (Postfix) with ESMTP id 53C767F53 for ; Mon, 31 Mar 2014 15:30:42 -0500 (CDT) Received: from eagdhcp-232-178.americas.sgi.com (eagdhcp-232-178.americas.sgi.com [128.162.232.178]) by relay3.corp.sgi.com (Postfix) with ESMTP id C4597AC002 for ; Mon, 31 Mar 2014 13:30:41 -0700 (PDT) Received: from eagdhcp-232-178.americas.sgi.com (localhost [127.0.0.1]) by eagdhcp-232-178.americas.sgi.com (8.14.5/8.14.5) with ESMTP id s2VKUfW9002497 for ; Mon, 31 Mar 2014 15:30:41 -0500 (CDT) (envelope-from tinguely@eagdhcp-232-178.americas.sgi.com) Message-Id: <20140331202615.264666182@sgi.com> Date: Mon, 31 Mar 2014 15:25:56 -0500 From: Mark Tinguely Subject: [PATCH v2 1/2] xfs: remove efi from AIL in log recovery References: <20140325195819.638326569@sgi.com> Content-Disposition: inline; filename=xfs-fix-efi-in-log-recovery-error.patch List-Id: XFS Filesystem from SGI List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: xfs-bounces@oss.sgi.com Sender: xfs-bounces@oss.sgi.com To: xfs@oss.sgi.com xlog_recover_process_efi{s}() functions are completing the second half of xfs_bmap_finish() which frees extents. If this operation fails, the EFI will stay on the AIL and prevents the xfs_ail_push all_sync() from completing and the mount will fail to unmount. Rather than have a special log recovery flag XFS_EFI_RECOVERED to decrement the EFI/EFD counter, call the same decrement function from the log recovery routine that is called then the EFI is added to the AIL from a log write. Remove all other unprocessed EFIs from the log recovery AIL when one is discovered in error. Signed-off-by: Mark Tinguely --- v2 per Brian's feedback: decrement the EFI counter again when removing EFI entries after the entry with the error. add comment in xlog_recover_process_efis() where we delete the EFIs. v1: Rewritten with suggestions from Dave. Note: calling xfs_efi_item_unpin() seemed more explainatory than calling the helper __xfs_efi_release(). fs/xfs/xfs_extfree_item.c | 9 +++------ fs/xfs/xfs_log_recover.c | 37 ++++++++++++++++++++++++------------- fs/xfs/xfs_trans.h | 1 + 3 files changed, 28 insertions(+), 19 deletions(-) Index: b/fs/xfs/xfs_extfree_item.c =================================================================== --- a/fs/xfs/xfs_extfree_item.c +++ b/fs/xfs/xfs_extfree_item.c @@ -134,9 +134,10 @@ xfs_efi_item_pin( * remove the EFI it's because the transaction has been cancelled and by * definition that means the EFI cannot be in the AIL so remove it from the * transaction and free it. Otherwise coordinate with xfs_efi_release() - * to determine who gets to free the EFI. + * to determine who gets to free the EFI. Call from log recovery of EFI + * entries so the EFD or error handling will remove the entry. */ -STATIC void +void xfs_efi_item_unpin( struct xfs_log_item *lip, int remove) @@ -313,10 +314,6 @@ xfs_efi_release(xfs_efi_log_item_t *efip { ASSERT(atomic_read(&efip->efi_next_extent) >= nextents); if (atomic_sub_and_test(nextents, &efip->efi_next_extent)) { - /* recovery needs us to drop the EFI reference, too */ - if (test_bit(XFS_EFI_RECOVERED, &efip->efi_flags)) - __xfs_efi_release(efip); - __xfs_efi_release(efip); /* efip may now have been freed, do not reference it again. */ } Index: b/fs/xfs/xfs_log_recover.c =================================================================== --- a/fs/xfs/xfs_log_recover.c +++ b/fs/xfs/xfs_log_recover.c @@ -3634,6 +3634,7 @@ xlog_recover_process_data( /* * Process an extent free intent item that was recovered from * the log. We need to free the extents that it describes. + * The caller will release this and any following EFIs upon error. */ STATIC int xlog_recover_process_efi( @@ -3648,6 +3649,13 @@ xlog_recover_process_efi( xfs_fsblock_t startblock_fsb; ASSERT(!test_bit(XFS_EFI_RECOVERED, &efip->efi_flags)); + set_bit(XFS_EFI_RECOVERED, &efip->efi_flags); + + /* + * Decrement the EFI/EFD counter so the EFI is removed after + * processing the EFD or error handling in the caller. + */ + xfs_efi_item_unpin(&efip->efi_item, 0); /* * First check the validity of the extents described by the @@ -3662,12 +3670,6 @@ xlog_recover_process_efi( (extp->ext_len == 0) || (startblock_fsb >= mp->m_sb.sb_dblocks) || (extp->ext_len >= mp->m_sb.sb_agblocks)) { - /* - * This will pull the EFI from the AIL and - * free the memory associated with it. - */ - set_bit(XFS_EFI_RECOVERED, &efip->efi_flags); - xfs_efi_release(efip, efip->efi_format.efi_nextents); return XFS_ERROR(EIO); } } @@ -3687,7 +3689,6 @@ xlog_recover_process_efi( extp->ext_len); } - set_bit(XFS_EFI_RECOVERED, &efip->efi_flags); error = xfs_trans_commit(tp, 0); return error; @@ -3718,8 +3719,8 @@ STATIC int xlog_recover_process_efis( struct xlog *log) { - xfs_log_item_t *lip; - xfs_efi_log_item_t *efip; + struct xfs_log_item *lip; + struct xfs_efi_log_item *efip; int error = 0; struct xfs_ail_cursor cur; struct xfs_ail *ailp; @@ -3750,13 +3751,23 @@ xlog_recover_process_efis( } spin_unlock(&ailp->xa_lock); - error = xlog_recover_process_efi(log->l_mp, efip); - spin_lock(&ailp->xa_lock); + if (error) { + /* + * An error happened while processing a previous + * EFI entry. Since the extents are freed in order, + * skip the remaining unprocessed EFIs. To do this + * the EFI entry counter must be decremented once + * here and the entry will be decremented and removed + * with the following xfs_efi_release(). + */ + xfs_efi_item_unpin(&efip->efi_item, 0); + } else + error = xlog_recover_process_efi(log->l_mp, efip); if (error) - goto out; + xfs_efi_release(efip, efip->efi_format.efi_nextents); + spin_lock(&ailp->xa_lock); lip = xfs_trans_ail_cursor_next(ailp, &cur); } -out: xfs_trans_ail_cursor_done(ailp, &cur); spin_unlock(&ailp->xa_lock); return error; Index: b/fs/xfs/xfs_trans.h =================================================================== --- a/fs/xfs/xfs_trans.h +++ b/fs/xfs/xfs_trans.h @@ -216,6 +216,7 @@ void xfs_trans_ijoin(struct xfs_trans * void xfs_trans_log_buf(xfs_trans_t *, struct xfs_buf *, uint, uint); void xfs_trans_log_inode(xfs_trans_t *, struct xfs_inode *, uint); struct xfs_efi_log_item *xfs_trans_get_efi(xfs_trans_t *, uint); +void xfs_efi_item_unpin(struct xfs_log_item *, int); void xfs_efi_release(struct xfs_efi_log_item *, uint); void xfs_trans_log_efi_extent(xfs_trans_t *, struct xfs_efi_log_item *, _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs