Re: [PATCH 1/8] xfs: hide log iovec alignment constraints

[Dave Chinner <david@fromorbit.com>]

On Sun, Apr 10, 2022 at 10:23:09PM -0700, Alli wrote:
> On Tue, 2022-03-15 at 09:06 +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > Callers currently have to round out the size of buffers to match the
> > aligment constraints of log iovecs and xlog_write(). They should not
> > need to know this detail, so introduce a new function to calculate
> > the iovec length (for use in ->iop_size implementations). Also
> > modify xlog_finish_iovec() to round up the length to the correct
> > alignment so the callers don't need to do this, either.
> > 
> > Convert the only user - inode forks - of this alignment rounding to
> > use the new interface.
> > 
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
.....
> >  void *xlog_prepare_iovec(struct xfs_log_vec *lv, struct
> > xfs_log_iovec **vecp,
> >  		uint type);
> >  
> > @@ -29,6 +40,12 @@ xlog_finish_iovec(struct xfs_log_vec *lv, struct
> > xfs_log_iovec *vec, int len)
> >  {
> >  	struct xlog_op_header	*oph = vec->i_addr;
> >  
> > +	/*
> > +	 * Always round up the length to the correct alignment so
> > callers don't
> > +	 * need to know anything about this log vec layout requirement.
> > +	 */
> > +	len = xlog_calc_iovec_len(len);Hmm, what code base was this on?
> > > 
> Hmm, I'm getting some merge conflicts in this area.  It looks like the
> round_up logic was already added in:
> 
> bde7cff67c39227c6ad503394e19e58debdbc5e3
> "xfs: format log items write directly into the linear CIL buffer"
> 
> So I think it's ok to drop this bit about rounding length.

Ok, I think that's why you are getting rounding assert failures in
the log code - this code replaces the fixed 4 byte allocation
roundup that is done for the inode fork data buffers, and if you
remove both the round-up I added to xlog_finish_iovec() and the
inode fork roundup, you get unaligned regions and assert failures in
xlog_write()....

The posted patchset was based on top of the xlog-write-rewrite
patchset I posted before this one, so I'd say that's where the
conflicts applying this to a base 5.18-rc2 kernel came from.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com