On Wed, Feb 24, 2021 at 05:34:50PM +1100, Dave Chinner wrote:
> Subject: xfs: embed the xlog_op_header in the unmount record
Uh... isn't this embedding the xlog op header in the *commit* record?
(Just saying for my own lazy purposes because my scripts choke badly
when a patchset has multiple patches with the same subject...)
--D
> 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.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
> fs/xfs/xfs_log.c | 33 +++++++++++++++------------------
> 1 file changed, 15 insertions(+), 18 deletions(-)
>
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index f3cb7482dfea..78b9c11b585f 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -1596,9 +1596,14 @@ xlog_commit_record(
> struct xlog_in_core **iclog,
> xfs_lsn_t *lsn)
> {
> + struct xlog_op_header ophdr = {
> + .oh_clientid = XFS_TRANSACTION,
> + .oh_tid = cpu_to_be32(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 = {
> @@ -1610,6 +1615,8 @@ xlog_commit_record(
> if (XLOG_FORCED_SHUTDOWN(log))
> return -EIO;
>
> + /* account for space used by record data */
> + ticket->t_curr_res -= reg.i_len;
> error = xlog_write(log, &vec, ticket, lsn, iclog, XLOG_COMMIT_TRANS);
> if (error)
> xfs_force_shutdown(log->l_mp, SHUTDOWN_LOG_IO_ERROR);
> @@ -2233,11 +2240,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--;
> }
> }
>
> @@ -2447,14 +2453,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");
> @@ -2518,14 +2516,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);
> --
> 2.28.0
>
