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@xxxxxx>
> ---
> 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@xxxxxxxxxx>
> + 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
>
