* [PATCH 1/4] xfs: factor xlog_write
2010-03-15 15:51 [PATCH 0/4] xfs_log_write cleanups and vector support Christoph Hellwig
@ 2010-03-15 15:51 ` Christoph Hellwig
2010-03-15 15:51 ` [PATCH 2/4] xfs: reindent xlog_write Christoph Hellwig
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2010-03-15 15:51 UTC (permalink / raw)
To: david; +Cc: Dave Chinner, xfs
[-- Attachment #1: xlog_write-refactor --]
[-- Type: text/plain, Size: 12510 bytes --]
From: Dave Chinner <dchinner@redhat.com>
xlog_write is a mess that takes a lot of effort to understand. It is
a mass of nested loops with 4 space indents to get it to fit in 80 columns
and lots of funky variables that aren't obvious what they mean or do.
Break it down into understandable chunks.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Index: xfs/fs/xfs/xfs_log.c
===================================================================
--- xfs.orig/fs/xfs/xfs_log.c 2010-03-15 12:54:37.860004031 +0100
+++ xfs/fs/xfs/xfs_log.c 2010-03-15 12:56:54.180255392 +0100
@@ -1616,6 +1616,193 @@ xlog_print_tic_res(xfs_mount_t *mp, xlog
}
/*
+ * 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.
+ */
+static int
+xlog_write_calc_vec_length(
+ struct xlog_ticket *ticket,
+ struct xfs_log_iovec reg[],
+ int nentries)
+{
+ int headers = 0;
+ int len = 0;
+ int i;
+
+ /* acct for start rec of xact */
+ if (ticket->t_flags & XLOG_TIC_INITED)
+ headers++;
+
+ for (i = 0; i < nentries; i++) {
+ /* each region gets >= 1 */
+ headers++;
+
+ len += reg[i].i_len;
+ xlog_tic_add_region(ticket, reg[i].i_len, reg[i].i_type);
+ }
+
+ ticket->t_res_num_ophdrs += headers;
+ len += headers * sizeof(struct xlog_op_header);
+
+ 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
+xlog_write_start_rec(
+ __psint_t ptr,
+ struct xlog_ticket *ticket)
+{
+ struct xlog_op_header *ophdr = (struct xlog_op_header *)ptr;
+
+ 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 *
+xlog_write_setup_ophdr(
+ struct log *log,
+ __psint_t ptr,
+ struct xlog_ticket *ticket,
+ uint flags)
+{
+ struct xlog_op_header *ophdr = (struct xlog_op_header *)ptr;
+
+ ophdr->oh_tid = cpu_to_be32(ticket->t_tid);
+ ophdr->oh_clientid = ticket->t_clientid;
+ ophdr->oh_res2 = 0;
+
+ /* are we copying a commit or unmount record? */
+ ophdr->oh_flags = flags;
+
+ /*
+ * We've seen logs corrupted with bad transaction client ids. This
+ * makes sure that XFS doesn't generate them on. Turn this into an EIO
+ * and shut down the filesystem.
+ */
+ switch (ophdr->oh_clientid) {
+ case XFS_TRANSACTION:
+ case XFS_VOLUME:
+ case XFS_LOG:
+ break;
+ default:
+ xfs_fs_cmn_err(CE_WARN, log->l_mp,
+ "Bad XFS transaction clientid 0x%x in ticket 0x%p",
+ ophdr->oh_clientid, ticket);
+ return NULL;
+ }
+
+ return ophdr;
+}
+
+/*
+ * Set up the parameters of the region copy into the log. This has
+ * to handle region write split across multiple log buffers - this
+ * state is kept external to this function so that this code can
+ * can be written in an obvious, self documenting manner.
+ */
+static int
+xlog_write_setup_copy(
+ struct xlog_ticket *ticket,
+ struct xlog_op_header *ophdr,
+ int space_available,
+ int space_required,
+ int *copy_off,
+ int *copy_len,
+ int *last_was_partial_copy,
+ int *bytes_consumed)
+{
+ int still_to_copy;
+
+ still_to_copy = space_required - *bytes_consumed;
+ *copy_off = *bytes_consumed;
+
+ if (still_to_copy <= space_available) {
+ /* write of region completes here */
+ *copy_len = still_to_copy;
+ ophdr->oh_len = cpu_to_be32(*copy_len);
+ if (*last_was_partial_copy)
+ ophdr->oh_flags|= (XLOG_END_TRANS|XLOG_WAS_CONT_TRANS);
+ *last_was_partial_copy = 0;
+ *bytes_consumed = 0;
+ return 0;
+ }
+
+ /* partial write of region, needs extra log op header reservation */
+ *copy_len = space_available;
+ ophdr->oh_len = cpu_to_be32(*copy_len);
+ ophdr->oh_flags |= XLOG_CONTINUE_TRANS;
+ if (*last_was_partial_copy)
+ ophdr->oh_flags |= XLOG_WAS_CONT_TRANS;
+ *bytes_consumed += *copy_len;
+ (*last_was_partial_copy)++;
+
+ /* account for new log op header */
+ ticket->t_curr_res -= sizeof(struct xlog_op_header);
+ ticket->t_res_num_ophdrs++;
+
+ return sizeof(struct xlog_op_header);
+}
+
+static int
+xlog_write_copy_finish(
+ struct log *log,
+ struct xlog_in_core *iclog,
+ uint flags,
+ int *record_cnt,
+ int *data_cnt,
+ int *partial_copy,
+ int *partial_copy_len,
+ int log_offset,
+ struct xlog_in_core **commit_iclog)
+{
+ if (*partial_copy) {
+ /*
+ * This iclog has already been marked WANT_SYNC by
+ * xlog_state_get_iclog_space.
+ */
+ xlog_state_finish_copy(log, iclog, *record_cnt, *data_cnt);
+ *record_cnt = 0;
+ *data_cnt = 0;
+ return xlog_state_release_iclog(log, iclog);
+ }
+
+ *partial_copy = 0;
+ *partial_copy_len = 0;
+
+ if (iclog->ic_size - log_offset <= sizeof(xlog_op_header_t)) {
+ /* no more space in this iclog - push it. */
+ xlog_state_finish_copy(log, iclog, *record_cnt, *data_cnt);
+ *record_cnt = 0;
+ *data_cnt = 0;
+
+ spin_lock(&log->l_icloglock);
+ xlog_state_want_sync(log, iclog);
+ spin_unlock(&log->l_icloglock);
+
+ if (!commit_iclog)
+ return xlog_state_release_iclog(log, iclog);
+ ASSERT(flags & XLOG_COMMIT_TRANS);
+ *commit_iclog = iclog;
+ }
+
+ return 0;
+}
+
+/*
* Write some region out to in-core log
*
* This will be called when writing externally provided regions or when
@@ -1675,7 +1862,6 @@ xlog_write(
int start_rec_copy; /* # bytes to copy for start record */
int partial_copy; /* did we split a region? */
int partial_copy_len;/* # bytes copied if split region */
- int need_copy; /* # bytes need to memcpy this region */
int copy_len; /* # bytes actually memcpy'ing */
int copy_off; /* # bytes from entry start */
int contwr; /* continued write of in-core log? */
@@ -1683,24 +1869,9 @@ xlog_write(
int record_cnt = 0, data_cnt = 0;
partial_copy_len = partial_copy = 0;
-
- /* Calculate potential maximum space. Each region gets its own
- * xlog_op_header_t and may need to be double word aligned.
- */
- len = 0;
- if (ticket->t_flags & XLOG_TIC_INITED) { /* acct for start rec of xact */
- len += sizeof(xlog_op_header_t);
- ticket->t_res_num_ophdrs++;
- }
-
- for (index = 0; index < nentries; index++) {
- len += sizeof(xlog_op_header_t); /* each region gets >= 1 */
- ticket->t_res_num_ophdrs++;
- len += reg[index].i_len;
- xlog_tic_add_region(ticket, reg[index].i_len, reg[index].i_type);
- }
contwr = *start_lsn = 0;
+ len = xlog_write_calc_vec_length(ticket, reg, nentries);
if (ticket->t_curr_res < len) {
xlog_print_tic_res(mp, ticket);
#ifdef DEBUG
@@ -1734,81 +1905,23 @@ xlog_write(
while (index < nentries) {
ASSERT(reg[index].i_len % sizeof(__int32_t) == 0);
ASSERT((__psint_t)ptr % sizeof(__int32_t) == 0);
- start_rec_copy = 0;
- /* 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.
- */
- if (ticket->t_flags & XLOG_TIC_INITED) {
- logop_head = (xlog_op_header_t *)ptr;
- logop_head->oh_tid = cpu_to_be32(ticket->t_tid);
- logop_head->oh_clientid = ticket->t_clientid;
- logop_head->oh_len = 0;
- logop_head->oh_flags = XLOG_START_TRANS;
- logop_head->oh_res2 = 0;
- ticket->t_flags &= ~XLOG_TIC_INITED; /* clear bit */
+ start_rec_copy = xlog_write_start_rec(ptr, ticket);
+ if (start_rec_copy) {
record_cnt++;
-
- start_rec_copy = sizeof(xlog_op_header_t);
xlog_write_adv_cnt(ptr, len, log_offset, start_rec_copy);
}
- /* Copy log operation header directly into data section */
- logop_head = (xlog_op_header_t *)ptr;
- logop_head->oh_tid = cpu_to_be32(ticket->t_tid);
- logop_head->oh_clientid = ticket->t_clientid;
- logop_head->oh_res2 = 0;
-
- /* header copied directly */
+ logop_head = xlog_write_setup_ophdr(log, ptr, ticket, flags);
+ if (!logop_head)
+ return XFS_ERROR(EIO);
xlog_write_adv_cnt(ptr, len, log_offset, sizeof(xlog_op_header_t));
- /* are we copying a commit or unmount record? */
- logop_head->oh_flags = flags;
-
- /*
- * We've seen logs corrupted with bad transaction client
- * ids. This makes sure that XFS doesn't generate them on.
- * Turn this into an EIO and shut down the filesystem.
- */
- switch (logop_head->oh_clientid) {
- case XFS_TRANSACTION:
- case XFS_VOLUME:
- case XFS_LOG:
- break;
- default:
- xfs_fs_cmn_err(CE_WARN, mp,
- "Bad XFS transaction clientid 0x%x in ticket 0x%p",
- logop_head->oh_clientid, ticket);
- return XFS_ERROR(EIO);
- }
-
- /* Partial write last time? => (partial_copy != 0)
- * need_copy is the amount we'd like to copy if everything could
- * fit in the current memcpy.
- */
- need_copy = reg[index].i_len - partial_copy_len;
-
- copy_off = partial_copy_len;
- if (need_copy <= iclog->ic_size - log_offset) { /*complete write */
- copy_len = need_copy;
- logop_head->oh_len = cpu_to_be32(copy_len);
- if (partial_copy)
- logop_head->oh_flags|= (XLOG_END_TRANS|XLOG_WAS_CONT_TRANS);
- partial_copy_len = partial_copy = 0;
- } else { /* partial write */
- copy_len = iclog->ic_size - log_offset;
- logop_head->oh_len = cpu_to_be32(copy_len);
- logop_head->oh_flags |= XLOG_CONTINUE_TRANS;
- if (partial_copy)
- logop_head->oh_flags |= XLOG_WAS_CONT_TRANS;
- partial_copy_len += copy_len;
- partial_copy++;
- len += sizeof(xlog_op_header_t); /* from splitting of region */
- /* account for new log op header */
- ticket->t_curr_res -= sizeof(xlog_op_header_t);
- ticket->t_res_num_ophdrs++;
- }
+ len += xlog_write_setup_copy(ticket, logop_head,
+ iclog->ic_size - log_offset,
+ reg[index].i_len, ©_off,
+ ©_len, &partial_copy,
+ &partial_copy_len);
xlog_verify_dest_ptr(log, ptr);
/* copy region */
@@ -1820,34 +1933,34 @@ xlog_write(
copy_len += start_rec_copy + sizeof(xlog_op_header_t);
record_cnt++;
data_cnt += contwr ? copy_len : 0;
- if (partial_copy) { /* copied partial region */
- /* already marked WANT_SYNC by xlog_state_get_iclog_space */
- xlog_state_finish_copy(log, iclog, record_cnt, data_cnt);
- record_cnt = data_cnt = 0;
- if ((error = xlog_state_release_iclog(log, iclog)))
- return error;
- break; /* don't increment index */
- } else { /* copied entire region */
- index++;
- partial_copy_len = partial_copy = 0;
-
- if (iclog->ic_size - log_offset <= sizeof(xlog_op_header_t)) {
- xlog_state_finish_copy(log, iclog, record_cnt, data_cnt);
- record_cnt = data_cnt = 0;
- spin_lock(&log->l_icloglock);
- xlog_state_want_sync(log, iclog);
- spin_unlock(&log->l_icloglock);
- if (commit_iclog) {
- ASSERT(flags & XLOG_COMMIT_TRANS);
- *commit_iclog = iclog;
- } else if ((error = xlog_state_release_iclog(log, iclog)))
- return error;
- if (index == nentries)
- return 0; /* we are done */
- else
- break;
- }
- } /* if (partial_copy) */
+
+ error = xlog_write_copy_finish(log, iclog, flags,
+ &record_cnt, &data_cnt,
+ &partial_copy, &partial_copy_len,
+ log_offset, commit_iclog);
+ if (error)
+ return error;
+
+ /*
+ * if we had a partial copy, we need to get more iclog
+ * space but we don't want to increment the region
+ * index because there is still more is this region to write.
+ *
+ * If we completed writing this region, and we flushed
+ * the iclog (indicated by resetting of the record
+ * count), then we also need to get more log space. If
+ * this was the last record, though, we are done and
+ * can just return.
+ */
+ if (partial_copy)
+ break;
+
+ index++;
+ if (record_cnt == 0) {
+ if (index == nentries)
+ return 0;
+ break;
+ }
} /* while (index < nentries) */
} /* for (index = 0; index < nentries; ) */
ASSERT(len == 0);
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 2/4] xfs: reindent xlog_write
2010-03-15 15:51 [PATCH 0/4] xfs_log_write cleanups and vector support Christoph Hellwig
2010-03-15 15:51 ` [PATCH 1/4] xfs: factor xlog_write Christoph Hellwig
@ 2010-03-15 15:51 ` Christoph Hellwig
2010-03-15 15:51 ` [PATCH 3/4] xfs: introduce new internal log vector structure Christoph Hellwig
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2010-03-15 15:51 UTC (permalink / raw)
To: david; +Cc: xfs
[-- Attachment #1: xlog_write-reindent --]
[-- Type: text/plain, Size: 8624 bytes --]
Reindent xlog_write to normal one tab indents and move all variable
declarations into the closest enclosing block.
Split from a bigger patch by Dave Chinner.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: xfs/fs/xfs/xfs_log.c
===================================================================
--- xfs.orig/fs/xfs/xfs_log.c 2010-03-15 13:36:00.380253575 +0100
+++ xfs/fs/xfs/xfs_log.c 2010-03-15 13:41:12.056255113 +0100
@@ -1852,127 +1852,141 @@ xlog_write(
struct xlog_in_core **commit_iclog,
uint flags)
{
- xlog_t *log = mp->m_log;
- xlog_in_core_t *iclog = NULL; /* ptr to current in-core log */
- xlog_op_header_t *logop_head; /* ptr to log operation header */
- __psint_t ptr; /* copy address into data region */
- int len; /* # xlog_write() bytes 2 still copy */
- int index; /* region index currently copying */
- int log_offset; /* offset (from 0) into data region */
- int start_rec_copy; /* # bytes to copy for start record */
- int partial_copy; /* did we split a region? */
- int partial_copy_len;/* # bytes copied if split region */
- int copy_len; /* # bytes actually memcpy'ing */
- int copy_off; /* # bytes from entry start */
- int contwr; /* continued write of in-core log? */
- int error;
- int record_cnt = 0, data_cnt = 0;
-
- partial_copy_len = partial_copy = 0;
- contwr = *start_lsn = 0;
-
- len = xlog_write_calc_vec_length(ticket, reg, nentries);
- if (ticket->t_curr_res < len) {
- xlog_print_tic_res(mp, ticket);
+ struct log *log = mp->m_log;
+ struct xlog_in_core *iclog = NULL;
+ int len;
+ int index;
+ int partial_copy = 0;
+ int partial_copy_len = 0;
+ int contwr = 0;
+ int record_cnt = 0;
+ int data_cnt = 0;
+ int error;
+
+ *start_lsn = 0;
+
+ len = xlog_write_calc_vec_length(ticket, reg, nentries);
+ if (ticket->t_curr_res < len) {
+ xlog_print_tic_res(mp, ticket);
#ifdef DEBUG
- xlog_panic(
- "xfs_log_write: reservation ran out. Need to up reservation");
+ xlog_panic(
+ "xfs_log_write: reservation ran out. Need to up reservation");
#else
- /* Customer configurable panic */
- xfs_cmn_err(XFS_PTAG_LOGRES, CE_ALERT, mp,
- "xfs_log_write: reservation ran out. Need to up reservation");
- /* If we did not panic, shutdown the filesystem */
- xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
+ /* Customer configurable panic */
+ xfs_cmn_err(XFS_PTAG_LOGRES, CE_ALERT, mp,
+ "xfs_log_write: reservation ran out. Need to up reservation");
+
+ /* If we did not panic, shutdown the filesystem */
+ xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
#endif
- } else
+ }
+
ticket->t_curr_res -= len;
- for (index = 0; index < nentries; ) {
- if ((error = xlog_state_get_iclog_space(log, len, &iclog, ticket,
- &contwr, &log_offset)))
- return error;
-
- ASSERT(log_offset <= iclog->ic_size - 1);
- ptr = (__psint_t) ((char *)iclog->ic_datap+log_offset);
-
- /* start_lsn is the first lsn written to. That's all we need. */
- if (! *start_lsn)
- *start_lsn = be64_to_cpu(iclog->ic_header.h_lsn);
-
- /* This loop writes out as many regions as can fit in the amount
- * of space which was allocated by xlog_state_get_iclog_space().
- */
- while (index < nentries) {
- ASSERT(reg[index].i_len % sizeof(__int32_t) == 0);
- ASSERT((__psint_t)ptr % sizeof(__int32_t) == 0);
-
- start_rec_copy = xlog_write_start_rec(ptr, ticket);
- if (start_rec_copy) {
- record_cnt++;
- xlog_write_adv_cnt(ptr, len, log_offset, start_rec_copy);
- }
-
- logop_head = xlog_write_setup_ophdr(log, ptr, ticket, flags);
- if (!logop_head)
- return XFS_ERROR(EIO);
- xlog_write_adv_cnt(ptr, len, log_offset, sizeof(xlog_op_header_t));
-
- len += xlog_write_setup_copy(ticket, logop_head,
- iclog->ic_size - log_offset,
- reg[index].i_len, ©_off,
- ©_len, &partial_copy,
- &partial_copy_len);
- xlog_verify_dest_ptr(log, ptr);
-
- /* copy region */
- ASSERT(copy_len >= 0);
- memcpy((xfs_caddr_t)ptr, reg[index].i_addr + copy_off, copy_len);
- xlog_write_adv_cnt(ptr, len, log_offset, copy_len);
-
- /* make copy_len total bytes copied, including headers */
- copy_len += start_rec_copy + sizeof(xlog_op_header_t);
- record_cnt++;
- data_cnt += contwr ? copy_len : 0;
-
- error = xlog_write_copy_finish(log, iclog, flags,
- &record_cnt, &data_cnt,
- &partial_copy, &partial_copy_len,
- log_offset, commit_iclog);
- if (error)
- return error;
-
- /*
- * if we had a partial copy, we need to get more iclog
- * space but we don't want to increment the region
- * index because there is still more is this region to write.
- *
- * If we completed writing this region, and we flushed
- * the iclog (indicated by resetting of the record
- * count), then we also need to get more log space. If
- * this was the last record, though, we are done and
- * can just return.
- */
- if (partial_copy)
- break;
-
- index++;
- if (record_cnt == 0) {
- if (index == nentries)
- return 0;
- break;
- }
- } /* while (index < nentries) */
- } /* for (index = 0; index < nentries; ) */
- ASSERT(len == 0);
+ for (index = 0; index < nentries; ) {
+ __psint_t ptr;
+ int log_offset;
+
+ error = xlog_state_get_iclog_space(log, len, &iclog, ticket,
+ &contwr, &log_offset);
+ if (error)
+ return error;
+
+ ASSERT(log_offset <= iclog->ic_size - 1);
+ ptr = (__psint_t) ((char *)iclog->ic_datap+log_offset);
+
+ /* start_lsn is the first lsn written to. That's all we need. */
+ if (!*start_lsn)
+ *start_lsn = be64_to_cpu(iclog->ic_header.h_lsn);
+
+ /*
+ * This loop writes out as many regions as can fit in the amount
+ * of space which was allocated by xlog_state_get_iclog_space().
+ */
+ while (index < nentries) {
+ struct xlog_op_header *ophdr;
+ int start_rec_copy;
+ int copy_len;
+ int copy_off;
+
+ ASSERT(reg[index].i_len % sizeof(__int32_t) == 0);
+ ASSERT((__psint_t)ptr % sizeof(__int32_t) == 0);
+
+ start_rec_copy = xlog_write_start_rec(ptr, ticket);
+ if (start_rec_copy) {
+ record_cnt++;
+ xlog_write_adv_cnt(ptr, len, log_offset,
+ start_rec_copy);
+ }
+
+ ophdr = xlog_write_setup_ophdr(log, ptr, ticket, flags);
+ if (!ophdr)
+ return XFS_ERROR(EIO);
+
+ xlog_write_adv_cnt(ptr, len, log_offset,
+ sizeof(struct xlog_op_header));
+
+ len += xlog_write_setup_copy(ticket, ophdr,
+ iclog->ic_size-log_offset,
+ reg[index].i_len,
+ ©_off, ©_len,
+ &partial_copy,
+ &partial_copy_len);
+ xlog_verify_dest_ptr(log, ptr);
+
+ /* copy region */
+ ASSERT(copy_len >= 0);
+ memcpy((xfs_caddr_t)ptr, reg[index].i_addr + copy_off,
+ copy_len);
+ xlog_write_adv_cnt(ptr, len, log_offset, copy_len);
+
+ copy_len += start_rec_copy + sizeof(xlog_op_header_t);
+ record_cnt++;
+ data_cnt += contwr ? copy_len : 0;
+
+ error = xlog_write_copy_finish(log, iclog, flags,
+ &record_cnt, &data_cnt,
+ &partial_copy,
+ &partial_copy_len,
+ log_offset,
+ commit_iclog);
+ if (error)
+ return error;
+
+ /*
+ * if we had a partial copy, we need to get more iclog
+ * space but we don't want to increment the region
+ * index because there is still more is this region to
+ * write.
+ *
+ * If we completed writing this region, and we flushed
+ * the iclog (indicated by resetting of the record
+ * count), then we also need to get more log space. If
+ * this was the last record, though, we are done and
+ * can just return.
+ */
+ if (partial_copy)
+ break;
+
+ index++;
+ if (record_cnt == 0) {
+ if (index == nentries)
+ return 0;
+ break;
+ }
+ }
+ }
+
+ ASSERT(len == 0);
+
+ xlog_state_finish_copy(log, iclog, record_cnt, data_cnt);
+ if (!commit_iclog)
+ return xlog_state_release_iclog(log, iclog);
- xlog_state_finish_copy(log, iclog, record_cnt, data_cnt);
- if (commit_iclog) {
ASSERT(flags & XLOG_COMMIT_TRANS);
*commit_iclog = iclog;
return 0;
- }
- return xlog_state_release_iclog(log, iclog);
-} /* xlog_write */
+}
/*****************************************************************************
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 3/4] xfs: introduce new internal log vector structure
2010-03-15 15:51 [PATCH 0/4] xfs_log_write cleanups and vector support Christoph Hellwig
2010-03-15 15:51 ` [PATCH 1/4] xfs: factor xlog_write Christoph Hellwig
2010-03-15 15:51 ` [PATCH 2/4] xfs: reindent xlog_write Christoph Hellwig
@ 2010-03-15 15:51 ` Christoph Hellwig
2010-03-15 15:51 ` [PATCH 4/4] xfs: clean up xlog_write_adv_cnt Christoph Hellwig
2010-03-16 1:53 ` [PATCH 0/4] xfs_log_write cleanups and vector support Dave Chinner
4 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2010-03-15 15:51 UTC (permalink / raw)
To: david; +Cc: Dave Chinner, xfs
[-- Attachment #1: xlog-vectors --]
[-- Type: text/plain, Size: 10117 bytes --]
From: Dave Chinner <dchinner@redhat.com>
The current log IO vector structure is a flat array and not
extensible. To make it possible to keep separate log IO vectors for
individual log items, we need a method of chaining log IO vectors
together.
Introduce a new log vector type that can be used to wrap the
existing log IO vectors on use that internally to the log. This
means that the existing external interface (xfs_log_write) does not
change and hence no changes to the transaction commit code are
required.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Index: xfs/fs/xfs/xfs_log.c
===================================================================
--- xfs.orig/fs/xfs/xfs_log.c 2010-03-15 13:41:50.000000000 +0100
+++ xfs/fs/xfs/xfs_log.c 2010-03-15 13:43:23.056255602 +0100
@@ -50,7 +50,7 @@ kmem_zone_t *xfs_log_ticket_zone;
(off) += (bytes);}
/* Local miscellaneous function prototypes */
-STATIC int xlog_commit_record(xfs_mount_t *mp, xlog_ticket_t *ticket,
+STATIC int xlog_commit_record(struct log *log, struct xlog_ticket *ticket,
xlog_in_core_t **, xfs_lsn_t *);
STATIC xlog_t * xlog_alloc_log(xfs_mount_t *mp,
xfs_buftarg_t *log_target,
@@ -59,11 +59,9 @@ STATIC xlog_t * xlog_alloc_log(xfs_moun
STATIC int xlog_space_left(xlog_t *log, int cycle, int bytes);
STATIC int xlog_sync(xlog_t *log, xlog_in_core_t *iclog);
STATIC void xlog_dealloc_log(xlog_t *log);
-STATIC int xlog_write(xfs_mount_t *mp, xfs_log_iovec_t region[],
- int nentries, struct xlog_ticket *tic,
- xfs_lsn_t *start_lsn,
- xlog_in_core_t **commit_iclog,
- uint flags);
+STATIC int xlog_write(struct log *log, struct xfs_log_vec *log_vector,
+ struct xlog_ticket *tic, xfs_lsn_t *start_lsn,
+ xlog_in_core_t **commit_iclog, uint flags);
/* local state machine functions */
STATIC void xlog_state_done_syncing(xlog_in_core_t *iclog, int);
@@ -258,7 +256,7 @@ xfs_log_done(
* If we get an error, just continue and give back the log ticket.
*/
(((ticket->t_flags & XLOG_TIC_INITED) == 0) &&
- (xlog_commit_record(mp, ticket, iclog, &lsn)))) {
+ (xlog_commit_record(log, ticket, iclog, &lsn)))) {
lsn = (xfs_lsn_t) -1;
if (ticket->t_flags & XLOG_TIC_PERM_RESERV) {
flags |= XFS_LOG_REL_PERM_RESERV;
@@ -516,18 +514,10 @@ xfs_log_unmount_write(xfs_mount_t *mp)
#ifdef DEBUG
xlog_in_core_t *first_iclog;
#endif
- xfs_log_iovec_t reg[1];
xlog_ticket_t *tic = NULL;
xfs_lsn_t lsn;
int error;
- /* the data section must be 32 bit size aligned */
- struct {
- __uint16_t magic;
- __uint16_t pad1;
- __uint32_t pad2; /* may as well make it 64 bits */
- } magic = { XLOG_UNMOUNT_TYPE, 0, 0 };
-
/*
* Don't write out unmount record on read-only mounts.
* Or, if we are doing a forced umount (typically because of IO errors).
@@ -549,16 +539,30 @@ xfs_log_unmount_write(xfs_mount_t *mp)
} while (iclog != first_iclog);
#endif
if (! (XLOG_FORCED_SHUTDOWN(log))) {
- reg[0].i_addr = (void*)&magic;
- reg[0].i_len = sizeof(magic);
- reg[0].i_type = XLOG_REG_TYPE_UNMOUNT;
-
error = xfs_log_reserve(mp, 600, 1, &tic,
XFS_LOG, 0, XLOG_UNMOUNT_REC_TYPE);
if (!error) {
+ /* the data section must be 32 bit size aligned */
+ struct {
+ __uint16_t magic;
+ __uint16_t pad1;
+ __uint32_t pad2; /* may as well make it 64 bits */
+ } magic = {
+ .magic = XLOG_UNMOUNT_TYPE,
+ };
+ struct xfs_log_iovec reg = {
+ .i_addr = (void *)&magic,
+ .i_len = sizeof(magic),
+ .i_type = XLOG_REG_TYPE_UNMOUNT,
+ };
+ struct xfs_log_vec vec = {
+ .lv_niovecs = 1,
+ .lv_iovecp = ®,
+ };
+
/* remove inited flag */
- ((xlog_ticket_t *)tic)->t_flags = 0;
- error = xlog_write(mp, reg, 1, tic, &lsn,
+ tic->t_flags = 0;
+ error = xlog_write(log, &vec, tic, &lsn,
NULL, XLOG_UNMOUNT_TRANS);
/*
* At this point, we're umounting anyway,
@@ -679,11 +683,15 @@ xfs_log_write(
{
struct log *log = mp->m_log;
int error;
+ struct xfs_log_vec vec = {
+ .lv_niovecs = nentries,
+ .lv_iovecp = reg,
+ };
if (XLOG_FORCED_SHUTDOWN(log))
return XFS_ERROR(EIO);
- error = xlog_write(mp, reg, nentries, tic, start_lsn, NULL, 0);
+ error = xlog_write(log, &vec, tic, start_lsn, NULL, 0);
if (error)
xfs_force_shutdown(mp, SHUTDOWN_LOG_IO_ERROR);
return error;
@@ -1176,26 +1184,31 @@ out:
* ticket. Return the lsn of the commit record.
*/
STATIC int
-xlog_commit_record(xfs_mount_t *mp,
- xlog_ticket_t *ticket,
- xlog_in_core_t **iclog,
- xfs_lsn_t *commitlsnp)
+xlog_commit_record(
+ struct log *log,
+ struct xlog_ticket *ticket,
+ struct xlog_in_core **iclog,
+ xfs_lsn_t *commitlsnp)
{
- int error;
- xfs_log_iovec_t reg[1];
-
- reg[0].i_addr = NULL;
- reg[0].i_len = 0;
- reg[0].i_type = XLOG_REG_TYPE_COMMIT;
+ struct xfs_mount *mp = log->l_mp;
+ int error;
+ struct xfs_log_iovec reg = {
+ .i_addr = NULL,
+ .i_len = 0,
+ .i_type = XLOG_REG_TYPE_COMMIT,
+ };
+ struct xfs_log_vec vec = {
+ .lv_niovecs = 1,
+ .lv_iovecp = ®,
+ };
ASSERT_ALWAYS(iclog);
- if ((error = xlog_write(mp, reg, 1, ticket, commitlsnp,
- iclog, XLOG_COMMIT_TRANS))) {
+ error = xlog_write(log, &vec, ticket, commitlsnp, iclog,
+ XLOG_COMMIT_TRANS);
+ if (error)
xfs_force_shutdown(mp, SHUTDOWN_LOG_IO_ERROR);
- }
return error;
-} /* xlog_commit_record */
-
+}
/*
* Push on the buffer cache code if we ever use more than 75% of the on-disk
@@ -1622,9 +1635,9 @@ xlog_print_tic_res(xfs_mount_t *mp, xlog
static int
xlog_write_calc_vec_length(
struct xlog_ticket *ticket,
- struct xfs_log_iovec reg[],
- int nentries)
+ struct xfs_log_vec *log_vector)
{
+ struct xfs_log_vec *lv;
int headers = 0;
int len = 0;
int i;
@@ -1633,12 +1646,15 @@ xlog_write_calc_vec_length(
if (ticket->t_flags & XLOG_TIC_INITED)
headers++;
- for (i = 0; i < nentries; i++) {
- /* each region gets >= 1 */
- headers++;
+ for (lv = log_vector; lv; lv = lv->lv_next) {
+ headers += lv->lv_niovecs;
+
+ for (i = 0; i < lv->lv_niovecs; i++) {
+ struct xfs_log_iovec *vecp = &lv->lv_iovecp[i];
- len += reg[i].i_len;
- xlog_tic_add_region(ticket, reg[i].i_len, reg[i].i_type);
+ len += vecp->i_len;
+ xlog_tic_add_region(ticket, vecp->i_len, vecp->i_type);
+ }
}
ticket->t_res_num_ophdrs += headers;
@@ -1844,16 +1860,16 @@ xlog_write_copy_finish(
*/
STATIC int
xlog_write(
- struct xfs_mount *mp,
- struct xfs_log_iovec reg[],
- int nentries,
+ struct log *log,
+ struct xfs_log_vec *log_vector,
struct xlog_ticket *ticket,
xfs_lsn_t *start_lsn,
struct xlog_in_core **commit_iclog,
uint flags)
{
- struct log *log = mp->m_log;
struct xlog_in_core *iclog = NULL;
+ struct xfs_log_iovec *vecp;
+ struct xfs_log_vec *lv;
int len;
int index;
int partial_copy = 0;
@@ -1865,25 +1881,28 @@ xlog_write(
*start_lsn = 0;
- len = xlog_write_calc_vec_length(ticket, reg, nentries);
+ len = xlog_write_calc_vec_length(ticket, log_vector);
if (ticket->t_curr_res < len) {
- xlog_print_tic_res(mp, ticket);
+ xlog_print_tic_res(log->l_mp, ticket);
#ifdef DEBUG
xlog_panic(
"xfs_log_write: reservation ran out. Need to up reservation");
#else
/* Customer configurable panic */
- xfs_cmn_err(XFS_PTAG_LOGRES, CE_ALERT, mp,
+ xfs_cmn_err(XFS_PTAG_LOGRES, CE_ALERT, log->l_mp,
"xfs_log_write: reservation ran out. Need to up reservation");
/* If we did not panic, shutdown the filesystem */
- xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
+ xfs_force_shutdown(log->l_mp, SHUTDOWN_CORRUPT_INCORE);
#endif
}
ticket->t_curr_res -= len;
- for (index = 0; index < nentries; ) {
+ index = 0;
+ lv = log_vector;
+ vecp = lv->lv_iovecp;
+ while (lv && index < lv->lv_niovecs) {
__psint_t ptr;
int log_offset;
@@ -1903,13 +1922,14 @@ xlog_write(
* This loop writes out as many regions as can fit in the amount
* of space which was allocated by xlog_state_get_iclog_space().
*/
- while (index < nentries) {
+ while (lv && index < lv->lv_niovecs) {
+ struct xfs_log_iovec *reg = &vecp[index];
struct xlog_op_header *ophdr;
int start_rec_copy;
int copy_len;
int copy_off;
- ASSERT(reg[index].i_len % sizeof(__int32_t) == 0);
+ ASSERT(reg->i_len % sizeof(__int32_t) == 0);
ASSERT((__psint_t)ptr % sizeof(__int32_t) == 0);
start_rec_copy = xlog_write_start_rec(ptr, ticket);
@@ -1928,7 +1948,7 @@ xlog_write(
len += xlog_write_setup_copy(ticket, ophdr,
iclog->ic_size-log_offset,
- reg[index].i_len,
+ reg->i_len,
©_off, ©_len,
&partial_copy,
&partial_copy_len);
@@ -1936,7 +1956,7 @@ xlog_write(
/* copy region */
ASSERT(copy_len >= 0);
- memcpy((xfs_caddr_t)ptr, reg[index].i_addr + copy_off,
+ memcpy((xfs_caddr_t)ptr, reg->i_addr + copy_off,
copy_len);
xlog_write_adv_cnt(ptr, len, log_offset, copy_len);
@@ -1968,9 +1988,14 @@ xlog_write(
if (partial_copy)
break;
- index++;
+ if (++index == lv->lv_niovecs) {
+ lv = lv->lv_next;
+ index = 0;
+ if (lv)
+ vecp = lv->lv_iovecp;
+ }
if (record_cnt == 0) {
- if (index == nentries)
+ if (!lv)
return 0;
break;
}
Index: xfs/fs/xfs/xfs_log.h
===================================================================
--- xfs.orig/fs/xfs/xfs_log.h 2010-03-15 13:41:44.000000000 +0100
+++ xfs/fs/xfs/xfs_log.h 2010-03-15 13:41:53.138032456 +0100
@@ -110,6 +110,12 @@ typedef struct xfs_log_iovec {
uint i_type; /* type of region */
} xfs_log_iovec_t;
+struct xfs_log_vec {
+ struct xfs_log_vec *lv_next; /* next lv in build list */
+ int lv_niovecs; /* number of iovecs in lv */
+ struct xfs_log_iovec *lv_iovecp; /* iovec array */
+};
+
/*
* Structure used to pass callback function and the function's argument
* to the log manager.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 4/4] xfs: clean up xlog_write_adv_cnt
2010-03-15 15:51 [PATCH 0/4] xfs_log_write cleanups and vector support Christoph Hellwig
` (2 preceding siblings ...)
2010-03-15 15:51 ` [PATCH 3/4] xfs: introduce new internal log vector structure Christoph Hellwig
@ 2010-03-15 15:51 ` Christoph Hellwig
2010-03-16 1:53 ` [PATCH 0/4] xfs_log_write cleanups and vector support Dave Chinner
4 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2010-03-15 15:51 UTC (permalink / raw)
To: david; +Cc: xfs
[-- Attachment #1: xfs-cleanup-xlog_write_adv_cnt --]
[-- Type: text/plain, Size: 5182 bytes --]
Replace the awkward xlog_write_adv_cnt with an inline helper that makes
it more obvious that it's modifying it's paramters, and replace the use
of an integer type for "ptr" with a real void pointer. Also move
xlog_write_adv_cnt to xfs_log_priv.h as it will be used outside of
xfs_log.c in the delayed logging series.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: xfs/fs/xfs/xfs_log.c
===================================================================
--- xfs.orig/fs/xfs/xfs_log.c 2010-03-15 13:43:23.000000000 +0100
+++ xfs/fs/xfs/xfs_log.c 2010-03-15 13:48:05.503006054 +0100
@@ -44,11 +44,6 @@
kmem_zone_t *xfs_log_ticket_zone;
-#define xlog_write_adv_cnt(ptr, len, off, bytes) \
- { (ptr) += (bytes); \
- (len) -= (bytes); \
- (off) += (bytes);}
-
/* Local miscellaneous function prototypes */
STATIC int xlog_commit_record(struct log *log, struct xlog_ticket *ticket,
xlog_in_core_t **, xfs_lsn_t *);
@@ -100,7 +95,7 @@ STATIC xlog_ticket_t *xlog_ticket_alloc(
uint flags);
#if defined(DEBUG)
-STATIC void xlog_verify_dest_ptr(xlog_t *log, __psint_t ptr);
+STATIC void xlog_verify_dest_ptr(xlog_t *log, char *ptr);
STATIC void xlog_verify_grant_head(xlog_t *log, int equals);
STATIC void xlog_verify_iclog(xlog_t *log, xlog_in_core_t *iclog,
int count, boolean_t syncing);
@@ -1669,11 +1664,9 @@ xlog_write_calc_vec_length(
*/
static int
xlog_write_start_rec(
- __psint_t ptr,
+ struct xlog_op_header *ophdr,
struct xlog_ticket *ticket)
{
- struct xlog_op_header *ophdr = (struct xlog_op_header *)ptr;
-
if (!(ticket->t_flags & XLOG_TIC_INITED))
return 0;
@@ -1691,12 +1684,10 @@ xlog_write_start_rec(
static xlog_op_header_t *
xlog_write_setup_ophdr(
struct log *log,
- __psint_t ptr,
+ struct xlog_op_header *ophdr,
struct xlog_ticket *ticket,
uint flags)
{
- struct xlog_op_header *ophdr = (struct xlog_op_header *)ptr;
-
ophdr->oh_tid = cpu_to_be32(ticket->t_tid);
ophdr->oh_clientid = ticket->t_clientid;
ophdr->oh_res2 = 0;
@@ -1903,7 +1894,7 @@ xlog_write(
lv = log_vector;
vecp = lv->lv_iovecp;
while (lv && index < lv->lv_niovecs) {
- __psint_t ptr;
+ void *ptr;
int log_offset;
error = xlog_state_get_iclog_space(log, len, &iclog, ticket,
@@ -1912,7 +1903,7 @@ xlog_write(
return error;
ASSERT(log_offset <= iclog->ic_size - 1);
- ptr = (__psint_t) ((char *)iclog->ic_datap+log_offset);
+ ptr = iclog->ic_datap + log_offset;
/* start_lsn is the first lsn written to. That's all we need. */
if (!*start_lsn)
@@ -1930,12 +1921,12 @@ xlog_write(
int copy_off;
ASSERT(reg->i_len % sizeof(__int32_t) == 0);
- ASSERT((__psint_t)ptr % 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++;
- xlog_write_adv_cnt(ptr, len, log_offset,
+ xlog_write_adv_cnt(&ptr, &len, &log_offset,
start_rec_copy);
}
@@ -1943,7 +1934,7 @@ xlog_write(
if (!ophdr)
return XFS_ERROR(EIO);
- xlog_write_adv_cnt(ptr, len, log_offset,
+ xlog_write_adv_cnt(&ptr, &len, &log_offset,
sizeof(struct xlog_op_header));
len += xlog_write_setup_copy(ticket, ophdr,
@@ -1956,9 +1947,8 @@ xlog_write(
/* copy region */
ASSERT(copy_len >= 0);
- memcpy((xfs_caddr_t)ptr, reg->i_addr + copy_off,
- copy_len);
- xlog_write_adv_cnt(ptr, len, log_offset, copy_len);
+ memcpy(ptr, reg->i_addr + copy_off, copy_len);
+ xlog_write_adv_cnt(&ptr, &len, &log_offset, copy_len);
copy_len += start_rec_copy + sizeof(xlog_op_header_t);
record_cnt++;
@@ -3440,20 +3430,22 @@ xlog_ticket_alloc(
* part of the log in case we trash the log structure.
*/
void
-xlog_verify_dest_ptr(xlog_t *log,
- __psint_t ptr)
+xlog_verify_dest_ptr(
+ struct log *log,
+ char *ptr)
{
int i;
int good_ptr = 0;
- for (i=0; i < log->l_iclog_bufs; i++) {
- if (ptr >= (__psint_t)log->l_iclog_bak[i] &&
- ptr <= (__psint_t)log->l_iclog_bak[i]+log->l_iclog_size)
+ for (i = 0; i < log->l_iclog_bufs; i++) {
+ if (ptr >= log->l_iclog_bak[i] &&
+ ptr <= log->l_iclog_bak[i]+log->l_iclog_size)
good_ptr++;
}
- if (! good_ptr)
+
+ if (!good_ptr)
xlog_panic("xlog_verify_dest_ptr: invalid ptr");
-} /* xlog_verify_dest_ptr */
+}
STATIC void
xlog_verify_grant_head(xlog_t *log, int equals)
Index: xfs/fs/xfs/xfs_log_priv.h
===================================================================
--- xfs.orig/fs/xfs/xfs_log_priv.h 2010-03-15 13:48:00.139261678 +0100
+++ xfs/fs/xfs/xfs_log_priv.h 2010-03-15 13:48:20.042033287 +0100
@@ -449,6 +449,14 @@ extern void xlog_pack_data(xlog_t *log,
extern kmem_zone_t *xfs_log_ticket_zone;
+static inline void
+xlog_write_adv_cnt(void **ptr, int *len, int *off, size_t bytes)
+{
+ *ptr += bytes;
+ *len -= bytes;
+ *off += bytes;
+}
+
/*
* Unmount record type is used as a pseudo transaction type for the ticket.
* It's value must be outside the range of XFS_TRANS_* values.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 0/4] xfs_log_write cleanups and vector support
2010-03-15 15:51 [PATCH 0/4] xfs_log_write cleanups and vector support Christoph Hellwig
` (3 preceding siblings ...)
2010-03-15 15:51 ` [PATCH 4/4] xfs: clean up xlog_write_adv_cnt Christoph Hellwig
@ 2010-03-16 1:53 ` Dave Chinner
4 siblings, 0 replies; 6+ messages in thread
From: Dave Chinner @ 2010-03-16 1:53 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
On Mon, Mar 15, 2010 at 11:51:51AM -0400, Christoph Hellwig wrote:
> As mention during the review of the first posting of the transaction
> subsystem cleanup series I really didn't like the concept of mixing
> the xfs_log_write cleanup with adding the new vector functionality.
>
> Here's a patch series to reshuffle this work into a series that I
> feel able to follow and throw in some minimal additional cleanups.
>
> First patch is the factoring out of helpers from xlog_write without
> functionality change, second is a re-indent of xlog_write including
> moving variable declaration to inner blocks, again without functionality
> change, third is the introduction of the log vectors, including the
> relatively small changes in xlog_write, and fourth is a small incremental
> cleanup patch.
I've had a quick look over this and it seems ok. I'll test and
review them more closely, and if they are OK I'll add them to the
trans-cleanup branch...
Thanks for doing this, Christoph.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread