Re: [PATCH 2/9] xfs: add a on-disk log header cycle array accessor

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

On Wed, Oct 15, 2025 at 06:32:38AM +0200, Christoph Hellwig wrote:
> On Tue, Oct 14, 2025 at 02:27:17PM -0700, Darrick J. Wong wrote:
> > > @@ -1543,14 +1538,6 @@ xlog_pack_data(
> > >  	if (xfs_has_logv2(log->l_mp)) {
> > >  		xlog_in_core_2_t *xhdr = iclog->ic_data;
> > >  
> > > -		for ( ; i < BTOBB(size); i++) {
> > > -			j = i / XLOG_CYCLE_DATA_SIZE;
> > > -			k = i % XLOG_CYCLE_DATA_SIZE;
> > > -			xhdr[j].hic_xheader.xh_cycle_data[k] = *(__be32 *)dp;
> > > -			*(__be32 *)dp = cycle_lsn;
> > > -			dp += BBSIZE;
> > > -		}
> > 
> > Just to orient myself -- this is the code that stamps (swazzled) LSN
> > cycle numbers into the log headers, right?
> 
> Yes.
> 
> > > +static inline __be32 *xlog_cycle_data(struct xlog_rec_header *rhead, unsigned i)
> > > +{
> > > +	if (i >= XLOG_CYCLE_DATA_SIZE) {
> > 
> > Does this need a xfs_has_logv2() check?  The old callsites all seem to
> > have them.
> > 
> > What should happen if i >= XLOG_CYCLE_DATA_SIZE && !logv2 ?  Should
> > this helper return NULL so that callers can return EFSCORRUPTED or
> > something like that?
> 
> It can't happen.  It is bound by l_iclog_hsize or h_len, which
> can't go into the subsequent blocks for v1 logs.  For the recovery
> case we also check for h_len being smaller than h_size, and h_size
> is fixed to XLOG_BIG_RECORD_BSIZE for v1 logs.

Oh ok.  Yeah, that's right, so
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D