Re: [PATCH 7/7] xfs: remove l_iclog_heads

[Brian Foster <bfoster@redhat.com>]

On Tue, Sep 16, 2025 at 06:56:31AM -0700, Christoph Hellwig wrote:
> l_iclog_heads is only used in one place and can be trivially derived
> from l_iclog_hsize by a single shift operation.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_log.c      | 11 +++--------
>  fs/xfs/xfs_log_priv.h |  1 -
>  2 files changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index a8e2539defbf..ca46cdef4ea4 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -1277,13 +1277,8 @@ xlog_get_iclog_buffer_size(
>  
>  	log->l_iclog_bufs = mp->m_logbufs;
>  	log->l_iclog_size = mp->m_logbsize;
> -
> -	/*
> -	 * # headers = size / 32k - one header holds cycles from 32k of data.
> -	 */
> -	log->l_iclog_heads =
> -		DIV_ROUND_UP(mp->m_logbsize, XLOG_HEADER_CYCLE_SIZE);
> -	log->l_iclog_hsize = log->l_iclog_heads << BBSHIFT;
> +	/* combined size of the log record headers: */
> +	log->l_iclog_hsize = DIV_ROUND_UP(mp->m_logbsize, XLOG_CYCLE_DATA_SIZE);

I haven't grokked the full series but this looked suspicious at first
glance because it rounds a byte unit field (logbsize) to a sector unit
value. After some rubber ducking with AI, it suggests this might be
problematic for log buf sizes < 32k (i.e. 16kb is the min supported).
I've since closed the window and lost the output, but I think it
suggested open coding the original calculation:

	hsize = DIV_ROUND_UP(m_logbsize, XLOG_HEADER_CYCLE_SIZE) << BBSHIFT;

Brian

>  }
>  
>  void
> @@ -1534,7 +1529,7 @@ xlog_pack_data(
>  		dp += BBSIZE;
>  	}
>  
> -	for (i = 0; i < log->l_iclog_heads - 1; i++)
> +	for (i = 0; i < (log->l_iclog_hsize >> BBSHIFT) - 1; i++)
>  		rhead->h_ext[i].xh_cycle = cycle_lsn;
>  }
>  
> diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
> index ac98ac71152d..17733ba7f251 100644
> --- a/fs/xfs/xfs_log_priv.h
> +++ b/fs/xfs/xfs_log_priv.h
> @@ -406,7 +406,6 @@ struct xlog {
>  	struct list_head	*l_buf_cancel_table;
>  	struct list_head	r_dfops;	/* recovered log intent items */
>  	int			l_iclog_hsize;  /* size of iclog header */
> -	int			l_iclog_heads;  /* # of iclog header sectors */
>  	uint			l_sectBBsize;   /* sector size in BBs (2^n) */
>  	int			l_iclog_size;	/* size of log in bytes */
>  	int			l_iclog_bufs;	/* number of iclog buffers */
> -- 
> 2.47.2
> 
>