From: Brian Foster <bfoster@redhat.com>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 05/11] xfs: merge the ->log_item defer op into ->create_intent
Date: Thu, 30 Apr 2020 07:04:02 -0400 [thread overview]
Message-ID: <20200430110402.GE5349@bfoster> (raw)
In-Reply-To: <20200429150511.2191150-6-hch@lst.de>
On Wed, Apr 29, 2020 at 05:05:05PM +0200, Christoph Hellwig wrote:
> These are aways called together, and my merging them we reduce the amount
> of indirect calls, improve type safety and in general clean up the code
> a bit.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/libxfs/xfs_defer.c | 6 ++---
> fs/xfs/libxfs/xfs_defer.h | 4 ++--
> fs/xfs/xfs_bmap_item.c | 47 +++++++++++++++---------------------
> fs/xfs/xfs_extfree_item.c | 49 ++++++++++++++++----------------------
> fs/xfs/xfs_refcount_item.c | 48 ++++++++++++++++---------------------
> fs/xfs/xfs_rmap_item.c | 48 ++++++++++++++++---------------------
> 6 files changed, 83 insertions(+), 119 deletions(-)
>
...
> diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
> index ee6f4229cebc4..dea97956d78d6 100644
> --- a/fs/xfs/xfs_bmap_item.c
> +++ b/fs/xfs/xfs_bmap_item.c
...
> @@ -355,6 +330,23 @@ xfs_bmap_update_log_item(
> bmap->bi_bmap.br_state);
> }
>
> +STATIC void *
> +xfs_bmap_update_create_intent(
> + struct xfs_trans *tp,
> + struct list_head *items,
> + unsigned int count)
> +{
> + struct xfs_bui_log_item *buip = xfs_bui_init(tp->t_mountp);
I'd prefer to see these _init() calls separate from the variable
declarations, but otherwise nice cleanup:
Reviewed-by: Brian Foster <bfoster@redhat.com>
> + struct xfs_bmap_intent *bmap;
> +
> + ASSERT(count == XFS_BUI_MAX_FAST_EXTENTS);
> +
> + xfs_trans_add_item(tp, &buip->bui_item);
> + list_for_each_entry(bmap, items, bi_list)
> + xfs_bmap_update_log_item(tp, buip, bmap);
> + return buip;
> +}
> +
> /* Get an BUD so we can process all the deferred rmap updates. */
> STATIC void *
> xfs_bmap_update_create_done(
> @@ -419,7 +411,6 @@ const struct xfs_defer_op_type xfs_bmap_update_defer_type = {
> .diff_items = xfs_bmap_update_diff_items,
> .create_intent = xfs_bmap_update_create_intent,
> .abort_intent = xfs_bmap_update_abort_intent,
> - .log_item = xfs_bmap_update_log_item,
> .create_done = xfs_bmap_update_create_done,
> .finish_item = xfs_bmap_update_finish_item,
> .cancel_item = xfs_bmap_update_cancel_item,
> diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
> index 00309b81607cd..cb22c7ad31817 100644
> --- a/fs/xfs/xfs_extfree_item.c
> +++ b/fs/xfs/xfs_extfree_item.c
> @@ -412,41 +412,16 @@ xfs_extent_free_diff_items(
> XFS_FSB_TO_AGNO(mp, rb->xefi_startblock);
> }
>
> -/* Get an EFI. */
> -STATIC void *
> -xfs_extent_free_create_intent(
> - struct xfs_trans *tp,
> - unsigned int count)
> -{
> - struct xfs_efi_log_item *efip;
> -
> - ASSERT(tp != NULL);
> - ASSERT(count > 0);
> -
> - efip = xfs_efi_init(tp->t_mountp, count);
> - ASSERT(efip != NULL);
> -
> - /*
> - * Get a log_item_desc to point at the new item.
> - */
> - xfs_trans_add_item(tp, &efip->efi_item);
> - return efip;
> -}
> -
> /* Log a free extent to the intent item. */
> STATIC void
> xfs_extent_free_log_item(
> struct xfs_trans *tp,
> - void *intent,
> - struct list_head *item)
> + struct xfs_efi_log_item *efip,
> + struct xfs_extent_free_item *free)
> {
> - struct xfs_efi_log_item *efip = intent;
> - struct xfs_extent_free_item *free;
> uint next_extent;
> struct xfs_extent *extp;
>
> - free = container_of(item, struct xfs_extent_free_item, xefi_list);
> -
> tp->t_flags |= XFS_TRANS_DIRTY;
> set_bit(XFS_LI_DIRTY, &efip->efi_item.li_flags);
>
> @@ -462,6 +437,24 @@ xfs_extent_free_log_item(
> extp->ext_len = free->xefi_blockcount;
> }
>
> +STATIC void *
> +xfs_extent_free_create_intent(
> + struct xfs_trans *tp,
> + struct list_head *items,
> + unsigned int count)
> +{
> + struct xfs_mount *mp = tp->t_mountp;
> + struct xfs_efi_log_item *efip = xfs_efi_init(mp, count);
> + struct xfs_extent_free_item *free;
> +
> + ASSERT(count > 0);
> +
> + xfs_trans_add_item(tp, &efip->efi_item);
> + list_for_each_entry(free, items, xefi_list)
> + xfs_extent_free_log_item(tp, efip, free);
> + return efip;
> +}
> +
> /* Get an EFD so we can process all the free extents. */
> STATIC void *
> xfs_extent_free_create_done(
> @@ -516,7 +509,6 @@ const struct xfs_defer_op_type xfs_extent_free_defer_type = {
> .diff_items = xfs_extent_free_diff_items,
> .create_intent = xfs_extent_free_create_intent,
> .abort_intent = xfs_extent_free_abort_intent,
> - .log_item = xfs_extent_free_log_item,
> .create_done = xfs_extent_free_create_done,
> .finish_item = xfs_extent_free_finish_item,
> .cancel_item = xfs_extent_free_cancel_item,
> @@ -582,7 +574,6 @@ const struct xfs_defer_op_type xfs_agfl_free_defer_type = {
> .diff_items = xfs_extent_free_diff_items,
> .create_intent = xfs_extent_free_create_intent,
> .abort_intent = xfs_extent_free_abort_intent,
> - .log_item = xfs_extent_free_log_item,
> .create_done = xfs_extent_free_create_done,
> .finish_item = xfs_agfl_free_finish_item,
> .cancel_item = xfs_extent_free_cancel_item,
> diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
> index 8eeed73928cdf..325d49fc0406c 100644
> --- a/fs/xfs/xfs_refcount_item.c
> +++ b/fs/xfs/xfs_refcount_item.c
> @@ -284,27 +284,6 @@ xfs_refcount_update_diff_items(
> XFS_FSB_TO_AGNO(mp, rb->ri_startblock);
> }
>
> -/* Get an CUI. */
> -STATIC void *
> -xfs_refcount_update_create_intent(
> - struct xfs_trans *tp,
> - unsigned int count)
> -{
> - struct xfs_cui_log_item *cuip;
> -
> - ASSERT(tp != NULL);
> - ASSERT(count > 0);
> -
> - cuip = xfs_cui_init(tp->t_mountp, count);
> - ASSERT(cuip != NULL);
> -
> - /*
> - * Get a log_item_desc to point at the new item.
> - */
> - xfs_trans_add_item(tp, &cuip->cui_item);
> - return cuip;
> -}
> -
> /* Set the phys extent flags for this reverse mapping. */
> static void
> xfs_trans_set_refcount_flags(
> @@ -328,16 +307,12 @@ xfs_trans_set_refcount_flags(
> STATIC void
> xfs_refcount_update_log_item(
> struct xfs_trans *tp,
> - void *intent,
> - struct list_head *item)
> + struct xfs_cui_log_item *cuip,
> + struct xfs_refcount_intent *refc)
> {
> - struct xfs_cui_log_item *cuip = intent;
> - struct xfs_refcount_intent *refc;
> uint next_extent;
> struct xfs_phys_extent *ext;
>
> - refc = container_of(item, struct xfs_refcount_intent, ri_list);
> -
> tp->t_flags |= XFS_TRANS_DIRTY;
> set_bit(XFS_LI_DIRTY, &cuip->cui_item.li_flags);
>
> @@ -354,6 +329,24 @@ xfs_refcount_update_log_item(
> xfs_trans_set_refcount_flags(ext, refc->ri_type);
> }
>
> +STATIC void *
> +xfs_refcount_update_create_intent(
> + struct xfs_trans *tp,
> + struct list_head *items,
> + unsigned int count)
> +{
> + struct xfs_mount *mp = tp->t_mountp;
> + struct xfs_cui_log_item *cuip = xfs_cui_init(mp, count);
> + struct xfs_refcount_intent *refc;
> +
> + ASSERT(count > 0);
> +
> + xfs_trans_add_item(tp, &cuip->cui_item);
> + list_for_each_entry(refc, items, ri_list)
> + xfs_refcount_update_log_item(tp, cuip, refc);
> + return cuip;
> +}
> +
> /* Get an CUD so we can process all the deferred refcount updates. */
> STATIC void *
> xfs_refcount_update_create_done(
> @@ -432,7 +425,6 @@ const struct xfs_defer_op_type xfs_refcount_update_defer_type = {
> .diff_items = xfs_refcount_update_diff_items,
> .create_intent = xfs_refcount_update_create_intent,
> .abort_intent = xfs_refcount_update_abort_intent,
> - .log_item = xfs_refcount_update_log_item,
> .create_done = xfs_refcount_update_create_done,
> .finish_item = xfs_refcount_update_finish_item,
> .finish_cleanup = xfs_refcount_update_finish_cleanup,
> diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
> index 4911b68f95dda..842d817f5168c 100644
> --- a/fs/xfs/xfs_rmap_item.c
> +++ b/fs/xfs/xfs_rmap_item.c
> @@ -352,41 +352,16 @@ xfs_rmap_update_diff_items(
> XFS_FSB_TO_AGNO(mp, rb->ri_bmap.br_startblock);
> }
>
> -/* Get an RUI. */
> -STATIC void *
> -xfs_rmap_update_create_intent(
> - struct xfs_trans *tp,
> - unsigned int count)
> -{
> - struct xfs_rui_log_item *ruip;
> -
> - ASSERT(tp != NULL);
> - ASSERT(count > 0);
> -
> - ruip = xfs_rui_init(tp->t_mountp, count);
> - ASSERT(ruip != NULL);
> -
> - /*
> - * Get a log_item_desc to point at the new item.
> - */
> - xfs_trans_add_item(tp, &ruip->rui_item);
> - return ruip;
> -}
> -
> /* Log rmap updates in the intent item. */
> STATIC void
> xfs_rmap_update_log_item(
> struct xfs_trans *tp,
> - void *intent,
> - struct list_head *item)
> + struct xfs_rui_log_item *ruip,
> + struct xfs_rmap_intent *rmap)
> {
> - struct xfs_rui_log_item *ruip = intent;
> - struct xfs_rmap_intent *rmap;
> uint next_extent;
> struct xfs_map_extent *map;
>
> - rmap = container_of(item, struct xfs_rmap_intent, ri_list);
> -
> tp->t_flags |= XFS_TRANS_DIRTY;
> set_bit(XFS_LI_DIRTY, &ruip->rui_item.li_flags);
>
> @@ -406,6 +381,24 @@ xfs_rmap_update_log_item(
> rmap->ri_bmap.br_state);
> }
>
> +STATIC void *
> +xfs_rmap_update_create_intent(
> + struct xfs_trans *tp,
> + struct list_head *items,
> + unsigned int count)
> +{
> + struct xfs_mount *mp = tp->t_mountp;
> + struct xfs_rui_log_item *ruip = xfs_rui_init(mp, count);
> + struct xfs_rmap_intent *rmap;
> +
> + ASSERT(count > 0);
> +
> + xfs_trans_add_item(tp, &ruip->rui_item);
> + list_for_each_entry(rmap, items, ri_list)
> + xfs_rmap_update_log_item(tp, ruip, rmap);
> + return ruip;
> +}
> +
> /* Get an RUD so we can process all the deferred rmap updates. */
> STATIC void *
> xfs_rmap_update_create_done(
> @@ -476,7 +469,6 @@ const struct xfs_defer_op_type xfs_rmap_update_defer_type = {
> .diff_items = xfs_rmap_update_diff_items,
> .create_intent = xfs_rmap_update_create_intent,
> .abort_intent = xfs_rmap_update_abort_intent,
> - .log_item = xfs_rmap_update_log_item,
> .create_done = xfs_rmap_update_create_done,
> .finish_item = xfs_rmap_update_finish_item,
> .finish_cleanup = xfs_rmap_update_finish_cleanup,
> --
> 2.26.2
>
next prev parent reply other threads:[~2020-04-30 11:04 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-29 15:05 deferred operations cleanup Christoph Hellwig
2020-04-29 15:05 ` [PATCH 01/11] xfs: remove the xfs_efi_log_item_t typedef Christoph Hellwig
2020-04-30 11:02 ` Brian Foster
2020-04-29 15:05 ` [PATCH 02/11] xfs: remove the xfs_efd_log_item_t typedef Christoph Hellwig
2020-04-30 11:03 ` Brian Foster
2020-04-29 15:05 ` [PATCH 03/11] xfs: remove the xfs_inode_log_item_t typedef Christoph Hellwig
2020-04-30 11:03 ` Brian Foster
2020-04-29 15:05 ` [PATCH 04/11] xfs: factor out a xfs_defer_create_intent helper Christoph Hellwig
2020-04-30 11:03 ` Brian Foster
2020-04-29 15:05 ` [PATCH 05/11] xfs: merge the ->log_item defer op into ->create_intent Christoph Hellwig
2020-04-30 11:04 ` Brian Foster [this message]
2020-04-29 15:05 ` [PATCH 06/11] xfs: merge the ->diff_items " Christoph Hellwig
2020-04-30 11:04 ` Brian Foster
2020-04-29 15:05 ` [PATCH 07/11] xfs: turn dfp_intent into a xfs_log_item Christoph Hellwig
2020-04-30 11:04 ` Brian Foster
2020-04-29 15:05 ` [PATCH 08/11] xfs: refactor xfs_defer_finish_noroll Christoph Hellwig
2020-04-30 11:04 ` Brian Foster
2020-04-29 15:05 ` [PATCH 09/11] xfs: turn dfp_done into a xfs_log_item Christoph Hellwig
2020-04-30 11:04 ` Brian Foster
2020-04-29 15:05 ` [PATCH 10/11] xfs: use a xfs_btree_cur for the ->finish_cleanup state Christoph Hellwig
2020-04-30 11:04 ` Brian Foster
2020-04-29 15:05 ` [PATCH 11/11] xfs: spell out the parameter name for ->cancel_item Christoph Hellwig
2020-04-30 11:04 ` Brian Foster
2020-04-30 15:35 ` deferred operations cleanup 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=20200430110402.GE5349@bfoster \
--to=bfoster@redhat.com \
--cc=hch@lst.de \
--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.