Re: [PATCH 01/11] xfs: don't try to write a start record into every iclog

Christoph Hellwig
2020-03-04T15:44:23Z

> /*
> - * Calculate the potential space needed by the log vector. Each region gets
> - * its own xlog_op_header_t and may need to be double word aligned.
> + * Calculate the potential space needed by the log vector. We may need a
> + * start record, and each region gets its own xlog_op_header_t and may need to
> + * be double word aligned.

s/xlog_op_header_t/struct xlog_op_header/ while you're at it.

> @@ -2404,25 +2391,29 @@ xlog_write(
> int record_cnt = 0;
> int data_cnt = 0;
> int error = 0;
> + int start_rec_size = sizeof(struct xlog_op_header);
>
> *start_lsn = 0;
>
> - len = xlog_write_calc_vec_length(ticket, log_vector);
>
> /*
> * Region headers and bytes are already accounted for.
> * We only need to take into account start records and
> * split regions in this function.
> */
> - if (ticket->t_flags & XLOG_TIC_INITED)
> + if (ticket->t_flags & XLOG_TIC_INITED) {
> ticket->t_curr_res -= sizeof(xlog_op_header_t);
> + ticket->t_flags &= ~XLOG_TIC_INITED;
> + }
>
> /*
> * Commit record headers need to be accounted for. These
> * come in as separate writes so are easy to detect.
> */
> - if (flags & (XLOG_COMMIT_TRANS | XLOG_UNMOUNT_TRANS))
> + if (flags & (XLOG_COMMIT_TRANS | XLOG_UNMOUNT_TRANS)) {
> ticket->t_curr_res -= sizeof(xlog_op_header_t);
> + start_rec_size = 0;
> + }
>
> if (ticket->t_curr_res < 0) {
> xfs_alert_tag(log->l_mp, XFS_PTAG_LOGRES,
> @@ -2431,6 +2422,8 @@ xlog_write(
> xfs_force_shutdown(log->l_mp, SHUTDOWN_LOG_IO_ERROR);
> }
>
> + len = xlog_write_calc_vec_length(ticket, log_vector, start_rec_size);

The last arg is used as a boolean in xlog_write_calc_vec_length. I
think it would make sense to have a need_start_rec boolean in this
function as well, and just hardcode the sizeof in the two places that
actually need the size.

> + copy_len += sizeof(xlog_op_header_t);

s/xlog_op_header_t/struct xlog_op_header/