Re: [PATCH 6/9] xfs: remove xlog_in_core_2_t

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

On Wed, Oct 15, 2025 at 06:41:09AM +0200, Christoph Hellwig wrote:
> On Tue, Oct 14, 2025 at 03:07:57PM -0700, Darrick J. Wong wrote:
> > > +	__be32		xh_cycle_data[XLOG_CYCLE_DATA_SIZE];
> > > +	__u8		xh_reserved[252];
> > 
> > Just out of curiosity, why do we reserve so much space at the end of the
> > extended header?  Wouldn't it have been more efficient to fill all 508
> > bytes with xh_cycle_data?
> 
> That's something I asked myself when doing this as well.  Or if we're
> so inefficient, why don't we at least put the cycle data into the same
> offset for both the initial and extended headers.  Also why do we write
> xh_cycle into the extended header and don't every read it?

Oh well, log v3 then. :(

> > > +
> > > +	__u8	  h_reserved[184];
> > > +	struct xlog_rec_ext_header h_ext[];
> > 
> > Ok, so you're explicitly padding struct xlog_rec_header and
> > xlog_rec_ext_header to be 512 bytes now, and making the xlog_rec_header
> > have a VLA of xlog_rec_ext_header.
> 
> Yes.
> 
> > The log buffer crc is computed from the start of xlog_rec_header::h_crc
> > to the end(ish) of the xlog_rec_header; and the first 256 bytes of each
> > xlog_rec_ext_header, right?
> 
> Yes.
> 
> > > +++ b/fs/xfs/xfs_log.c
> > > @@ -1526,12 +1526,8 @@ xlog_pack_data(
> > >  		dp += BBSIZE;
> > >  	}
> > >  
> > > -	if (xfs_has_logv2(log->l_mp)) {
> > 
> > Is the xfs_has_logv2 still necessary here?
> > 
> > What happens if log->l_iclog_heads > 1 && !logv2?  Or has the kernel
> > already checked for that and aborted the mount?
> 
> l_iclog_heads is set based on m_logbsize, and xfs_finish_flags verifies
> that it is never bigger than XLOG_BIG_RECORD_BSIZE for v1 logs.

Sounds good to me!
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D