From: Chandan Babu R <chandanrlinux@gmail.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 21/28] xfs: refactor releasing finished intents during log recovery
Date: Wed, 06 May 2020 09:36:59 +0530 [thread overview]
Message-ID: <1624585.PyqrBo7VhX@garuda> (raw)
In-Reply-To: <158864116741.182683.12547831138234795563.stgit@magnolia>
On Tuesday 5 May 2020 6:42:47 AM IST Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Replace the open-coded AIL item walking with a proper helper when we're
> trying to release an intent item that has been finished.
>
The functionality is the same as was before applying this patch.
Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_log_recover.h | 3 +++
> fs/xfs/xfs_bmap_item.c | 42 +++++++++------------------------------
> fs/xfs/xfs_extfree_item.c | 42 +++++++++------------------------------
> fs/xfs/xfs_log_recover.c | 35 ++++++++++++++++++++++++++++++++-
> fs/xfs/xfs_refcount_item.c | 42 +++++++++------------------------------
> fs/xfs/xfs_rmap_item.c | 42 +++++++++------------------------------
> fs/xfs/xfs_trans.h | 1 +
> 7 files changed, 78 insertions(+), 129 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_log_recover.h b/fs/xfs/libxfs/xfs_log_recover.h
> index d4d6d4f84fda..b875819a1c04 100644
> --- a/fs/xfs/libxfs/xfs_log_recover.h
> +++ b/fs/xfs/libxfs/xfs_log_recover.h
> @@ -126,4 +126,7 @@ bool xlog_put_buffer_cancelled(struct xlog *log, xfs_daddr_t blkno, uint len);
> void xlog_recover_iodone(struct xfs_buf *bp);
> int xlog_recover_process_unlinked(struct xlog *log);
>
> +void xlog_recover_release_intent(struct xlog *log, unsigned short intent_type,
> + uint64_t intent_id);
> +
> #endif /* __XFS_LOG_RECOVER_H__ */
> diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
> index f88ebf8634c4..96627ea800c8 100644
> --- a/fs/xfs/xfs_bmap_item.c
> +++ b/fs/xfs/xfs_bmap_item.c
> @@ -578,12 +578,21 @@ xfs_bui_item_recover(
> return error;
> }
>
> +STATIC bool
> +xfs_bui_item_match(
> + struct xfs_log_item *lip,
> + uint64_t intent_id)
> +{
> + return BUI_ITEM(lip)->bui_format.bui_id == intent_id;
> +}
> +
> static const struct xfs_item_ops xfs_bui_item_ops = {
> .iop_size = xfs_bui_item_size,
> .iop_format = xfs_bui_item_format,
> .iop_unpin = xfs_bui_item_unpin,
> .iop_release = xfs_bui_item_release,
> .iop_recover = xfs_bui_item_recover,
> + .iop_match = xfs_bui_item_match,
> };
>
> /*
> @@ -675,45 +684,14 @@ xlog_recover_bmap_done_commit_pass2(
> xfs_lsn_t lsn)
> {
> struct xfs_bud_log_format *bud_formatp;
> - struct xfs_bui_log_item *buip = NULL;
> - struct xfs_log_item *lip;
> - uint64_t bui_id;
> - struct xfs_ail_cursor cur;
> - struct xfs_ail *ailp = log->l_ailp;
>
> bud_formatp = item->ri_buf[0].i_addr;
> if (item->ri_buf[0].i_len != sizeof(struct xfs_bud_log_format)) {
> XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, log->l_mp);
> return -EFSCORRUPTED;
> }
> - bui_id = bud_formatp->bud_bui_id;
> -
> - /*
> - * Search for the BUI with the id in the BUD format structure in the
> - * AIL.
> - */
> - spin_lock(&ailp->ail_lock);
> - lip = xfs_trans_ail_cursor_first(ailp, &cur, 0);
> - while (lip != NULL) {
> - if (lip->li_type == XFS_LI_BUI) {
> - buip = (struct xfs_bui_log_item *)lip;
> - if (buip->bui_format.bui_id == bui_id) {
> - /*
> - * Drop the BUD reference to the BUI. This
> - * removes the BUI from the AIL and frees it.
> - */
> - spin_unlock(&ailp->ail_lock);
> - xfs_bui_release(buip);
> - spin_lock(&ailp->ail_lock);
> - break;
> - }
> - }
> - lip = xfs_trans_ail_cursor_next(ailp, &cur);
> - }
> -
> - xfs_trans_ail_cursor_done(&cur);
> - spin_unlock(&ailp->ail_lock);
>
> + xlog_recover_release_intent(log, XFS_LI_BUI, bud_formatp->bud_bui_id);
> return 0;
> }
>
> diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
> index 3fc8a9864217..4e1b10ab17a5 100644
> --- a/fs/xfs/xfs_extfree_item.c
> +++ b/fs/xfs/xfs_extfree_item.c
> @@ -665,12 +665,21 @@ xfs_efi_item_recover(
> return error;
> }
>
> +STATIC bool
> +xfs_efi_item_match(
> + struct xfs_log_item *lip,
> + uint64_t intent_id)
> +{
> + return EFI_ITEM(lip)->efi_format.efi_id == intent_id;
> +}
> +
> static const struct xfs_item_ops xfs_efi_item_ops = {
> .iop_size = xfs_efi_item_size,
> .iop_format = xfs_efi_item_format,
> .iop_unpin = xfs_efi_item_unpin,
> .iop_release = xfs_efi_item_release,
> .iop_recover = xfs_efi_item_recover,
> + .iop_match = xfs_efi_item_match,
> };
>
>
> @@ -734,46 +743,15 @@ xlog_recover_extfree_done_commit_pass2(
> struct xlog_recover_item *item,
> xfs_lsn_t lsn)
> {
> - struct xfs_ail_cursor cur;
> struct xfs_efd_log_format *efd_formatp;
> - struct xfs_efi_log_item *efip = NULL;
> - struct xfs_log_item *lip;
> - struct xfs_ail *ailp = log->l_ailp;
> - uint64_t efi_id;
>
> efd_formatp = item->ri_buf[0].i_addr;
> ASSERT((item->ri_buf[0].i_len == (sizeof(xfs_efd_log_format_32_t) +
> ((efd_formatp->efd_nextents - 1) * sizeof(xfs_extent_32_t)))) ||
> (item->ri_buf[0].i_len == (sizeof(xfs_efd_log_format_64_t) +
> ((efd_formatp->efd_nextents - 1) * sizeof(xfs_extent_64_t)))));
> - efi_id = efd_formatp->efd_efi_id;
> -
> - /*
> - * Search for the EFI with the id in the EFD format structure in the
> - * AIL.
> - */
> - spin_lock(&ailp->ail_lock);
> - lip = xfs_trans_ail_cursor_first(ailp, &cur, 0);
> - while (lip != NULL) {
> - if (lip->li_type == XFS_LI_EFI) {
> - efip = (struct xfs_efi_log_item *)lip;
> - if (efip->efi_format.efi_id == efi_id) {
> - /*
> - * Drop the EFD reference to the EFI. This
> - * removes the EFI from the AIL and frees it.
> - */
> - spin_unlock(&ailp->ail_lock);
> - xfs_efi_release(efip);
> - spin_lock(&ailp->ail_lock);
> - break;
> - }
> - }
> - lip = xfs_trans_ail_cursor_next(ailp, &cur);
> - }
> -
> - xfs_trans_ail_cursor_done(&cur);
> - spin_unlock(&ailp->ail_lock);
>
> + xlog_recover_release_intent(log, XFS_LI_EFI, efd_formatp->efd_efi_id);
> return 0;
> }
>
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 0ccc09c004f1..55477b9b9311 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -1779,6 +1779,38 @@ xlog_clear_stale_blocks(
> return 0;
> }
>
> +/*
> + * Release the recovered intent item in the AIL that matches the given intent
> + * type and intent id.
> + */
> +void
> +xlog_recover_release_intent(
> + struct xlog *log,
> + unsigned short intent_type,
> + uint64_t intent_id)
> +{
> + struct xfs_ail_cursor cur;
> + struct xfs_log_item *lip;
> + struct xfs_ail *ailp = log->l_ailp;
> +
> + spin_lock(&ailp->ail_lock);
> + for (lip = xfs_trans_ail_cursor_first(ailp, &cur, 0); lip != NULL;
> + lip = xfs_trans_ail_cursor_next(ailp, &cur)) {
> + if (lip->li_type != intent_type)
> + continue;
> + if (!lip->li_ops->iop_match(lip, intent_id))
> + continue;
> +
> + spin_unlock(&ailp->ail_lock);
> + lip->li_ops->iop_release(lip);
> + spin_lock(&ailp->ail_lock);
> + break;
> + }
> +
> + xfs_trans_ail_cursor_done(&cur);
> + spin_unlock(&ailp->ail_lock);
> +}
> +
> /******************************************************************************
> *
> * Log recover routines
> @@ -2590,7 +2622,8 @@ xlog_finish_defer_ops(
> /* Is this log item a deferred action intent? */
> static inline bool xlog_item_is_intent(struct xfs_log_item *lip)
> {
> - return lip->li_ops->iop_recover != NULL;
> + return lip->li_ops->iop_recover != NULL &&
> + lip->li_ops->iop_match != NULL;
> }
>
> /*
> diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
> index 5b72eebd8764..27126b136b5a 100644
> --- a/fs/xfs/xfs_refcount_item.c
> +++ b/fs/xfs/xfs_refcount_item.c
> @@ -591,12 +591,21 @@ xfs_cui_item_recover(
> return error;
> }
>
> +STATIC bool
> +xfs_cui_item_match(
> + struct xfs_log_item *lip,
> + uint64_t intent_id)
> +{
> + return CUI_ITEM(lip)->cui_format.cui_id == intent_id;
> +}
> +
> static const struct xfs_item_ops xfs_cui_item_ops = {
> .iop_size = xfs_cui_item_size,
> .iop_format = xfs_cui_item_format,
> .iop_unpin = xfs_cui_item_unpin,
> .iop_release = xfs_cui_item_release,
> .iop_recover = xfs_cui_item_recover,
> + .iop_match = xfs_cui_item_match,
> };
>
> /*
> @@ -684,45 +693,14 @@ xlog_recover_refcount_done_commit_pass2(
> xfs_lsn_t lsn)
> {
> struct xfs_cud_log_format *cud_formatp;
> - struct xfs_cui_log_item *cuip = NULL;
> - struct xfs_log_item *lip;
> - uint64_t cui_id;
> - struct xfs_ail_cursor cur;
> - struct xfs_ail *ailp = log->l_ailp;
>
> cud_formatp = item->ri_buf[0].i_addr;
> if (item->ri_buf[0].i_len != sizeof(struct xfs_cud_log_format)) {
> XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, log->l_mp);
> return -EFSCORRUPTED;
> }
> - cui_id = cud_formatp->cud_cui_id;
> -
> - /*
> - * Search for the CUI with the id in the CUD format structure in the
> - * AIL.
> - */
> - spin_lock(&ailp->ail_lock);
> - lip = xfs_trans_ail_cursor_first(ailp, &cur, 0);
> - while (lip != NULL) {
> - if (lip->li_type == XFS_LI_CUI) {
> - cuip = (struct xfs_cui_log_item *)lip;
> - if (cuip->cui_format.cui_id == cui_id) {
> - /*
> - * Drop the CUD reference to the CUI. This
> - * removes the CUI from the AIL and frees it.
> - */
> - spin_unlock(&ailp->ail_lock);
> - xfs_cui_release(cuip);
> - spin_lock(&ailp->ail_lock);
> - break;
> - }
> - }
> - lip = xfs_trans_ail_cursor_next(ailp, &cur);
> - }
> -
> - xfs_trans_ail_cursor_done(&cur);
> - spin_unlock(&ailp->ail_lock);
>
> + xlog_recover_release_intent(log, XFS_LI_CUI, cud_formatp->cud_cui_id);
> return 0;
> }
>
> diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
> index e763dd8ed0a6..3987f217415c 100644
> --- a/fs/xfs/xfs_rmap_item.c
> +++ b/fs/xfs/xfs_rmap_item.c
> @@ -606,12 +606,21 @@ xfs_rui_item_recover(
> return error;
> }
>
> +STATIC bool
> +xfs_rui_item_match(
> + struct xfs_log_item *lip,
> + uint64_t intent_id)
> +{
> + return RUI_ITEM(lip)->rui_format.rui_id == intent_id;
> +}
> +
> static const struct xfs_item_ops xfs_rui_item_ops = {
> .iop_size = xfs_rui_item_size,
> .iop_format = xfs_rui_item_format,
> .iop_unpin = xfs_rui_item_unpin,
> .iop_release = xfs_rui_item_release,
> .iop_recover = xfs_rui_item_recover,
> + .iop_match = xfs_rui_item_match,
> };
>
> /*
> @@ -675,42 +684,11 @@ xlog_recover_rmap_done_commit_pass2(
> xfs_lsn_t lsn)
> {
> struct xfs_rud_log_format *rud_formatp;
> - struct xfs_rui_log_item *ruip = NULL;
> - struct xfs_log_item *lip;
> - uint64_t rui_id;
> - struct xfs_ail_cursor cur;
> - struct xfs_ail *ailp = log->l_ailp;
>
> rud_formatp = item->ri_buf[0].i_addr;
> ASSERT(item->ri_buf[0].i_len == sizeof(struct xfs_rud_log_format));
> - rui_id = rud_formatp->rud_rui_id;
> -
> - /*
> - * Search for the RUI with the id in the RUD format structure in the
> - * AIL.
> - */
> - spin_lock(&ailp->ail_lock);
> - lip = xfs_trans_ail_cursor_first(ailp, &cur, 0);
> - while (lip != NULL) {
> - if (lip->li_type == XFS_LI_RUI) {
> - ruip = (struct xfs_rui_log_item *)lip;
> - if (ruip->rui_format.rui_id == rui_id) {
> - /*
> - * Drop the RUD reference to the RUI. This
> - * removes the RUI from the AIL and frees it.
> - */
> - spin_unlock(&ailp->ail_lock);
> - xfs_rui_release(ruip);
> - spin_lock(&ailp->ail_lock);
> - break;
> - }
> - }
> - lip = xfs_trans_ail_cursor_next(ailp, &cur);
> - }
> -
> - xfs_trans_ail_cursor_done(&cur);
> - spin_unlock(&ailp->ail_lock);
>
> + xlog_recover_release_intent(log, XFS_LI_RUI, rud_formatp->rud_rui_id);
> return 0;
> }
>
> diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
> index 3f6a79108991..3e8808bb07c5 100644
> --- a/fs/xfs/xfs_trans.h
> +++ b/fs/xfs/xfs_trans.h
> @@ -78,6 +78,7 @@ struct xfs_item_ops {
> xfs_lsn_t (*iop_committed)(struct xfs_log_item *, xfs_lsn_t);
> void (*iop_error)(struct xfs_log_item *, xfs_buf_t *);
> int (*iop_recover)(struct xfs_log_item *lip, struct xfs_trans *tp);
> + bool (*iop_match)(struct xfs_log_item *item, uint64_t id);
> };
>
> /*
>
>
--
chandan
next prev parent reply other threads:[~2020-05-06 4:07 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 [this message]
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
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=1624585.PyqrBo7VhX@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox