On Mon, Mar 08, 2021 at 06:36:44PM -0800, Darrick J. Wong wrote:
> On Fri, Mar 05, 2021 at 04:11:25PM +1100, Dave Chinner wrote:
> > @@ -2306,7 +2282,6 @@ xlog_write(
> > xfs_force_shutdown(log->l_mp, SHUTDOWN_LOG_IO_ERROR);
> > }
> >
> > - len = xlog_write_calc_vec_length(ticket, log_vector, optype);
> > if (start_lsn)
> > *start_lsn = 0;
> > while (lv && (!lv->lv_niovecs || index < lv->lv_niovecs)) {
> > diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
> > index 7a5e6bdb7876..34abc3bae587 100644
> > --- a/fs/xfs/xfs_log_cil.c
> > +++ b/fs/xfs/xfs_log_cil.c
> > @@ -710,11 +710,12 @@ xlog_cil_build_trans_hdr(
> > sizeof(struct xfs_trans_header);
> > hdr->lhdr[1].i_type = XLOG_REG_TYPE_TRANSHDR;
> >
> > - tic->t_curr_res -= hdr->lhdr[0].i_len + hdr->lhdr[1].i_len;
> > -
> > lvhdr->lv_niovecs = 2;
> > lvhdr->lv_iovecp = &hdr->lhdr[0];
> > + lvhdr->lv_bytes = hdr->lhdr[0].i_len + hdr->lhdr[1].i_len;
>
> Er... does this code change belong in an earlier patch? Or if not, why
> wasn't it important to set lv_bytes here?
It belongs in this patch.
It was not necessary to set lv_bytes before this because
xlog_write_calc_vec_length() walked the entire change calculating
the length of the chain by adding up the region lengths itself.
Because this isn't a dynamically allocated log vector associated
with a log item, we never actually use the lv_bytes field in it for
anything.
In the case of this patch, we need to add the size of the data in
the log vector to our accumulated total that xlog_cil_push_work()
now passes in to xlog_write() to replace the chain walk that
xlog_write_calc_vec_length() did to calculate the length. Hence we
have to pass the accumulated region length back to the caller, and
rather than add another parameter we fill out lv_bytes so that it
matches all the other log vectors in the chain....
> > @@ -893,6 +898,9 @@ xlog_cil_push_work(
> > * transaction header here as it is not accounted for in xlog_write().
> > */
> > xlog_cil_build_trans_hdr(ctx, &thdr, &lvhdr, num_iovecs);
> > + num_iovecs += lvhdr.lv_niovecs;
> > + num_bytes += lvhdr.lv_bytes;
> > +
> >
>
> No need to have two blank lines here.
Fixed.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
