Re: [PATCH 02/10] xfs: set lv_bytes in xlog_write_one_vec

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

On Mon, Nov 03, 2025 at 11:43:19AM +0100, Christoph Hellwig wrote:
> On Fri, Oct 31, 2025 at 05:04:09PM -0700, Darrick J. Wong wrote:
> > > diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> > > index ed83a0e3578e..382c55f4d8d2 100644
> > > --- a/fs/xfs/xfs_log.c
> > > +++ b/fs/xfs/xfs_log.c
> > > @@ -858,14 +858,15 @@ xlog_write_one_vec(
> > >  	struct xfs_log_vec	lv = {
> > >  		.lv_niovecs	= 1,
> > >  		.lv_iovecp	= reg,
> > > +		.lv_bytes	= reg->i_len,
> > 
> > I'm surprised that nothing's noticed the zero lv_bytes, but I guess
> > unmount and commit record writes have always wanted a full write anyway?
> > 
> > Question: if lv_bytes is no longer zero, can this fall into the
> > xlog_write_partial branch?
> 
> The unmount format is smaller than an opheader, so we won't split it
> because of that, but unless I'm misreading things it could fix a bug
> where we'd not get a new iclog when needed for it otherwise?

Yes, I think you're right about that.  Currently we always call
xlog_write_full on the unmount record, and there's no check that
write_len <= *bytes_left so I guess we just memcpy off the end of the
buffer IF it happens to be the case that the current iclog has exactly
enough space for the unmount ophdr but not the xfs_unmount_log_format.
In that case you'd get the ophdr at the end of the log, but not the
"Un".

Maybe this needs to get turned into a proper bug fix?  Though you'd
think we'd have heard about this by now if it were a real occurrence.
OTOH some people might just xfs_repair -L and get away with it.

> The commit record is just an an opheader without any payload, so there
> is no way to even split it in theory.

Agreed.

--D