| Message ID | 20211118231352.2051947-5-david@fromorbit.com (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
| Series | xfs: rework xlog_write() | expand |
On 19 Nov 2021 at 04:43, Dave Chinner wrote: > Remove the final case where xlog_write() has to prepend an opheader > to a log transaction. Similar to the start record, the commit record > is just an empty opheader with a XLOG_COMMIT_TRANS type, so we can > just make this the payload for the region being passed to > xlog_write() and remove the special handling in xlog_write() for > the commit record. > I have verfified the following, 1. xfs_log_vec and xfs_log_iovec contents have been initialized correctly. 2. Checkpoint transaction's log reservation is accounted for correctly. 3. Commit record's embedded xlog_op_header's oh_len is correctly set to 0 by xlog_write() since there is no corresponding payload. Reviewed-by: Chandan Babu R <chandan.babu@oracle.com> > Signed-off-by: Dave Chinner <dchinner@redhat.com> > Reviewed-by: Darrick J. Wong <djwong@kernel.org> > Reviewed-by: Christoph Hellwig <hch@lst.de> > --- > fs/xfs/xfs_log.c | 22 ++++++---------------- > fs/xfs/xfs_log_cil.c | 11 +++++++++-- > 2 files changed, 15 insertions(+), 18 deletions(-) > > diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c > index e5515d0c85c4..f789acd2f755 100644 > --- a/fs/xfs/xfs_log.c > +++ b/fs/xfs/xfs_log.c > @@ -2281,11 +2281,10 @@ xlog_write_calc_vec_length( > > /* Don't account for regions with embedded ophdrs */ > if (optype && headers > 0) { > - if (optype & XLOG_UNMOUNT_TRANS) > - headers--; > + headers--; > if (optype & XLOG_START_TRANS) { > - ASSERT(headers >= 2); > - headers -= 2; > + ASSERT(headers >= 1); > + headers--; > } > } > > @@ -2486,14 +2485,6 @@ xlog_write( > int data_cnt = 0; > int error = 0; > > - /* > - * If this is a commit or unmount transaction, we don't need a start > - * record to be written. We do, however, have to account for the commit > - * header that gets written. Hence we always have to account for an > - * extra xlog_op_header here for commit records. > - */ > - if (optype & XLOG_COMMIT_TRANS) > - ticket->t_curr_res -= sizeof(struct xlog_op_header); > if (ticket->t_curr_res < 0) { > xfs_alert_tag(log->l_mp, XFS_PTAG_LOGRES, > "ctx ticket reservation ran out. Need to up reservation"); > @@ -2550,14 +2541,13 @@ xlog_write( > /* > * The XLOG_START_TRANS has embedded ophdrs for the > * start record and transaction header. They will always > - * be the first two regions in the lv chain. > + * be the first two regions in the lv chain. Commit and > + * unmount records also have embedded ophdrs. > */ > - if (optype & XLOG_START_TRANS) { > + if (optype) { > ophdr = reg->i_addr; > if (index) > optype &= ~XLOG_START_TRANS; > - } else if (optype & XLOG_UNMOUNT_TRANS) { > - ophdr = reg->i_addr; > } else { > ophdr = xlog_write_setup_ophdr(log, ptr, > ticket, optype); > diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c > index 9a810a2c92e9..2c4fb55d4897 100644 > --- a/fs/xfs/xfs_log_cil.c > +++ b/fs/xfs/xfs_log_cil.c > @@ -810,9 +810,14 @@ xlog_cil_write_commit_record( > struct xfs_cil_ctx *ctx) > { > struct xlog *log = ctx->cil->xc_log; > + struct xlog_op_header ophdr = { > + .oh_clientid = XFS_TRANSACTION, > + .oh_tid = cpu_to_be32(ctx->ticket->t_tid), > + .oh_flags = XLOG_COMMIT_TRANS, > + }; > struct xfs_log_iovec reg = { > - .i_addr = NULL, > - .i_len = 0, > + .i_addr = &ophdr, > + .i_len = sizeof(struct xlog_op_header), > .i_type = XLOG_REG_TYPE_COMMIT, > }; > struct xfs_log_vec vec = { > @@ -828,6 +833,8 @@ xlog_cil_write_commit_record( > if (error) > return error; > > + /* account for space used by record data */ > + ctx->ticket->t_curr_res -= reg.i_len; > error = xlog_write(log, ctx, &vec, ctx->ticket, XLOG_COMMIT_TRANS); > if (error) > xfs_force_shutdown(log->l_mp, SHUTDOWN_LOG_IO_ERROR);
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c index e5515d0c85c4..f789acd2f755 100644 --- a/fs/xfs/xfs_log.c +++ b/fs/xfs/xfs_log.c @@ -2281,11 +2281,10 @@ xlog_write_calc_vec_length( /* Don't account for regions with embedded ophdrs */ if (optype && headers > 0) { - if (optype & XLOG_UNMOUNT_TRANS) - headers--; + headers--; if (optype & XLOG_START_TRANS) { - ASSERT(headers >= 2); - headers -= 2; + ASSERT(headers >= 1); + headers--; } } @@ -2486,14 +2485,6 @@ xlog_write( int data_cnt = 0; int error = 0; - /* - * If this is a commit or unmount transaction, we don't need a start - * record to be written. We do, however, have to account for the commit - * header that gets written. Hence we always have to account for an - * extra xlog_op_header here for commit records. - */ - if (optype & XLOG_COMMIT_TRANS) - ticket->t_curr_res -= sizeof(struct xlog_op_header); if (ticket->t_curr_res < 0) { xfs_alert_tag(log->l_mp, XFS_PTAG_LOGRES, "ctx ticket reservation ran out. Need to up reservation"); @@ -2550,14 +2541,13 @@ xlog_write( /* * The XLOG_START_TRANS has embedded ophdrs for the * start record and transaction header. They will always - * be the first two regions in the lv chain. + * be the first two regions in the lv chain. Commit and + * unmount records also have embedded ophdrs. */ - if (optype & XLOG_START_TRANS) { + if (optype) { ophdr = reg->i_addr; if (index) optype &= ~XLOG_START_TRANS; - } else if (optype & XLOG_UNMOUNT_TRANS) { - ophdr = reg->i_addr; } else { ophdr = xlog_write_setup_ophdr(log, ptr, ticket, optype); diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c index 9a810a2c92e9..2c4fb55d4897 100644 --- a/fs/xfs/xfs_log_cil.c +++ b/fs/xfs/xfs_log_cil.c @@ -810,9 +810,14 @@ xlog_cil_write_commit_record( struct xfs_cil_ctx *ctx) { struct xlog *log = ctx->cil->xc_log; + struct xlog_op_header ophdr = { + .oh_clientid = XFS_TRANSACTION, + .oh_tid = cpu_to_be32(ctx->ticket->t_tid), + .oh_flags = XLOG_COMMIT_TRANS, + }; struct xfs_log_iovec reg = { - .i_addr = NULL, - .i_len = 0, + .i_addr = &ophdr, + .i_len = sizeof(struct xlog_op_header), .i_type = XLOG_REG_TYPE_COMMIT, }; struct xfs_log_vec vec = { @@ -828,6 +833,8 @@ xlog_cil_write_commit_record( if (error) return error; + /* account for space used by record data */ + ctx->ticket->t_curr_res -= reg.i_len; error = xlog_write(log, ctx, &vec, ctx->ticket, XLOG_COMMIT_TRANS); if (error) xfs_force_shutdown(log->l_mp, SHUTDOWN_LOG_IO_ERROR);