Re: [PATCH 1/7] xfs: remove the EFD size asserts in xlog_recover_efd_commit_pass2

["Darrick J. Wong" <djwong@kernel.org>]

On Mon, Apr 19, 2021 at 10:27:58AM +0200, Christoph Hellwig wrote:
> We never actually look at the extent array in the efd items, and should
> eventually stop writing them out at all when it is time for a incompat
> log change.  Ѕo don't bother with the asserts at all, and thus with the
> the structures defined just to be used with it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Seems reasonable enough...
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_log_format.h | 20 ++------------------
>  fs/xfs/xfs_extfree_item.c      | 10 ++--------
>  fs/xfs/xfs_extfree_item.h      |  2 +-
>  fs/xfs/xfs_ondisk.h            |  2 --
>  4 files changed, 5 insertions(+), 29 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
> index 8bd00da6d2a40f..ea0fe9f121adff 100644
> --- a/fs/xfs/libxfs/xfs_log_format.h
> +++ b/fs/xfs/libxfs/xfs_log_format.h
> @@ -598,29 +598,13 @@ typedef struct xfs_efi_log_format_64 {
>   * log.  The efd_extents array is a variable size array whose
>   * size is given by efd_nextents;
>   */
> -typedef struct xfs_efd_log_format {
> +struct xfs_efd_log_format {
>  	uint16_t		efd_type;	/* efd log item type */
>  	uint16_t		efd_size;	/* size of this item */
>  	uint32_t		efd_nextents;	/* # of extents freed */
>  	uint64_t		efd_efi_id;	/* id of corresponding efi */
>  	xfs_extent_t		efd_extents[1];	/* array of extents freed */
> -} xfs_efd_log_format_t;
> -
> -typedef struct xfs_efd_log_format_32 {
> -	uint16_t		efd_type;	/* efd log item type */
> -	uint16_t		efd_size;	/* size of this item */
> -	uint32_t		efd_nextents;	/* # of extents freed */
> -	uint64_t		efd_efi_id;	/* id of corresponding efi */
> -	xfs_extent_32_t		efd_extents[1];	/* array of extents freed */
> -} __attribute__((packed)) xfs_efd_log_format_32_t;
> -
> -typedef struct xfs_efd_log_format_64 {
> -	uint16_t		efd_type;	/* efd log item type */
> -	uint16_t		efd_size;	/* size of this item */
> -	uint32_t		efd_nextents;	/* # of extents freed */
> -	uint64_t		efd_efi_id;	/* id of corresponding efi */
> -	xfs_extent_64_t		efd_extents[1];	/* array of extents freed */
> -} xfs_efd_log_format_64_t;
> +};
>  
>  /*
>   * RUI/RUD (reverse mapping) log format definitions
> diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
> index 93223ebb33721e..ac17fdb9283489 100644
> --- a/fs/xfs/xfs_extfree_item.c
> +++ b/fs/xfs/xfs_extfree_item.c
> @@ -253,7 +253,7 @@ static inline int
>  xfs_efd_item_sizeof(
>  	struct xfs_efd_log_item *efdp)
>  {
> -	return sizeof(xfs_efd_log_format_t) +
> +	return sizeof(struct xfs_efd_log_format) +
>  	       (efdp->efd_format.efd_nextents - 1) * sizeof(xfs_extent_t);
>  }
>  
> @@ -743,13 +743,7 @@ xlog_recover_efd_commit_pass2(
>  	struct xlog_recover_item	*item,
>  	xfs_lsn_t			lsn)
>  {
> -	struct xfs_efd_log_format	*efd_formatp;
> -
> -	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)))));
> +	struct xfs_efd_log_format	*efd_formatp = item->ri_buf[0].i_addr;
>  
>  	xlog_recover_release_intent(log, XFS_LI_EFI, efd_formatp->efd_efi_id);
>  	return 0;
> diff --git a/fs/xfs/xfs_extfree_item.h b/fs/xfs/xfs_extfree_item.h
> index cd2860c875bf50..6b80452ad2a71b 100644
> --- a/fs/xfs/xfs_extfree_item.h
> +++ b/fs/xfs/xfs_extfree_item.h
> @@ -61,7 +61,7 @@ struct xfs_efd_log_item {
>  	struct xfs_log_item	efd_item;
>  	struct xfs_efi_log_item *efd_efip;
>  	uint			efd_next_extent;
> -	xfs_efd_log_format_t	efd_format;
> +	struct xfs_efd_log_format efd_format;
>  };
>  
>  /*
> diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> index 0aa87c2101049c..7328ff92e0ee8a 100644
> --- a/fs/xfs/xfs_ondisk.h
> +++ b/fs/xfs/xfs_ondisk.h
> @@ -118,8 +118,6 @@ xfs_check_ondisk_structs(void)
>  	/* log structures */
>  	XFS_CHECK_STRUCT_SIZE(struct xfs_buf_log_format,	88);
>  	XFS_CHECK_STRUCT_SIZE(struct xfs_dq_logformat,		24);
> -	XFS_CHECK_STRUCT_SIZE(struct xfs_efd_log_format_32,	28);
> -	XFS_CHECK_STRUCT_SIZE(struct xfs_efd_log_format_64,	32);
>  	XFS_CHECK_STRUCT_SIZE(struct xfs_efi_log_format_32,	28);
>  	XFS_CHECK_STRUCT_SIZE(struct xfs_efi_log_format_64,	32);
>  	XFS_CHECK_STRUCT_SIZE(struct xfs_extent_32,		12);
> -- 
> 2.30.1
>