All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 09/11] xfs: turn dfp_done into a xfs_log_item
Date: Thu, 30 Apr 2020 07:04:32 -0400	[thread overview]
Message-ID: <20200430110432.GI5349@bfoster> (raw)
In-Reply-To: <20200429150511.2191150-10-hch@lst.de>

On Wed, Apr 29, 2020 at 05:05:09PM +0200, Christoph Hellwig wrote:
> All defer op instance place their own extension of the log item into
> the dfp_done field.  Replace that with a xfs_log_item to improve type
> safety and make the code easier to follow.
> 
> Also use the opportunity to improve the ->finish_item calling conventions
> to place the done log item as the higher level structure before the
> list_entry used for the individual items.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_defer.c  |  2 +-
>  fs/xfs/libxfs/xfs_defer.h  | 10 +++++-----
>  fs/xfs/xfs_bmap_item.c     |  8 ++++----
>  fs/xfs/xfs_extfree_item.c  | 12 ++++++------
>  fs/xfs/xfs_refcount_item.c |  8 ++++----
>  fs/xfs/xfs_rmap_item.c     |  8 ++++----
>  6 files changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
> index 20950b56cdd07..5f37f42cda67b 100644
> --- a/fs/xfs/libxfs/xfs_defer.c
> +++ b/fs/xfs/libxfs/xfs_defer.c
> @@ -371,7 +371,7 @@ xfs_defer_finish_one(
>  	list_for_each_safe(li, n, &dfp->dfp_work) {
>  		list_del(li);
>  		dfp->dfp_count--;
> -		error = ops->finish_item(tp, li, dfp->dfp_done, &state);
> +		error = ops->finish_item(tp, dfp->dfp_done, li, &state);
>  		if (error == -EAGAIN) {
>  			/*
>  			 * Caller wants a fresh transaction; put the work item
> diff --git a/fs/xfs/libxfs/xfs_defer.h b/fs/xfs/libxfs/xfs_defer.h
> index 7b6cc3808a91b..a86c890e63d20 100644
> --- a/fs/xfs/libxfs/xfs_defer.h
> +++ b/fs/xfs/libxfs/xfs_defer.h
> @@ -29,7 +29,7 @@ struct xfs_defer_pending {
>  	struct list_head		dfp_list;	/* pending items */
>  	struct list_head		dfp_work;	/* work items */
>  	struct xfs_log_item		*dfp_intent;	/* log intent item */
> -	void				*dfp_done;	/* log done item */
> +	struct xfs_log_item		*dfp_done;	/* log done item */
>  	unsigned int			dfp_count;	/* # extent items */
>  	enum xfs_defer_ops_type		dfp_type;
>  };
> @@ -46,10 +46,10 @@ struct xfs_defer_op_type {
>  	struct xfs_log_item *(*create_intent)(struct xfs_trans *tp,
>  			struct list_head *items, unsigned int count, bool sort);
>  	void (*abort_intent)(struct xfs_log_item *intent);
> -	void *(*create_done)(struct xfs_trans *tp, struct xfs_log_item *intent,
> -			unsigned int count);
> -	int (*finish_item)(struct xfs_trans *, struct list_head *, void *,
> -			void **);
> +	struct xfs_log_item *(*create_done)(struct xfs_trans *tp,
> +			struct xfs_log_item *intent, unsigned int count);
> +	int (*finish_item)(struct xfs_trans *tp, struct xfs_log_item *done,
> +			struct list_head *item, void **state);
>  	void (*finish_cleanup)(struct xfs_trans *, void *, int);
>  	void (*cancel_item)(struct list_head *);
>  	unsigned int		max_items;
> diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
> index 7b2153fca2d9e..feadd44a67e4b 100644
> --- a/fs/xfs/xfs_bmap_item.c
> +++ b/fs/xfs/xfs_bmap_item.c
> @@ -352,21 +352,21 @@ xfs_bmap_update_create_intent(
>  }
>  
>  /* Get an BUD so we can process all the deferred rmap updates. */
> -STATIC void *
> +static struct xfs_log_item *
>  xfs_bmap_update_create_done(
>  	struct xfs_trans		*tp,
>  	struct xfs_log_item		*intent,
>  	unsigned int			count)
>  {
> -	return xfs_trans_get_bud(tp, BUI_ITEM(intent));
> +	return &xfs_trans_get_bud(tp, BUI_ITEM(intent))->bud_item;
>  }
>  
>  /* Process a deferred rmap update. */
>  STATIC int
>  xfs_bmap_update_finish_item(
>  	struct xfs_trans		*tp,
> +	struct xfs_log_item		*done,
>  	struct list_head		*item,
> -	void				*done_item,
>  	void				**state)
>  {
>  	struct xfs_bmap_intent		*bmap;
> @@ -375,7 +375,7 @@ xfs_bmap_update_finish_item(
>  
>  	bmap = container_of(item, struct xfs_bmap_intent, bi_list);
>  	count = bmap->bi_bmap.br_blockcount;
> -	error = xfs_trans_log_finish_bmap_update(tp, done_item,
> +	error = xfs_trans_log_finish_bmap_update(tp, BUD_ITEM(done),
>  			bmap->bi_type,
>  			bmap->bi_owner, bmap->bi_whichfork,
>  			bmap->bi_bmap.br_startoff,
> diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
> index 0453b6f2b1d69..633628f70e128 100644
> --- a/fs/xfs/xfs_extfree_item.c
> +++ b/fs/xfs/xfs_extfree_item.c
> @@ -459,28 +459,28 @@ xfs_extent_free_create_intent(
>  }
>  
>  /* Get an EFD so we can process all the free extents. */
> -STATIC void *
> +static struct xfs_log_item *
>  xfs_extent_free_create_done(
>  	struct xfs_trans		*tp,
>  	struct xfs_log_item		*intent,
>  	unsigned int			count)
>  {
> -	return xfs_trans_get_efd(tp, EFI_ITEM(intent), count);
> +	return &xfs_trans_get_efd(tp, EFI_ITEM(intent), count)->efd_item;
>  }
>  
>  /* Process a free extent. */
>  STATIC int
>  xfs_extent_free_finish_item(
>  	struct xfs_trans		*tp,
> +	struct xfs_log_item		*done,
>  	struct list_head		*item,
> -	void				*done_item,
>  	void				**state)
>  {
>  	struct xfs_extent_free_item	*free;
>  	int				error;
>  
>  	free = container_of(item, struct xfs_extent_free_item, xefi_list);
> -	error = xfs_trans_free_extent(tp, done_item,
> +	error = xfs_trans_free_extent(tp, EFD_ITEM(done),
>  			free->xefi_startblock,
>  			free->xefi_blockcount,
>  			&free->xefi_oinfo, free->xefi_skip_discard);
> @@ -523,12 +523,12 @@ const struct xfs_defer_op_type xfs_extent_free_defer_type = {
>  STATIC int
>  xfs_agfl_free_finish_item(
>  	struct xfs_trans		*tp,
> +	struct xfs_log_item		*done,
>  	struct list_head		*item,
> -	void				*done_item,
>  	void				**state)
>  {
>  	struct xfs_mount		*mp = tp->t_mountp;
> -	struct xfs_efd_log_item		*efdp = done_item;
> +	struct xfs_efd_log_item		*efdp = EFD_ITEM(done);
>  	struct xfs_extent_free_item	*free;
>  	struct xfs_extent		*extp;
>  	struct xfs_buf			*agbp;
> diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
> index e8d3278e066e3..f1c2e559a7ae7 100644
> --- a/fs/xfs/xfs_refcount_item.c
> +++ b/fs/xfs/xfs_refcount_item.c
> @@ -351,21 +351,21 @@ xfs_refcount_update_create_intent(
>  }
>  
>  /* Get an CUD so we can process all the deferred refcount updates. */
> -STATIC void *
> +static struct xfs_log_item *
>  xfs_refcount_update_create_done(
>  	struct xfs_trans		*tp,
>  	struct xfs_log_item		*intent,
>  	unsigned int			count)
>  {
> -	return xfs_trans_get_cud(tp, CUI_ITEM(intent));
> +	return &xfs_trans_get_cud(tp, CUI_ITEM(intent))->cud_item;
>  }
>  
>  /* Process a deferred refcount update. */
>  STATIC int
>  xfs_refcount_update_finish_item(
>  	struct xfs_trans		*tp,
> +	struct xfs_log_item		*done,
>  	struct list_head		*item,
> -	void				*done_item,
>  	void				**state)
>  {
>  	struct xfs_refcount_intent	*refc;
> @@ -374,7 +374,7 @@ xfs_refcount_update_finish_item(
>  	int				error;
>  
>  	refc = container_of(item, struct xfs_refcount_intent, ri_list);
> -	error = xfs_trans_log_finish_refcount_update(tp, done_item,
> +	error = xfs_trans_log_finish_refcount_update(tp, CUD_ITEM(done),
>  			refc->ri_type,
>  			refc->ri_startblock,
>  			refc->ri_blockcount,
> diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
> index a417e15fd0ce4..f6a2a388e5ac9 100644
> --- a/fs/xfs/xfs_rmap_item.c
> +++ b/fs/xfs/xfs_rmap_item.c
> @@ -403,28 +403,28 @@ xfs_rmap_update_create_intent(
>  }
>  
>  /* Get an RUD so we can process all the deferred rmap updates. */
> -STATIC void *
> +static struct xfs_log_item *
>  xfs_rmap_update_create_done(
>  	struct xfs_trans		*tp,
>  	struct xfs_log_item		*intent,
>  	unsigned int			count)
>  {
> -	return xfs_trans_get_rud(tp, RUI_ITEM(intent));
> +	return &xfs_trans_get_rud(tp, RUI_ITEM(intent))->rud_item;
>  }
>  
>  /* Process a deferred rmap update. */
>  STATIC int
>  xfs_rmap_update_finish_item(
>  	struct xfs_trans		*tp,
> +	struct xfs_log_item		*done,
>  	struct list_head		*item,
> -	void				*done_item,
>  	void				**state)
>  {
>  	struct xfs_rmap_intent		*rmap;
>  	int				error;
>  
>  	rmap = container_of(item, struct xfs_rmap_intent, ri_list);
> -	error = xfs_trans_log_finish_rmap_update(tp, done_item,
> +	error = xfs_trans_log_finish_rmap_update(tp, RUD_ITEM(done),
>  			rmap->ri_type,
>  			rmap->ri_owner, rmap->ri_whichfork,
>  			rmap->ri_bmap.br_startoff,
> -- 
> 2.26.2
> 


  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
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 [this message]
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=20200430110432.GI5349@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.