On Wed, Nov 10, 2021 at 11:55:58PM -0800, Christoph Hellwig wrote:
> > struct xlog_cil_trans_hdr {
> > + struct xlog_op_header oph[2];
> > struct xfs_trans_header thdr;
> > + struct xfs_log_iovec lhdr[2];
>
> I find the logic where xlog_cil_build_trans_hdr stuffs oph[1] and
> thdr into a single log_iovec rather confusing even if it is correct.
> But I'd also rather get this series in and see if it can be cleaned
> up later, so I'll just leave that as a note here.
>
> > struct xlog_ticket *tic = ctx->ticket;
> > + uint32_t tid = cpu_to_be32(tic->t_tid);
>
> This needs to be a __be32.
*nod*.
> > - hdr->thdr.th_tid = tic->t_tid;
> > + hdr->thdr.th_tid = tid;
>
> And this needs to keep using the not byteswapped version.
> (but it appears we never look at the trans header in recovery anyway
> currently).
Right, it's never, ever been used, except in xfs_logprint. Prior to
delayed logging, it was defined as:
__int32_t th_tid; /* transaction id (unused) */
So it was always zero. It is still unused by anything except
logprint, in which case I noticed that it was byteswapped compared
to the op header TID, which made searches for ophdrs that matched
the transaction header TID difficult.
I've changed it back to what it was and added a note to the code to
indicate that the transaction header is in host byte order, not big
endian.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
