From patchwork Wed Aug 1 13:19:39 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian Foster X-Patchwork-Id: 10552263 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id A2A1B1708 for ; Wed, 1 Aug 2018 13:19:55 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 937A12B346 for ; Wed, 1 Aug 2018 13:19:55 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 876A42B3C4; Wed, 1 Aug 2018 13:19:55 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C726F2B346 for ; Wed, 1 Aug 2018 13:19:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2389405AbeHAPFj (ORCPT ); Wed, 1 Aug 2018 11:05:39 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:35716 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S2389381AbeHAPFj (ORCPT ); Wed, 1 Aug 2018 11:05:39 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id ACFDC40241C5 for ; Wed, 1 Aug 2018 13:19:52 +0000 (UTC) Received: from bfoster.bos.redhat.com (dhcp-41-2.bos.redhat.com [10.18.41.2]) by smtp.corp.redhat.com (Postfix) with ESMTP id 98C9E2026D68 for ; Wed, 1 Aug 2018 13:19:52 +0000 (UTC) From: Brian Foster To: linux-xfs@vger.kernel.org Subject: [PATCH v2 02/15] xfs: use transaction for intent recovery instead of raw dfops Date: Wed, 1 Aug 2018 09:19:39 -0400 Message-Id: <20180801131952.60250-3-bfoster@redhat.com> In-Reply-To: <20180801131952.60250-1-bfoster@redhat.com> References: <20180801131952.60250-1-bfoster@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Wed, 01 Aug 2018 13:19:52 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Wed, 01 Aug 2018 13:19:52 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'bfoster@redhat.com' RCPT:'' Sender: linux-xfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Log intent recovery is the last user of an external (on-stack) dfops. The pattern exists because the dfops is used to collect additional deferred operations queued during the whole recovery sequence. The dfops is finished with a new transaction after intent recovery completes. We already have a mechanism to create an empty, container-like transaction to support the scrub infrastructure. We can reuse that mechanism here to drop the final user of external dfops. This facilitates folding dfops state (i.e., dop_low) into the transaction, the elimination of now unused external dfops support and also eliminates the only caller of __xfs_defer_cancel(). Replace the on-stack dfops with an empty transaction and pass it around to the various helpers that queue and finish deferred operations during intent recovery. Signed-off-by: Brian Foster Reviewed-by: Christoph Hellwig Reviewed-by: Darrick J. Wong --- fs/xfs/xfs_bmap_item.c | 12 ++++----- fs/xfs/xfs_bmap_item.h | 3 +-- fs/xfs/xfs_log_recover.c | 51 ++++++++++++++++++++++---------------- fs/xfs/xfs_refcount_item.c | 12 ++++----- fs/xfs/xfs_refcount_item.h | 3 +-- 5 files changed, 43 insertions(+), 38 deletions(-) diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c index e1d6c127b07d..57429055e608 100644 --- a/fs/xfs/xfs_bmap_item.c +++ b/fs/xfs/xfs_bmap_item.c @@ -375,9 +375,8 @@ xfs_bud_init( */ int xfs_bui_recover( - struct xfs_mount *mp, - struct xfs_bui_log_item *buip, - struct xfs_defer_ops *dfops) + struct xfs_trans *parent_tp, + struct xfs_bui_log_item *buip) { int error = 0; unsigned int bui_type; @@ -393,6 +392,7 @@ xfs_bui_recover( struct xfs_trans *tp; struct xfs_inode *ip = NULL; struct xfs_bmbt_irec irec; + struct xfs_mount *mp = parent_tp->t_mountp; ASSERT(!test_bit(XFS_BUI_RECOVERED, &buip->bui_flags)); @@ -446,7 +446,7 @@ xfs_bui_recover( * finishes them on completion. Transfer current dfops state to this * transaction and transfer the result back before we return. */ - xfs_defer_move(tp->t_dfops, dfops); + xfs_defer_move(tp->t_dfops, parent_tp->t_dfops); budp = xfs_trans_get_bud(tp, buip); /* Grab the inode. */ @@ -494,7 +494,7 @@ xfs_bui_recover( } set_bit(XFS_BUI_RECOVERED, &buip->bui_flags); - xfs_defer_move(dfops, tp->t_dfops); + xfs_defer_move(parent_tp->t_dfops, tp->t_dfops); error = xfs_trans_commit(tp); xfs_iunlock(ip, XFS_ILOCK_EXCL); xfs_irele(ip); @@ -502,7 +502,7 @@ xfs_bui_recover( return error; err_inode: - xfs_defer_move(dfops, tp->t_dfops); + xfs_defer_move(parent_tp->t_dfops, tp->t_dfops); xfs_trans_cancel(tp); if (ip) { xfs_iunlock(ip, XFS_ILOCK_EXCL); diff --git a/fs/xfs/xfs_bmap_item.h b/fs/xfs/xfs_bmap_item.h index fd1a1b13df51..89e043a88bb8 100644 --- a/fs/xfs/xfs_bmap_item.h +++ b/fs/xfs/xfs_bmap_item.h @@ -79,7 +79,6 @@ struct xfs_bud_log_item *xfs_bud_init(struct xfs_mount *, struct xfs_bui_log_item *); void xfs_bui_item_free(struct xfs_bui_log_item *); void xfs_bui_release(struct xfs_bui_log_item *); -int xfs_bui_recover(struct xfs_mount *mp, struct xfs_bui_log_item *buip, - struct xfs_defer_ops *dfops); +int xfs_bui_recover(struct xfs_trans *parent_tp, struct xfs_bui_log_item *buip); #endif /* __XFS_BMAP_ITEM_H__ */ diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c index 7776fde9430c..fc1ce9a644e3 100644 --- a/fs/xfs/xfs_log_recover.c +++ b/fs/xfs/xfs_log_recover.c @@ -4733,10 +4733,9 @@ xlog_recover_cancel_rui( /* Recover the CUI if necessary. */ STATIC int xlog_recover_process_cui( - struct xfs_mount *mp, + struct xfs_trans *parent_tp, struct xfs_ail *ailp, - struct xfs_log_item *lip, - struct xfs_defer_ops *dfops) + struct xfs_log_item *lip) { struct xfs_cui_log_item *cuip; int error; @@ -4749,7 +4748,7 @@ xlog_recover_process_cui( return 0; spin_unlock(&ailp->ail_lock); - error = xfs_cui_recover(mp, cuip, dfops); + error = xfs_cui_recover(parent_tp, cuip); spin_lock(&ailp->ail_lock); return error; @@ -4774,10 +4773,9 @@ xlog_recover_cancel_cui( /* Recover the BUI if necessary. */ STATIC int xlog_recover_process_bui( - struct xfs_mount *mp, + struct xfs_trans *parent_tp, struct xfs_ail *ailp, - struct xfs_log_item *lip, - struct xfs_defer_ops *dfops) + struct xfs_log_item *lip) { struct xfs_bui_log_item *buip; int error; @@ -4790,7 +4788,7 @@ xlog_recover_process_bui( return 0; spin_unlock(&ailp->ail_lock); - error = xfs_bui_recover(mp, buip, dfops); + error = xfs_bui_recover(parent_tp, buip); spin_lock(&ailp->ail_lock); return error; @@ -4829,9 +4827,9 @@ static inline bool xlog_item_is_intent(struct xfs_log_item *lip) /* Take all the collected deferred ops and finish them in order. */ static int xlog_finish_defer_ops( - struct xfs_mount *mp, - struct xfs_defer_ops *dfops) + struct xfs_trans *parent_tp) { + struct xfs_mount *mp = parent_tp->t_mountp; struct xfs_trans *tp; int64_t freeblks; uint resblks; @@ -4855,7 +4853,7 @@ xlog_finish_defer_ops( if (error) return error; /* transfer all collected dfops to this transaction */ - xfs_defer_move(tp->t_dfops, dfops); + xfs_defer_move(tp->t_dfops, parent_tp->t_dfops); return xfs_trans_commit(tp); } @@ -4880,22 +4878,34 @@ STATIC int xlog_recover_process_intents( struct xlog *log) { - struct xfs_defer_ops dfops; + struct xfs_trans *parent_tp; struct xfs_ail_cursor cur; struct xfs_log_item *lip; struct xfs_ail *ailp; - int error = 0; + int error; #if defined(DEBUG) || defined(XFS_WARN) xfs_lsn_t last_lsn; #endif + /* + * The intent recovery handlers commit transactions to complete recovery + * for individual intents, but any new deferred operations that are + * queued during that process are held off until the very end. The + * purpose of this transaction is to serve as a container for deferred + * operations. Each intent recovery handler must transfer dfops here + * before its local transaction commits, and we'll finish the entire + * list below. + */ + error = xfs_trans_alloc_empty(log->l_mp, &parent_tp); + if (error) + return error; + ailp = log->l_ailp; spin_lock(&ailp->ail_lock); lip = xfs_trans_ail_cursor_first(ailp, &cur, 0); #if defined(DEBUG) || defined(XFS_WARN) last_lsn = xlog_assign_lsn(log->l_curr_cycle, log->l_curr_block); #endif - xfs_defer_init(NULL, &dfops); while (lip != NULL) { /* * We're done when we see something other than an intent. @@ -4930,12 +4940,10 @@ xlog_recover_process_intents( error = xlog_recover_process_rui(log->l_mp, ailp, lip); break; case XFS_LI_CUI: - error = xlog_recover_process_cui(log->l_mp, ailp, lip, - &dfops); + error = xlog_recover_process_cui(parent_tp, ailp, lip); break; case XFS_LI_BUI: - error = xlog_recover_process_bui(log->l_mp, ailp, lip, - &dfops); + error = xlog_recover_process_bui(parent_tp, ailp, lip); break; } if (error) @@ -4945,10 +4953,9 @@ xlog_recover_process_intents( out: xfs_trans_ail_cursor_done(&cur); spin_unlock(&ailp->ail_lock); - if (error) - __xfs_defer_cancel(&dfops); - else - error = xlog_finish_defer_ops(log->l_mp, &dfops); + if (!error) + error = xlog_finish_defer_ops(parent_tp); + xfs_trans_cancel(parent_tp); return error; } diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c index d3582a06626f..011e1d0640fb 100644 --- a/fs/xfs/xfs_refcount_item.c +++ b/fs/xfs/xfs_refcount_item.c @@ -380,9 +380,8 @@ xfs_cud_init( */ int xfs_cui_recover( - struct xfs_mount *mp, - struct xfs_cui_log_item *cuip, - struct xfs_defer_ops *dfops) + struct xfs_trans *parent_tp, + struct xfs_cui_log_item *cuip) { int i; int error = 0; @@ -398,6 +397,7 @@ xfs_cui_recover( xfs_extlen_t new_len; struct xfs_bmbt_irec irec; bool requeue_only = false; + struct xfs_mount *mp = parent_tp->t_mountp; ASSERT(!test_bit(XFS_CUI_RECOVERED, &cuip->cui_flags)); @@ -457,7 +457,7 @@ xfs_cui_recover( * finishes them on completion. Transfer current dfops state to this * transaction and transfer the result back before we return. */ - xfs_defer_move(tp->t_dfops, dfops); + xfs_defer_move(tp->t_dfops, parent_tp->t_dfops); cudp = xfs_trans_get_cud(tp, cuip); for (i = 0; i < cuip->cui_format.cui_nextents; i++) { @@ -522,13 +522,13 @@ xfs_cui_recover( xfs_refcount_finish_one_cleanup(tp, rcur, error); set_bit(XFS_CUI_RECOVERED, &cuip->cui_flags); - xfs_defer_move(dfops, tp->t_dfops); + xfs_defer_move(parent_tp->t_dfops, tp->t_dfops); error = xfs_trans_commit(tp); return error; abort_error: xfs_refcount_finish_one_cleanup(tp, rcur, error); - xfs_defer_move(dfops, tp->t_dfops); + xfs_defer_move(parent_tp->t_dfops, tp->t_dfops); xfs_trans_cancel(tp); return error; } diff --git a/fs/xfs/xfs_refcount_item.h b/fs/xfs/xfs_refcount_item.h index dd830b69cd1e..3896dcc2368f 100644 --- a/fs/xfs/xfs_refcount_item.h +++ b/fs/xfs/xfs_refcount_item.h @@ -82,7 +82,6 @@ struct xfs_cud_log_item *xfs_cud_init(struct xfs_mount *, struct xfs_cui_log_item *); void xfs_cui_item_free(struct xfs_cui_log_item *); void xfs_cui_release(struct xfs_cui_log_item *); -int xfs_cui_recover(struct xfs_mount *mp, struct xfs_cui_log_item *cuip, - struct xfs_defer_ops *dfops); +int xfs_cui_recover(struct xfs_trans *parent_tp, struct xfs_cui_log_item *cuip); #endif /* __XFS_REFCOUNT_ITEM_H__ */