xfs: don't try to write a start record into every iclog

The xlog_write() function iterates over iclogs until it completes
writing all the log vectors passed in. The ticket tracks whether
a start record has been written or not, so only the first iclog gets
a start record. We only ever pass single use tickets to
xlog_write() so we only ever need to write a start record once per
xlog_write() call.

Hence we don't need to store whether we should write a start record
in the ticket as the callers provide all the information we need to
determine if a start record should be written. For the moment, we
have to ensure that we clear the XLOG_TIC_INITED appropriately so
the code in xfs_log_done() still works correctly for committing
transactions.

(darrick: Note the slight behavior change that we always deduct the
size of the op header from the ticket, even for unmount records)

Signed-off-by: Dave Chinner <dchinner@redhat.com>
[hch: pass an explicit need_start_rec argument]
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 4a53768c5397bf59f788705ec43bae7990b4120b..33f089aa2a25b7ded6a3ec77fa4f2dace0837a94 100644 (file)
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -921,7 +921,7 @@ xfs_log_write_unmount_record(
        /* remove inited flag, and account for space used */
        tic->t_flags = 0;
        tic->t_curr_res -= sizeof(magic);
-       error = xlog_write(log, &vec, tic, &lsn, NULL, flags);
+       error = xlog_write(log, &vec, tic, &lsn, NULL, flags, false);
        /*
         * At this point, we're umounting anyway, so there's no point in
         * transitioning log state to IOERROR. Just continue...
@@ -1541,7 +1541,7 @@ xlog_commit_record(
 
        ASSERT_ALWAYS(iclog);
        error = xlog_write(log, &vec, ticket, commitlsnp, iclog,
-                                       XLOG_COMMIT_TRANS);
+                                       XLOG_COMMIT_TRANS, false);
        if (error)
                xfs_force_shutdown(mp, SHUTDOWN_LOG_IO_ERROR);
        return error;
@@ -2118,23 +2118,21 @@ xlog_print_trans(
 }
 
 /*
- * 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 struct xlog_op_header and may need to be
+ * double word aligned.
  */
 static int
 xlog_write_calc_vec_length(
        struct xlog_ticket      *ticket,
-       struct xfs_log_vec      *log_vector)
+       struct xfs_log_vec      *log_vector,
+       bool                    need_start_rec)
 {
        struct xfs_log_vec      *lv;
-       int                     headers = 0;
+       int                     headers = need_start_rec ? 1 : 0;
        int                     len = 0;
        int                     i;
 
-       /* acct for start rec of xact */
-       if (ticket->t_flags & XLOG_TIC_INITED)
-               headers++;
-
        for (lv = log_vector; lv; lv = lv->lv_next) {
                /* we don't write ordered log vectors */
                if (lv->lv_buf_len == XFS_LOG_VEC_ORDERED)
@@ -2156,27 +2154,16 @@ xlog_write_calc_vec_length(
        return len;
 }
 
-/*
- * If first write for transaction, insert start record  We can't be trying to
- * commit if we are inited.  We can't have any "partial_copy" if we are inited.
- */
-static int
+static void
 xlog_write_start_rec(
        struct xlog_op_header   *ophdr,
        struct xlog_ticket      *ticket)
 {
-       if (!(ticket->t_flags & XLOG_TIC_INITED))
-               return 0;
-
        ophdr->oh_tid   = cpu_to_be32(ticket->t_tid);
        ophdr->oh_clientid = ticket->t_clientid;
        ophdr->oh_len = 0;
        ophdr->oh_flags = XLOG_START_TRANS;
        ophdr->oh_res2 = 0;
-
-       ticket->t_flags &= ~XLOG_TIC_INITED;
-
-       return sizeof(struct xlog_op_header);
 }
 
 static xlog_op_header_t *
@@ -2365,7 +2352,8 @@ xlog_write(
        struct xlog_ticket      *ticket,
        xfs_lsn_t               *start_lsn,
        struct xlog_in_core     **commit_iclog,
-       uint                    flags)
+       uint                    flags,
+       bool                    need_start_rec)
 {
        struct xlog_in_core     *iclog = NULL;
        struct xfs_log_iovec    *vecp;
@@ -2381,23 +2369,22 @@ xlog_write(
 
        *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.
+        * 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)
-               ticket->t_curr_res -= sizeof(xlog_op_header_t);
+       if (ticket->t_flags & XLOG_TIC_INITED) {
+               ticket->t_curr_res -= sizeof(struct xlog_op_header);
+               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.
+        * Commit record headers and unmount records need to be accounted for.
+        * These come in as separate writes so are easy to detect.
         */
-       if (flags & (XLOG_COMMIT_TRANS | XLOG_UNMOUNT_TRANS))
-               ticket->t_curr_res -= sizeof(xlog_op_header_t);
-
+       if (!need_start_rec)
+               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");
@@ -2405,6 +2392,8 @@ xlog_write(
                xfs_force_shutdown(log->l_mp, SHUTDOWN_LOG_IO_ERROR);
        }
 
+       len = xlog_write_calc_vec_length(ticket, log_vector, need_start_rec);
+
        index = 0;
        lv = log_vector;
        vecp = lv->lv_iovecp;
@@ -2431,7 +2420,6 @@ xlog_write(
                while (lv && (!lv->lv_niovecs || index < lv->lv_niovecs)) {
                        struct xfs_log_iovec    *reg;
                        struct xlog_op_header   *ophdr;
-                       int                     start_rec_copy;
                        int                     copy_len;
                        int                     copy_off;
                        bool                    ordered = false;
@@ -2447,11 +2435,15 @@ xlog_write(
                        ASSERT(reg->i_len % sizeof(int32_t) == 0);
                        ASSERT((unsigned long)ptr % sizeof(int32_t) == 0);
 
-                       start_rec_copy = xlog_write_start_rec(ptr, ticket);
-                       if (start_rec_copy) {
-                               record_cnt++;
+                       /*
+                        * Before we start formatting log vectors, we need to
+                        * write a start record. Only do this for the first
+                        * iclog we write to.
+                        */
+                       if (need_start_rec) {
+                               xlog_write_start_rec(ptr, ticket);
                                xlog_write_adv_cnt(&ptr, &len, &log_offset,
-                                                  start_rec_copy);
+                                               sizeof(struct xlog_op_header));
                        }
 
                        ophdr = xlog_write_setup_ophdr(log, ptr, ticket, flags);
@@ -2483,8 +2475,13 @@ xlog_write(
                                xlog_write_adv_cnt(&ptr, &len, &log_offset,
                                                   copy_len);
                        }
-                       copy_len += start_rec_copy + sizeof(xlog_op_header_t);
+                       copy_len += sizeof(struct xlog_op_header);
                        record_cnt++;
+                       if (need_start_rec) {
+                               copy_len += sizeof(struct xlog_op_header);
+                               record_cnt++;
+                               need_start_rec = false;
+                       }
                        data_cnt += contwr ? copy_len : 0;
 
                        error = xlog_write_copy_finish(log, iclog, flags,

diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
index 64cc0bf2ab3b9fe6e16d4f02af2f48bd5399d8e0..e0aeb316ce6c6261ba8601e1f5a1b453fe9b6590 100644 (file)
--- a/fs/xfs/xfs_log_cil.c
+++ b/fs/xfs/xfs_log_cil.c
@@ -801,7 +801,7 @@ xlog_cil_push_work(
        lvhdr.lv_iovecp = &lhdr;
        lvhdr.lv_next = ctx->lv_chain;
 
-       error = xlog_write(log, &lvhdr, tic, &ctx->start_lsn, NULL, 0);
+       error = xlog_write(log, &lvhdr, tic, &ctx->start_lsn, NULL, 0, true);
        if (error)
                goto out_abort_free_ticket;
 

diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
index 2b0aec37e73e46958dd991dcf18eda163a476e0c..b895e16460ee559b34617afa4b3acdda5e9a0d98 100644 (file)
--- a/fs/xfs/xfs_log_priv.h
+++ b/fs/xfs/xfs_log_priv.h
@@ -439,14 +439,10 @@ xlog_write_adv_cnt(void **ptr, int *len, int *off, size_t bytes)
 
 void   xlog_print_tic_res(struct xfs_mount *mp, struct xlog_ticket *ticket);
 void   xlog_print_trans(struct xfs_trans *);
-int
-xlog_write(
-       struct xlog             *log,
-       struct xfs_log_vec      *log_vector,
-       struct xlog_ticket      *tic,
-       xfs_lsn_t               *start_lsn,
-       struct xlog_in_core     **commit_iclog,
-       uint                    flags);
+int    xlog_write(struct xlog *log, struct xfs_log_vec *log_vector,
+               struct xlog_ticket *tic, xfs_lsn_t *start_lsn,
+               struct xlog_in_core **commit_iclog, uint flags,
+               bool need_start_rec);
 
 /*
  * When we crack an atomic LSN, we sample it first so that the value will not