From: Chandan Babu R <chandanrlinux@gmail.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 24/28] xfs: refactor intent item iop_recover calls
Date: Wed, 06 May 2020 10:44:49 +0530 [thread overview]
Message-ID: <1832116.EI6kFzkkEO@garuda> (raw)
In-Reply-To: <158864118627.182683.12692460464900065895.stgit@magnolia>
On Tuesday 5 May 2020 6:43:06 AM IST Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Now that we've made the recovered item tests all the same, we can hoist
> the test and the ail locking code to the ->iop_recover caller and call
> the recovery function directly.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/xfs_bmap_item.c | 48 ++++++++++++--------------------------------
> fs/xfs/xfs_extfree_item.c | 44 ++++++++++------------------------------
> fs/xfs/xfs_log_recover.c | 8 ++++++-
> fs/xfs/xfs_refcount_item.c | 46 +++++++++++-------------------------------
> fs/xfs/xfs_rmap_item.c | 45 +++++++++++------------------------------
> 5 files changed, 54 insertions(+), 137 deletions(-)
>
>
> diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
> index 8dd157fc44fa..8f0dc6d550d1 100644
> --- a/fs/xfs/xfs_bmap_item.c
> +++ b/fs/xfs/xfs_bmap_item.c
> @@ -421,25 +421,26 @@ const struct xfs_defer_op_type xfs_bmap_update_defer_type = {
> * We need to update some inode's bmbt.
> */
> STATIC int
> -xfs_bui_recover(
> - struct xfs_trans *parent_tp,
> - struct xfs_bui_log_item *buip)
> +xfs_bui_item_recover(
> + struct xfs_log_item *lip,
> + struct xfs_trans *parent_tp)
> {
> - int error = 0;
> - unsigned int bui_type;
> + struct xfs_bmbt_irec irec;
> + struct xfs_bui_log_item *buip = BUI_ITEM(lip);
> + struct xfs_trans *tp;
> + struct xfs_inode *ip = NULL;
> + struct xfs_mount *mp = parent_tp->t_mountp;
> struct xfs_map_extent *bmap;
> + struct xfs_bud_log_item *budp;
> xfs_fsblock_t startblock_fsb;
> xfs_fsblock_t inode_fsb;
> xfs_filblks_t count;
> - bool op_ok;
> - struct xfs_bud_log_item *budp;
> + xfs_exntst_t state;
> enum xfs_bmap_intent_type type;
> + bool op_ok;
> + unsigned int bui_type;
> int whichfork;
> - xfs_exntst_t state;
> - struct xfs_trans *tp;
> - struct xfs_inode *ip = NULL;
> - struct xfs_bmbt_irec irec;
> - struct xfs_mount *mp = parent_tp->t_mountp;
> + int error = 0;
>
> ASSERT(!test_bit(XFS_LI_RECOVERED, &buip->bui_item.li_flags));
>
> @@ -555,29 +556,6 @@ xfs_bui_recover(
> return error;
> }
>
> -/* Recover the BUI if necessary. */
> -STATIC int
> -xfs_bui_item_recover(
> - struct xfs_log_item *lip,
> - struct xfs_trans *tp)
> -{
> - struct xfs_ail *ailp = lip->li_ailp;
> - struct xfs_bui_log_item *buip = BUI_ITEM(lip);
> - int error;
> -
> - /*
> - * Skip BUIs that we've already processed.
> - */
> - if (test_bit(XFS_LI_RECOVERED, &buip->bui_item.li_flags))
> - return 0;
> -
> - spin_unlock(&ailp->ail_lock);
> - error = xfs_bui_recover(tp, buip);
> - spin_lock(&ailp->ail_lock);
> -
> - return error;
> -}
> -
> STATIC bool
> xfs_bui_item_match(
> struct xfs_log_item *lip,
> diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
> index 635c99fdda85..ec8a79fe6cab 100644
> --- a/fs/xfs/xfs_extfree_item.c
> +++ b/fs/xfs/xfs_extfree_item.c
> @@ -581,16 +581,18 @@ const struct xfs_defer_op_type xfs_agfl_free_defer_type = {
> * the log. We need to free the extents that it describes.
> */
> STATIC int
> -xfs_efi_recover(
> - struct xfs_mount *mp,
> - struct xfs_efi_log_item *efip)
> +xfs_efi_item_recover(
> + struct xfs_log_item *lip,
> + struct xfs_trans *parent_tp)
> {
> - struct xfs_efd_log_item *efdp;
> - struct xfs_trans *tp;
> - int i;
> - int error = 0;
> - xfs_extent_t *extp;
> - xfs_fsblock_t startblock_fsb;
> + struct xfs_efi_log_item *efip = EFI_ITEM(lip);
> + struct xfs_mount *mp = parent_tp->t_mountp;
> + struct xfs_efd_log_item *efdp;
> + struct xfs_trans *tp;
> + struct xfs_extent *extp;
> + xfs_fsblock_t startblock_fsb;
> + int i;
> + int error = 0;
>
> ASSERT(!test_bit(XFS_LI_RECOVERED, &efip->efi_item.li_flags));
>
> @@ -641,30 +643,6 @@ xfs_efi_recover(
> return error;
> }
>
> -/* Recover the EFI if necessary. */
> -STATIC int
> -xfs_efi_item_recover(
> - struct xfs_log_item *lip,
> - struct xfs_trans *tp)
> -{
> - struct xfs_ail *ailp = lip->li_ailp;
> - struct xfs_efi_log_item *efip;
> - int error;
> -
> - /*
> - * Skip EFIs that we've already processed.
> - */
> - efip = container_of(lip, struct xfs_efi_log_item, efi_item);
> - if (test_bit(XFS_LI_RECOVERED, &efip->efi_item.li_flags))
> - return 0;
> -
> - spin_unlock(&ailp->ail_lock);
> - error = xfs_efi_recover(tp->t_mountp, efip);
> - spin_lock(&ailp->ail_lock);
> -
> - return error;
> -}
> -
> STATIC bool
> xfs_efi_item_match(
> struct xfs_log_item *lip,
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index a2c03d87c374..8ff957da2845 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -2667,7 +2667,7 @@ xlog_recover_process_intents(
> struct xfs_ail_cursor cur;
> struct xfs_log_item *lip;
> struct xfs_ail *ailp;
> - int error;
> + int error = 0;
'error' variable's value gets set unconditionally by the return value of
xfs_trans_alloc_empty(). Hence it does not need to be initialized
explicitly. This is also seen in the individual ->iop_recover() methods as
well (However those weren't set by this patch).
Apart from the above nit, the rest looks good to me.
Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>
> #if defined(DEBUG) || defined(XFS_WARN)
> xfs_lsn_t last_lsn;
> #endif
> @@ -2717,7 +2717,11 @@ xlog_recover_process_intents(
> * this routine or else those subsequent intents will get
> * replayed in the wrong order!
> */
> - error = lip->li_ops->iop_recover(lip, parent_tp);
> + if (!test_bit(XFS_LI_RECOVERED, &lip->li_flags)) {
> + spin_unlock(&ailp->ail_lock);
> + error = lip->li_ops->iop_recover(lip, parent_tp);
> + spin_lock(&ailp->ail_lock);
> + }
> if (error)
> goto out;
> lip = xfs_trans_ail_cursor_next(ailp, &cur);
> diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
> index 4b242b3b33a3..fab821fce76b 100644
> --- a/fs/xfs/xfs_refcount_item.c
> +++ b/fs/xfs/xfs_refcount_item.c
> @@ -421,25 +421,26 @@ const struct xfs_defer_op_type xfs_refcount_update_defer_type = {
> * We need to update the refcountbt.
> */
> STATIC int
> -xfs_cui_recover(
> - struct xfs_trans *parent_tp,
> - struct xfs_cui_log_item *cuip)
> +xfs_cui_item_recover(
> + struct xfs_log_item *lip,
> + struct xfs_trans *parent_tp)
> {
> - int i;
> - int error = 0;
> - unsigned int refc_type;
> + struct xfs_bmbt_irec irec;
> + struct xfs_cui_log_item *cuip = CUI_ITEM(lip);
> struct xfs_phys_extent *refc;
> - xfs_fsblock_t startblock_fsb;
> - bool op_ok;
> struct xfs_cud_log_item *cudp;
> struct xfs_trans *tp;
> struct xfs_btree_cur *rcur = NULL;
> - enum xfs_refcount_intent_type type;
> + struct xfs_mount *mp = parent_tp->t_mountp;
> + xfs_fsblock_t startblock_fsb;
> xfs_fsblock_t new_fsb;
> xfs_extlen_t new_len;
> - struct xfs_bmbt_irec irec;
> + unsigned int refc_type;
> + bool op_ok;
> bool requeue_only = false;
> - struct xfs_mount *mp = parent_tp->t_mountp;
> + enum xfs_refcount_intent_type type;
> + int i;
> + int error = 0;
>
> ASSERT(!test_bit(XFS_LI_RECOVERED, &cuip->cui_item.li_flags));
>
> @@ -568,29 +569,6 @@ xfs_cui_recover(
> return error;
> }
>
> -/* Recover the CUI if necessary. */
> -STATIC int
> -xfs_cui_item_recover(
> - struct xfs_log_item *lip,
> - struct xfs_trans *tp)
> -{
> - struct xfs_ail *ailp = lip->li_ailp;
> - struct xfs_cui_log_item *cuip = CUI_ITEM(lip);
> - int error;
> -
> - /*
> - * Skip CUIs that we've already processed.
> - */
> - if (test_bit(XFS_LI_RECOVERED, &cuip->cui_item.li_flags))
> - return 0;
> -
> - spin_unlock(&ailp->ail_lock);
> - error = xfs_cui_recover(tp, cuip);
> - spin_lock(&ailp->ail_lock);
> -
> - return error;
> -}
> -
> STATIC bool
> xfs_cui_item_match(
> struct xfs_log_item *lip,
> diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
> index 625eaf954d74..c9233a220551 100644
> --- a/fs/xfs/xfs_rmap_item.c
> +++ b/fs/xfs/xfs_rmap_item.c
> @@ -464,21 +464,23 @@ const struct xfs_defer_op_type xfs_rmap_update_defer_type = {
> * We need to update the rmapbt.
> */
> STATIC int
> -xfs_rui_recover(
> - struct xfs_mount *mp,
> - struct xfs_rui_log_item *ruip)
> +xfs_rui_item_recover(
> + struct xfs_log_item *lip,
> + struct xfs_trans *parent_tp)
> {
> - int i;
> - int error = 0;
> + struct xfs_rui_log_item *ruip = RUI_ITEM(lip);
> struct xfs_map_extent *rmap;
> - xfs_fsblock_t startblock_fsb;
> - bool op_ok;
> struct xfs_rud_log_item *rudp;
> - enum xfs_rmap_intent_type type;
> - int whichfork;
> - xfs_exntst_t state;
> struct xfs_trans *tp;
> struct xfs_btree_cur *rcur = NULL;
> + struct xfs_mount *mp = parent_tp->t_mountp;
> + xfs_fsblock_t startblock_fsb;
> + enum xfs_rmap_intent_type type;
> + xfs_exntst_t state;
> + bool op_ok;
> + int i;
> + int whichfork;
> + int error = 0;
>
> ASSERT(!test_bit(XFS_LI_RECOVERED, &ruip->rui_item.li_flags));
>
> @@ -583,29 +585,6 @@ xfs_rui_recover(
> return error;
> }
>
> -/* Recover the RUI if necessary. */
> -STATIC int
> -xfs_rui_item_recover(
> - struct xfs_log_item *lip,
> - struct xfs_trans *tp)
> -{
> - struct xfs_ail *ailp = lip->li_ailp;
> - struct xfs_rui_log_item *ruip = RUI_ITEM(lip);
> - int error;
> -
> - /*
> - * Skip RUIs that we've already processed.
> - */
> - if (test_bit(XFS_LI_RECOVERED, &ruip->rui_item.li_flags))
> - return 0;
> -
> - spin_unlock(&ailp->ail_lock);
> - error = xfs_rui_recover(tp->t_mountp, ruip);
> - spin_lock(&ailp->ail_lock);
> -
> - return error;
> -}
> -
> STATIC bool
> xfs_rui_item_match(
> struct xfs_log_item *lip,
>
>
--
chandan
next prev parent reply other threads:[~2020-05-06 5:14 UTC|newest]
Thread overview: 94+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-05 1:10 [PATCH v3 00/28] xfs: refactor log recovery Darrick J. Wong
2020-05-05 1:10 ` [PATCH 01/28] xfs: convert xfs_log_recover_item_t to struct xfs_log_recover_item Darrick J. Wong
2020-05-05 3:33 ` Chandan Babu R
2020-05-06 14:59 ` Christoph Hellwig
2020-05-05 1:10 ` [PATCH 02/28] xfs: refactor log recovery item sorting into a generic dispatch structure Darrick J. Wong
2020-05-05 4:11 ` Chandan Babu R
2020-05-06 15:03 ` Christoph Hellwig
2020-05-06 18:36 ` Darrick J. Wong
2020-05-05 1:10 ` [PATCH 03/28] xfs: refactor log recovery item dispatch for pass2 readhead functions Darrick J. Wong
2020-05-05 4:32 ` Chandan Babu R
2020-05-06 15:04 ` Christoph Hellwig
2020-05-05 1:10 ` [PATCH 04/28] xfs: refactor log recovery item dispatch for pass1 commit functions Darrick J. Wong
2020-05-05 4:40 ` Chandan Babu R
2020-05-06 15:07 ` Christoph Hellwig
2020-05-05 1:11 ` [PATCH 05/28] xfs: refactor log recovery buffer item dispatch for pass2 " Darrick J. Wong
2020-05-05 5:03 ` Chandan Babu R
2020-05-06 15:09 ` Christoph Hellwig
2020-05-05 1:11 ` [PATCH 06/28] xfs: refactor log recovery inode " Darrick J. Wong
2020-05-05 5:09 ` Chandan Babu R
2020-05-06 15:10 ` Christoph Hellwig
2020-05-05 1:11 ` [PATCH 07/28] xfs: refactor log recovery dquot " Darrick J. Wong
2020-05-05 5:13 ` Chandan Babu R
2020-05-06 15:11 ` Christoph Hellwig
2020-05-05 1:11 ` [PATCH 08/28] xfs: refactor log recovery icreate " Darrick J. Wong
2020-05-05 6:10 ` Chandan Babu R
2020-05-06 15:11 ` Christoph Hellwig
2020-05-05 1:11 ` [PATCH 09/28] xfs: refactor log recovery EFI " Darrick J. Wong
2020-05-05 6:46 ` Chandan Babu R
2020-05-06 15:12 ` Christoph Hellwig
2020-05-05 1:11 ` [PATCH 10/28] xfs: refactor log recovery RUI " Darrick J. Wong
2020-05-05 7:02 ` Chandan Babu R
2020-05-06 15:12 ` Christoph Hellwig
2020-05-06 15:13 ` Christoph Hellwig
2020-05-05 1:11 ` [PATCH 11/28] xfs: refactor log recovery CUI " Darrick J. Wong
2020-05-05 7:06 ` Chandan Babu R
2020-05-06 15:13 ` Christoph Hellwig
2020-05-05 1:11 ` [PATCH 12/28] xfs: refactor log recovery BUI " Darrick J. Wong
2020-05-05 7:14 ` Chandan Babu R
2020-05-06 15:14 ` Christoph Hellwig
2020-05-05 1:11 ` [PATCH 13/28] xfs: remove log recovery quotaoff " Darrick J. Wong
2020-05-05 7:32 ` Chandan Babu R
2020-05-06 15:16 ` Christoph Hellwig
2020-05-06 16:48 ` Darrick J. Wong
2020-05-05 1:12 ` [PATCH 14/28] xfs: refactor recovered EFI log item playback Darrick J. Wong
2020-05-05 9:03 ` Chandan Babu R
2020-05-06 15:18 ` Christoph Hellwig
2020-05-06 18:59 ` Darrick J. Wong
2020-05-05 1:12 ` [PATCH 15/28] xfs: refactor recovered RUI " Darrick J. Wong
2020-05-05 9:10 ` Chandan Babu R
2020-05-06 15:18 ` Christoph Hellwig
2020-05-06 15:19 ` Christoph Hellwig
2020-05-05 1:12 ` [PATCH 16/28] xfs: refactor recovered CUI " Darrick J. Wong
2020-05-05 9:29 ` Chandan Babu R
2020-05-05 9:29 ` Chandan Babu R
2020-05-06 15:19 ` Christoph Hellwig
2020-05-05 1:12 ` [PATCH 17/28] xfs: refactor recovered BUI " Darrick J. Wong
2020-05-05 9:49 ` Chandan Babu R
2020-05-06 15:21 ` Christoph Hellwig
2020-05-05 1:12 ` [PATCH 18/28] xfs: refactor unlinked inode recovery Darrick J. Wong
2020-05-05 13:05 ` Chandan Babu R
2020-05-06 15:26 ` Christoph Hellwig
2020-05-06 16:51 ` Darrick J. Wong
2020-05-05 1:12 ` [PATCH 19/28] xfs: refactor xlog_recover_process_unlinked Darrick J. Wong
2020-05-05 13:19 ` Chandan Babu R
2020-05-05 13:30 ` Chandan Babu R
2020-05-06 19:11 ` Darrick J. Wong
2020-05-06 15:27 ` Christoph Hellwig
2020-05-05 1:12 ` [PATCH 20/28] xfs: report iunlink recovery failure upwards Darrick J. Wong
2020-05-05 13:43 ` Chandan Babu R
2020-05-06 15:27 ` Christoph Hellwig
2020-05-05 1:12 ` [PATCH 21/28] xfs: refactor releasing finished intents during log recovery Darrick J. Wong
2020-05-06 4:06 ` Chandan Babu R
2020-05-06 15:29 ` Christoph Hellwig
2020-05-05 1:12 ` [PATCH 22/28] xfs: refactor adding recovered intent items to the log Darrick J. Wong
2020-05-06 15:31 ` Christoph Hellwig
2020-05-06 19:28 ` Darrick J. Wong
2020-05-05 1:12 ` [PATCH 23/28] xfs: refactor intent item RECOVERED flag into the log item Darrick J. Wong
2020-05-06 4:45 ` Chandan Babu R
2020-05-06 15:32 ` Christoph Hellwig
2020-05-05 1:13 ` [PATCH 24/28] xfs: refactor intent item iop_recover calls Darrick J. Wong
2020-05-06 5:14 ` Chandan Babu R [this message]
2020-05-06 15:34 ` Christoph Hellwig
2020-05-05 1:13 ` [PATCH 25/28] xfs: hoist setting of XFS_LI_RECOVERED to caller Darrick J. Wong
2020-05-06 5:34 ` Chandan Babu R
2020-05-06 15:35 ` Christoph Hellwig
2020-05-05 1:13 ` [PATCH 26/28] xfs: move log recovery buffer cancellation code to xfs_buf_item_recover.c Darrick J. Wong
2020-05-06 6:42 ` Chandan Babu R
2020-05-06 15:35 ` Christoph Hellwig
2020-05-05 1:13 ` [PATCH 27/28] xfs: remove unnecessary includes from xfs_log_recover.c Darrick J. Wong
2020-05-06 7:21 ` Chandan Babu R
2020-05-05 1:13 ` [PATCH 28/28] xfs: use parallel processing to clear unlinked metadata Darrick J. Wong
2020-05-06 7:57 ` Chandan Babu R
2020-05-06 15:36 ` Christoph Hellwig
2020-05-06 16:54 ` Darrick J. Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1832116.EI6kFzkkEO@garuda \
--to=chandanrlinux@gmail.com \
--cc=darrick.wong@oracle.com \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.