* [PATCH 1/9] xfs: don't try to write a start record into every iclog
2020-03-23 13:06 xfs: clean up log tickets and record writes v2 Christoph Hellwig
@ 2020-03-23 13:06 ` Christoph Hellwig
2020-03-23 13:06 ` [PATCH 2/9] xfs: re-order initial space accounting checks in xlog_write Christoph Hellwig
` (8 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2020-03-23 13:06 UTC (permalink / raw)
To: linux-xfs; +Cc: david, Dave Chinner
From: Dave Chinner <dchinner@redhat.com>
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() 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.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
[hch: use an need_start_rec bool]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_log.c | 63 ++++++++++++++++++++++++------------------------
1 file changed, 32 insertions(+), 31 deletions(-)
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 2a90a483c2d6..bf071552094a 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -2112,23 +2112,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)
@@ -2150,27 +2148,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 *
@@ -2372,25 +2359,29 @@ xlog_write(
int record_cnt = 0;
int data_cnt = 0;
int error = 0;
+ bool need_start_rec = true;
*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)
- 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.
*/
- if (flags & (XLOG_COMMIT_TRANS | XLOG_UNMOUNT_TRANS))
- ticket->t_curr_res -= sizeof(xlog_op_header_t);
+ if (flags & (XLOG_COMMIT_TRANS | XLOG_UNMOUNT_TRANS)) {
+ ticket->t_curr_res -= sizeof(struct xlog_op_header);
+ need_start_rec = false;
+ }
if (ticket->t_curr_res < 0) {
xfs_alert_tag(log->l_mp, XFS_PTAG_LOGRES,
@@ -2399,6 +2390,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;
@@ -2425,7 +2418,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;
@@ -2441,11 +2433,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);
@@ -2477,8 +2473,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,
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 2/9] xfs: re-order initial space accounting checks in xlog_write
2020-03-23 13:06 xfs: clean up log tickets and record writes v2 Christoph Hellwig
2020-03-23 13:06 ` [PATCH 1/9] xfs: don't try to write a start record into every iclog Christoph Hellwig
@ 2020-03-23 13:06 ` Christoph Hellwig
2020-03-23 13:07 ` [PATCH 3/9] xfs: refactor and split xfs_log_done() Christoph Hellwig
` (7 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2020-03-23 13:06 UTC (permalink / raw)
To: linux-xfs; +Cc: david, Dave Chinner
From: Dave Chinner <dchinner@redhat.com>
Commit and unmount records records do not need start records to be
written, so rearrange the logic in xlog_write() to remove the need
to check for XLOG_TIC_INITED to determine if we should account for
the space used by a start record.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_log.c | 38 ++++++++++++--------------------------
1 file changed, 12 insertions(+), 26 deletions(-)
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index bf071552094a..116f59b16b04 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -2349,39 +2349,27 @@ xlog_write(
uint flags)
{
struct xlog_in_core *iclog = NULL;
- struct xfs_log_iovec *vecp;
- struct xfs_log_vec *lv;
+ struct xfs_log_vec *lv = log_vector;
+ struct xfs_log_iovec *vecp = lv->lv_iovecp;
+ int index = 0;
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 = 0;
- bool need_start_rec = true;
-
- *start_lsn = 0;
-
+ bool need_start_rec;
/*
- * Region headers and bytes are already accounted for.
- * We only need to take into account start records and
- * split regions in this function.
+ * 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 or unmount header that gets written. Hence we always have
+ * to account for an extra xlog_op_header here.
*/
- if (ticket->t_flags & XLOG_TIC_INITED) {
- ticket->t_curr_res -= sizeof(struct xlog_op_header);
+ ticket->t_curr_res -= sizeof(struct xlog_op_header);
+ if (ticket->t_flags & XLOG_TIC_INITED)
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)) {
- ticket->t_curr_res -= sizeof(struct xlog_op_header);
- need_start_rec = false;
- }
if (ticket->t_curr_res < 0) {
xfs_alert_tag(log->l_mp, XFS_PTAG_LOGRES,
@@ -2390,11 +2378,9 @@ xlog_write(
xfs_force_shutdown(log->l_mp, SHUTDOWN_LOG_IO_ERROR);
}
+ need_start_rec = !(flags & (XLOG_COMMIT_TRANS | XLOG_UNMOUNT_TRANS));
len = xlog_write_calc_vec_length(ticket, log_vector, need_start_rec);
-
- index = 0;
- lv = log_vector;
- vecp = lv->lv_iovecp;
+ *start_lsn = 0;
while (lv && (!lv->lv_niovecs || index < lv->lv_niovecs)) {
void *ptr;
int log_offset;
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 3/9] xfs: refactor and split xfs_log_done()
2020-03-23 13:06 xfs: clean up log tickets and record writes v2 Christoph Hellwig
2020-03-23 13:06 ` [PATCH 1/9] xfs: don't try to write a start record into every iclog Christoph Hellwig
2020-03-23 13:06 ` [PATCH 2/9] xfs: re-order initial space accounting checks in xlog_write Christoph Hellwig
@ 2020-03-23 13:07 ` Christoph Hellwig
2020-03-23 13:07 ` [PATCH 4/9] xfs: split xlog_ticket_done Christoph Hellwig
` (6 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2020-03-23 13:07 UTC (permalink / raw)
To: linux-xfs; +Cc: david, Dave Chinner
From: Dave Chinner <dchinner@redhat.com>
xfs_log_done() does two separate things. Firstly, it triggers commit
records to be written for permanent transactions, and secondly it
releases or regrants transaction reservation space.
Since delayed logging was introduced, transactions no longer write
directly to the log, hence they never have the XLOG_TIC_INITED flag
cleared on them. Hence transactions never write commit records to
the log and only need to modify reservation space.
Split up xfs_log_done into two parts, and only call the parts of the
operation needed for the context xfs_log_done() is currently being
called from.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_log.c | 65 +++++++++++++------------------------------
fs/xfs/xfs_log.h | 4 ---
fs/xfs/xfs_log_cil.c | 13 +++++----
fs/xfs/xfs_log_priv.h | 16 +++++------
fs/xfs/xfs_trans.c | 24 ++++++++--------
5 files changed, 47 insertions(+), 75 deletions(-)
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 116f59b16b04..4b24fbf8b06c 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -487,62 +487,40 @@ xfs_log_reserve(
*/
/*
- * This routine is called when a user of a log manager ticket is done with
- * the reservation. If the ticket was ever used, then a commit record for
- * the associated transaction is written out as a log operation header with
- * no data. The flag XLOG_TIC_INITED is set when the first write occurs with
- * a given ticket. If the ticket was one with a permanent reservation, then
- * a few operations are done differently. Permanent reservation tickets by
- * default don't release the reservation. They just commit the current
- * transaction with the belief that the reservation is still needed. A flag
- * must be passed in before permanent reservations are actually released.
- * When these type of tickets are not released, they need to be set into
- * the inited state again. By doing this, a start record will be written
- * out when the next write occurs.
+ * Write a commit record to the log to close off a running log write.
*/
-xfs_lsn_t
-xfs_log_done(
- struct xfs_mount *mp,
+int
+xlog_write_done(
+ struct xlog *log,
struct xlog_ticket *ticket,
struct xlog_in_core **iclog,
- bool regrant)
+ xfs_lsn_t *lsn)
{
- struct xlog *log = mp->m_log;
- xfs_lsn_t lsn = 0;
-
- if (XLOG_FORCED_SHUTDOWN(log) ||
- /*
- * If nothing was ever written, don't write out commit record.
- * If we get an error, just continue and give back the log ticket.
- */
- (((ticket->t_flags & XLOG_TIC_INITED) == 0) &&
- (xlog_commit_record(log, ticket, iclog, &lsn)))) {
- lsn = (xfs_lsn_t) -1;
- regrant = false;
- }
+ if (XLOG_FORCED_SHUTDOWN(log))
+ return -EIO;
+ return xlog_commit_record(log, ticket, iclog, lsn);
+}
+/*
+ * Release or regrant the ticket reservation now the transaction is done with
+ * it depending on caller context. Rolling transactions need the ticket
+ * regranted, otherwise we release it completely.
+ */
+void
+xlog_ticket_done(
+ struct xlog *log,
+ struct xlog_ticket *ticket,
+ bool regrant)
+{
if (!regrant) {
trace_xfs_log_done_nonperm(log, ticket);
-
- /*
- * Release ticket if not permanent reservation or a specific
- * request has been made to release a permanent reservation.
- */
xlog_ungrant_log_space(log, ticket);
} else {
trace_xfs_log_done_perm(log, ticket);
-
xlog_regrant_reserve_log_space(log, ticket);
- /* If this ticket was a permanent reservation and we aren't
- * trying to release it, reset the inited flags; so next time
- * we write, a start record will be written out.
- */
- ticket->t_flags |= XLOG_TIC_INITED;
}
-
xfs_log_ticket_put(ticket);
- return lsn;
}
static bool
@@ -2368,9 +2346,6 @@ xlog_write(
* to account for an extra xlog_op_header here.
*/
ticket->t_curr_res -= sizeof(struct xlog_op_header);
- if (ticket->t_flags & XLOG_TIC_INITED)
- ticket->t_flags &= ~XLOG_TIC_INITED;
-
if (ticket->t_curr_res < 0) {
xfs_alert_tag(log->l_mp, XFS_PTAG_LOGRES,
"ctx ticket reservation ran out. Need to up reservation");
diff --git a/fs/xfs/xfs_log.h b/fs/xfs/xfs_log.h
index cc77cc36560a..1412d6993f1e 100644
--- a/fs/xfs/xfs_log.h
+++ b/fs/xfs/xfs_log.h
@@ -105,10 +105,6 @@ struct xfs_log_item;
struct xfs_item_ops;
struct xfs_trans;
-xfs_lsn_t xfs_log_done(struct xfs_mount *mp,
- struct xlog_ticket *ticket,
- struct xlog_in_core **iclog,
- bool regrant);
int xfs_log_force(struct xfs_mount *mp, uint flags);
int xfs_log_force_lsn(struct xfs_mount *mp, xfs_lsn_t lsn, uint flags,
int *log_forced);
diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
index 64cc0bf2ab3b..880de1aa4288 100644
--- a/fs/xfs/xfs_log_cil.c
+++ b/fs/xfs/xfs_log_cil.c
@@ -839,10 +839,11 @@ xlog_cil_push_work(
}
spin_unlock(&cil->xc_push_lock);
- /* xfs_log_done always frees the ticket on error. */
- commit_lsn = xfs_log_done(log->l_mp, tic, &commit_iclog, false);
- if (commit_lsn == -1)
- goto out_abort;
+ error = xlog_write_done(log, tic, &commit_iclog, &commit_lsn);
+ if (error)
+ goto out_abort_free_ticket;
+
+ xlog_ticket_done(log, tic, false);
spin_lock(&commit_iclog->ic_callback_lock);
if (commit_iclog->ic_state == XLOG_STATE_IOERROR) {
@@ -875,7 +876,7 @@ xlog_cil_push_work(
return;
out_abort_free_ticket:
- xfs_log_ticket_put(tic);
+ xlog_ticket_done(log, tic, false);
out_abort:
ASSERT(XLOG_FORCED_SHUTDOWN(log));
xlog_cil_committed(ctx);
@@ -1007,7 +1008,7 @@ xfs_log_commit_cil(
if (commit_lsn)
*commit_lsn = xc_commit_lsn;
- xfs_log_done(mp, tp->t_ticket, NULL, regrant);
+ xlog_ticket_done(log, tp->t_ticket, regrant);
tp->t_ticket = NULL;
xfs_trans_unreserve_and_mod_sb(tp);
diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
index 2b0aec37e73e..32bb6856e69d 100644
--- a/fs/xfs/xfs_log_priv.h
+++ b/fs/xfs/xfs_log_priv.h
@@ -439,14 +439,14 @@ 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);
+int xlog_write_done(struct xlog *log, struct xlog_ticket *ticket,
+ struct xlog_in_core **iclog, xfs_lsn_t *lsn);
+void xlog_ticket_done(struct xlog *log, struct xlog_ticket *ticket,
+ bool regrant);
/*
* When we crack an atomic LSN, we sample it first so that the value will not
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 73c534093f09..123ecc8435f6 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -9,6 +9,7 @@
#include "xfs_shared.h"
#include "xfs_format.h"
#include "xfs_log_format.h"
+#include "xfs_log_priv.h"
#include "xfs_trans_resv.h"
#include "xfs_mount.h"
#include "xfs_extent_busy.h"
@@ -150,8 +151,9 @@ xfs_trans_reserve(
uint blocks,
uint rtextents)
{
- int error = 0;
- bool rsvd = (tp->t_flags & XFS_TRANS_RESERVE) != 0;
+ struct xfs_mount *mp = tp->t_mountp;
+ int error = 0;
+ bool rsvd = (tp->t_flags & XFS_TRANS_RESERVE) != 0;
/* Mark this thread as being in a transaction */
current_set_flags_nested(&tp->t_pflags, PF_MEMALLOC_NOFS);
@@ -162,7 +164,7 @@ xfs_trans_reserve(
* fail if the count would go below zero.
*/
if (blocks > 0) {
- error = xfs_mod_fdblocks(tp->t_mountp, -((int64_t)blocks), rsvd);
+ error = xfs_mod_fdblocks(mp, -((int64_t)blocks), rsvd);
if (error != 0) {
current_restore_flags_nested(&tp->t_pflags, PF_MEMALLOC_NOFS);
return -ENOSPC;
@@ -191,9 +193,9 @@ xfs_trans_reserve(
if (tp->t_ticket != NULL) {
ASSERT(resp->tr_logflags & XFS_TRANS_PERM_LOG_RES);
- error = xfs_log_regrant(tp->t_mountp, tp->t_ticket);
+ error = xfs_log_regrant(mp, tp->t_ticket);
} else {
- error = xfs_log_reserve(tp->t_mountp,
+ error = xfs_log_reserve(mp,
resp->tr_logres,
resp->tr_logcount,
&tp->t_ticket, XFS_TRANSACTION,
@@ -213,7 +215,7 @@ xfs_trans_reserve(
* fail if the count would go below zero.
*/
if (rtextents > 0) {
- error = xfs_mod_frextents(tp->t_mountp, -((int64_t)rtextents));
+ error = xfs_mod_frextents(mp, -((int64_t)rtextents));
if (error) {
error = -ENOSPC;
goto undo_log;
@@ -229,7 +231,7 @@ xfs_trans_reserve(
*/
undo_log:
if (resp->tr_logres > 0) {
- xfs_log_done(tp->t_mountp, tp->t_ticket, NULL, false);
+ xlog_ticket_done(mp->m_log, tp->t_ticket, false);
tp->t_ticket = NULL;
tp->t_log_res = 0;
tp->t_flags &= ~XFS_TRANS_PERM_LOG_RES;
@@ -237,7 +239,7 @@ xfs_trans_reserve(
undo_blocks:
if (blocks > 0) {
- xfs_mod_fdblocks(tp->t_mountp, (int64_t)blocks, rsvd);
+ xfs_mod_fdblocks(mp, (int64_t)blocks, rsvd);
tp->t_blk_res = 0;
}
@@ -999,9 +1001,7 @@ __xfs_trans_commit(
*/
xfs_trans_unreserve_and_mod_dquots(tp);
if (tp->t_ticket) {
- commit_lsn = xfs_log_done(mp, tp->t_ticket, NULL, regrant);
- if (commit_lsn == -1 && !error)
- error = -EIO;
+ xlog_ticket_done(mp->m_log, tp->t_ticket, regrant);
tp->t_ticket = NULL;
}
current_restore_flags_nested(&tp->t_pflags, PF_MEMALLOC_NOFS);
@@ -1060,7 +1060,7 @@ xfs_trans_cancel(
xfs_trans_unreserve_and_mod_dquots(tp);
if (tp->t_ticket) {
- xfs_log_done(mp, tp->t_ticket, NULL, false);
+ xlog_ticket_done(mp->m_log, tp->t_ticket, false);
tp->t_ticket = NULL;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 4/9] xfs: split xlog_ticket_done
2020-03-23 13:06 xfs: clean up log tickets and record writes v2 Christoph Hellwig
` (2 preceding siblings ...)
2020-03-23 13:07 ` [PATCH 3/9] xfs: refactor and split xfs_log_done() Christoph Hellwig
@ 2020-03-23 13:07 ` Christoph Hellwig
2020-03-24 0:09 ` Dave Chinner
2020-03-23 13:07 ` [PATCH 5/9] xfs: merge xlog_commit_record with xlog_write_done() Christoph Hellwig
` (5 subsequent siblings)
9 siblings, 1 reply; 13+ messages in thread
From: Christoph Hellwig @ 2020-03-23 13:07 UTC (permalink / raw)
To: linux-xfs; +Cc: david
Split the regrant case out of xlog_ticket_done and into a new
xlog_ticket_regrant helper. Merge both functions with the low-level
functions implementing the actual functionality and adjust the
tracepoints.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_log.c | 84 ++++++++++++++-----------------------------
fs/xfs/xfs_log_cil.c | 9 +++--
fs/xfs/xfs_log_priv.h | 4 +--
fs/xfs/xfs_trace.h | 14 ++++----
fs/xfs/xfs_trans.c | 9 +++--
5 files changed, 47 insertions(+), 73 deletions(-)
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 4b24fbf8b06c..c4ee4d95c658 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -66,14 +66,6 @@ xlog_grant_push_ail(
struct xlog *log,
int need_bytes);
STATIC void
-xlog_regrant_reserve_log_space(
- struct xlog *log,
- struct xlog_ticket *ticket);
-STATIC void
-xlog_ungrant_log_space(
- struct xlog *log,
- struct xlog_ticket *ticket);
-STATIC void
xlog_sync(
struct xlog *log,
struct xlog_in_core *iclog);
@@ -502,27 +494,6 @@ xlog_write_done(
return xlog_commit_record(log, ticket, iclog, lsn);
}
-/*
- * Release or regrant the ticket reservation now the transaction is done with
- * it depending on caller context. Rolling transactions need the ticket
- * regranted, otherwise we release it completely.
- */
-void
-xlog_ticket_done(
- struct xlog *log,
- struct xlog_ticket *ticket,
- bool regrant)
-{
- if (!regrant) {
- trace_xfs_log_done_nonperm(log, ticket);
- xlog_ungrant_log_space(log, ticket);
- } else {
- trace_xfs_log_done_perm(log, ticket);
- xlog_regrant_reserve_log_space(log, ticket);
- }
- xfs_log_ticket_put(ticket);
-}
-
static bool
__xlog_state_release_iclog(
struct xlog *log,
@@ -921,8 +892,7 @@ xfs_log_write_unmount_record(
if (tic) {
trace_xfs_log_umount_write(log, tic);
- xlog_ungrant_log_space(log, tic);
- xfs_log_ticket_put(tic);
+ xlog_ticket_done(log, tic);
}
}
@@ -2987,19 +2957,18 @@ xlog_state_get_iclog_space(
return 0;
} /* xlog_state_get_iclog_space */
-/* The first cnt-1 times through here we don't need to
- * move the grant write head because the permanent
- * reservation has reserved cnt times the unit amount.
- * Release part of current permanent unit reservation and
- * reset current reservation to be one units worth. Also
- * move grant reservation head forward.
+/*
+ * The first cnt-1 times through here we don't need to move the grant write head
+ * because the permanent reservation has reserved cnt times the unit amount.
+ * Release part of current permanent unit reservation and reset current
+ * reservation to be one units worth. Also move grant reservation head forward.
*/
-STATIC void
-xlog_regrant_reserve_log_space(
+void
+xlog_ticket_regrant(
struct xlog *log,
struct xlog_ticket *ticket)
{
- trace_xfs_log_regrant_reserve_enter(log, ticket);
+ trace_xfs_log_ticket_regrant(log, ticket);
if (ticket->t_cnt > 0)
ticket->t_cnt--;
@@ -3011,21 +2980,20 @@ xlog_regrant_reserve_log_space(
ticket->t_curr_res = ticket->t_unit_res;
xlog_tic_reset_res(ticket);
- trace_xfs_log_regrant_reserve_sub(log, ticket);
+ trace_xfs_log_ticket_regrant_sub(log, ticket);
/* just return if we still have some of the pre-reserved space */
- if (ticket->t_cnt > 0)
- return;
+ if (!ticket->t_cnt) {
+ xlog_grant_add_space(log, &log->l_reserve_head.grant,
+ ticket->t_unit_res);
+ trace_xfs_log_ticket_regrant_exit(log, ticket);
- xlog_grant_add_space(log, &log->l_reserve_head.grant,
- ticket->t_unit_res);
-
- trace_xfs_log_regrant_reserve_exit(log, ticket);
-
- ticket->t_curr_res = ticket->t_unit_res;
- xlog_tic_reset_res(ticket);
-} /* xlog_regrant_reserve_log_space */
+ ticket->t_curr_res = ticket->t_unit_res;
+ xlog_tic_reset_res(ticket);
+ }
+ xfs_log_ticket_put(ticket);
+}
/*
* Give back the space left from a reservation.
@@ -3041,18 +3009,19 @@ xlog_regrant_reserve_log_space(
* space, the count will stay at zero and the only space remaining will be
* in the current reservation field.
*/
-STATIC void
-xlog_ungrant_log_space(
+void
+xlog_ticket_done(
struct xlog *log,
struct xlog_ticket *ticket)
{
- int bytes;
+ int bytes;
+
+ trace_xfs_log_ticket_done(log, ticket);
if (ticket->t_cnt > 0)
ticket->t_cnt--;
- trace_xfs_log_ungrant_enter(log, ticket);
- trace_xfs_log_ungrant_sub(log, ticket);
+ trace_xfs_log_ticket_done_sub(log, ticket);
/*
* If this is a permanent reservation ticket, we may be able to free
@@ -3067,9 +3036,10 @@ xlog_ungrant_log_space(
xlog_grant_sub_space(log, &log->l_reserve_head.grant, bytes);
xlog_grant_sub_space(log, &log->l_write_head.grant, bytes);
- trace_xfs_log_ungrant_exit(log, ticket);
+ trace_xfs_log_ticket_done_exit(log, ticket);
xfs_log_space_wake(log->l_mp);
+ xfs_log_ticket_put(ticket);
}
/*
diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
index 880de1aa4288..27de462d2ba4 100644
--- a/fs/xfs/xfs_log_cil.c
+++ b/fs/xfs/xfs_log_cil.c
@@ -843,7 +843,7 @@ xlog_cil_push_work(
if (error)
goto out_abort_free_ticket;
- xlog_ticket_done(log, tic, false);
+ xlog_ticket_done(log, tic);
spin_lock(&commit_iclog->ic_callback_lock);
if (commit_iclog->ic_state == XLOG_STATE_IOERROR) {
@@ -876,7 +876,7 @@ xlog_cil_push_work(
return;
out_abort_free_ticket:
- xlog_ticket_done(log, tic, false);
+ xlog_ticket_done(log, tic);
out_abort:
ASSERT(XLOG_FORCED_SHUTDOWN(log));
xlog_cil_committed(ctx);
@@ -1008,7 +1008,10 @@ xfs_log_commit_cil(
if (commit_lsn)
*commit_lsn = xc_commit_lsn;
- xlog_ticket_done(log, tp->t_ticket, regrant);
+ if (regrant)
+ xlog_ticket_regrant(log, tp->t_ticket);
+ else
+ xlog_ticket_done(log, tp->t_ticket);
tp->t_ticket = NULL;
xfs_trans_unreserve_and_mod_sb(tp);
diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
index 32bb6856e69d..d4f53664ae12 100644
--- a/fs/xfs/xfs_log_priv.h
+++ b/fs/xfs/xfs_log_priv.h
@@ -445,8 +445,8 @@ int xlog_write(struct xlog *log, struct xfs_log_vec *log_vector,
struct xlog_in_core **commit_iclog, uint flags);
int xlog_write_done(struct xlog *log, struct xlog_ticket *ticket,
struct xlog_in_core **iclog, xfs_lsn_t *lsn);
-void xlog_ticket_done(struct xlog *log, struct xlog_ticket *ticket,
- bool regrant);
+void xlog_ticket_done(struct xlog *log, struct xlog_ticket *ticket);
+void xlog_ticket_regrant(struct xlog *log, struct xlog_ticket *ticket);
/*
* When we crack an atomic LSN, we sample it first so that the value will not
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index efc7751550d9..fbfdd9cf160d 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -1001,8 +1001,6 @@ DECLARE_EVENT_CLASS(xfs_loggrant_class,
DEFINE_EVENT(xfs_loggrant_class, name, \
TP_PROTO(struct xlog *log, struct xlog_ticket *tic), \
TP_ARGS(log, tic))
-DEFINE_LOGGRANT_EVENT(xfs_log_done_nonperm);
-DEFINE_LOGGRANT_EVENT(xfs_log_done_perm);
DEFINE_LOGGRANT_EVENT(xfs_log_umount_write);
DEFINE_LOGGRANT_EVENT(xfs_log_grant_sleep);
DEFINE_LOGGRANT_EVENT(xfs_log_grant_wake);
@@ -1011,12 +1009,12 @@ DEFINE_LOGGRANT_EVENT(xfs_log_reserve);
DEFINE_LOGGRANT_EVENT(xfs_log_reserve_exit);
DEFINE_LOGGRANT_EVENT(xfs_log_regrant);
DEFINE_LOGGRANT_EVENT(xfs_log_regrant_exit);
-DEFINE_LOGGRANT_EVENT(xfs_log_regrant_reserve_enter);
-DEFINE_LOGGRANT_EVENT(xfs_log_regrant_reserve_exit);
-DEFINE_LOGGRANT_EVENT(xfs_log_regrant_reserve_sub);
-DEFINE_LOGGRANT_EVENT(xfs_log_ungrant_enter);
-DEFINE_LOGGRANT_EVENT(xfs_log_ungrant_exit);
-DEFINE_LOGGRANT_EVENT(xfs_log_ungrant_sub);
+DEFINE_LOGGRANT_EVENT(xfs_log_ticket_regrant);
+DEFINE_LOGGRANT_EVENT(xfs_log_ticket_regrant_exit);
+DEFINE_LOGGRANT_EVENT(xfs_log_ticket_regrant_sub);
+DEFINE_LOGGRANT_EVENT(xfs_log_ticket_done);
+DEFINE_LOGGRANT_EVENT(xfs_log_ticket_done_sub);
+DEFINE_LOGGRANT_EVENT(xfs_log_ticket_done_exit);
DECLARE_EVENT_CLASS(xfs_log_item_class,
TP_PROTO(struct xfs_log_item *lip),
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 123ecc8435f6..6542b541c47c 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -231,7 +231,7 @@ xfs_trans_reserve(
*/
undo_log:
if (resp->tr_logres > 0) {
- xlog_ticket_done(mp->m_log, tp->t_ticket, false);
+ xlog_ticket_done(mp->m_log, tp->t_ticket);
tp->t_ticket = NULL;
tp->t_log_res = 0;
tp->t_flags &= ~XFS_TRANS_PERM_LOG_RES;
@@ -1001,7 +1001,10 @@ __xfs_trans_commit(
*/
xfs_trans_unreserve_and_mod_dquots(tp);
if (tp->t_ticket) {
- xlog_ticket_done(mp->m_log, tp->t_ticket, regrant);
+ if (regrant)
+ xlog_ticket_regrant(mp->m_log, tp->t_ticket);
+ else
+ xlog_ticket_done(mp->m_log, tp->t_ticket);
tp->t_ticket = NULL;
}
current_restore_flags_nested(&tp->t_pflags, PF_MEMALLOC_NOFS);
@@ -1060,7 +1063,7 @@ xfs_trans_cancel(
xfs_trans_unreserve_and_mod_dquots(tp);
if (tp->t_ticket) {
- xlog_ticket_done(mp->m_log, tp->t_ticket, false);
+ xlog_ticket_done(mp->m_log, tp->t_ticket);
tp->t_ticket = NULL;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 4/9] xfs: split xlog_ticket_done
2020-03-23 13:07 ` [PATCH 4/9] xfs: split xlog_ticket_done Christoph Hellwig
@ 2020-03-24 0:09 ` Dave Chinner
0 siblings, 0 replies; 13+ messages in thread
From: Dave Chinner @ 2020-03-24 0:09 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Mar 23, 2020 at 02:07:01PM +0100, Christoph Hellwig wrote:
> Split the regrant case out of xlog_ticket_done and into a new
> xlog_ticket_regrant helper. Merge both functions with the low-level
> functions implementing the actual functionality and adjust the
> tracepoints.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/xfs_log.c | 84 ++++++++++++++-----------------------------
> fs/xfs/xfs_log_cil.c | 9 +++--
> fs/xfs/xfs_log_priv.h | 4 +--
> fs/xfs/xfs_trace.h | 14 ++++----
> fs/xfs/xfs_trans.c | 9 +++--
> 5 files changed, 47 insertions(+), 73 deletions(-)
Nice. That unwinds it quite well.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 5/9] xfs: merge xlog_commit_record with xlog_write_done()
2020-03-23 13:06 xfs: clean up log tickets and record writes v2 Christoph Hellwig
` (3 preceding siblings ...)
2020-03-23 13:07 ` [PATCH 4/9] xfs: split xlog_ticket_done Christoph Hellwig
@ 2020-03-23 13:07 ` Christoph Hellwig
2020-03-23 13:07 ` [PATCH 6/9] xfs: factor out unmount record writing Christoph Hellwig
` (4 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2020-03-23 13:07 UTC (permalink / raw)
To: linux-xfs; +Cc: david, Dave Chinner
From: Dave Chinner <dchinner@redhat.com>
xlog_write_done() is just a thin wrapper around
xlog_commit_record(), so they can be merged together easily. Convert
all the xlog_commit_record() callers to use xlog_write_done() and
merge the implementations.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_log.c | 44 +++++++++++---------------------------------
1 file changed, 11 insertions(+), 33 deletions(-)
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index c4ee4d95c658..e34aaa7d3da3 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -24,13 +24,6 @@
kmem_zone_t *xfs_log_ticket_zone;
/* Local miscellaneous function prototypes */
-STATIC int
-xlog_commit_record(
- struct xlog *log,
- struct xlog_ticket *ticket,
- struct xlog_in_core **iclog,
- xfs_lsn_t *commitlsnp);
-
STATIC struct xlog *
xlog_alloc_log(
struct xfs_mount *mp,
@@ -478,22 +471,6 @@ xfs_log_reserve(
* marked as with WANT_SYNC.
*/
-/*
- * Write a commit record to the log to close off a running log write.
- */
-int
-xlog_write_done(
- struct xlog *log,
- struct xlog_ticket *ticket,
- struct xlog_in_core **iclog,
- xfs_lsn_t *lsn)
-{
- if (XLOG_FORCED_SHUTDOWN(log))
- return -EIO;
-
- return xlog_commit_record(log, ticket, iclog, lsn);
-}
-
static bool
__xlog_state_release_iclog(
struct xlog *log,
@@ -1463,20 +1440,17 @@ xlog_alloc_log(
return ERR_PTR(error);
} /* xlog_alloc_log */
-
/*
* Write out the commit record of a transaction associated with the given
- * ticket. Return the lsn of the commit record.
+ * ticket to close off a running log write. Return the lsn of the commit record.
*/
-STATIC int
-xlog_commit_record(
+int
+xlog_write_done(
struct xlog *log,
struct xlog_ticket *ticket,
struct xlog_in_core **iclog,
- xfs_lsn_t *commitlsnp)
+ xfs_lsn_t *lsn)
{
- struct xfs_mount *mp = log->l_mp;
- int error;
struct xfs_log_iovec reg = {
.i_addr = NULL,
.i_len = 0,
@@ -1486,12 +1460,16 @@ xlog_commit_record(
.lv_niovecs = 1,
.lv_iovecp = ®,
};
+ int error;
ASSERT_ALWAYS(iclog);
- error = xlog_write(log, &vec, ticket, commitlsnp, iclog,
- XLOG_COMMIT_TRANS);
+
+ if (XLOG_FORCED_SHUTDOWN(log))
+ return -EIO;
+
+ error = xlog_write(log, &vec, ticket, lsn, iclog, XLOG_COMMIT_TRANS);
if (error)
- xfs_force_shutdown(mp, SHUTDOWN_LOG_IO_ERROR);
+ xfs_force_shutdown(log->l_mp, SHUTDOWN_LOG_IO_ERROR);
return error;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 6/9] xfs: factor out unmount record writing
2020-03-23 13:06 xfs: clean up log tickets and record writes v2 Christoph Hellwig
` (4 preceding siblings ...)
2020-03-23 13:07 ` [PATCH 5/9] xfs: merge xlog_commit_record with xlog_write_done() Christoph Hellwig
@ 2020-03-23 13:07 ` Christoph Hellwig
2020-03-23 13:07 ` [PATCH 7/9] xfs: rename the log unmount writing functions Christoph Hellwig
` (3 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2020-03-23 13:07 UTC (permalink / raw)
To: linux-xfs; +Cc: david, Dave Chinner
From: Dave Chinner <dchinner@redhat.com>
Separate out the unmount record writing from the rest of the
ticket and log state futzing necessary to make it work. This is
a no-op, just makes the code cleaner and places the unmount record
formatting and writing alongside the commit record formatting and
writing code.
We can also get rid of the ticket flag clearing before the
xlog_write() call because it no longer cares about the state of
XLOG_TIC_INITED.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_log.c | 59 ++++++++++++++++++++++++++++--------------------
1 file changed, 35 insertions(+), 24 deletions(-)
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index e34aaa7d3da3..5a23a84973bb 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -471,6 +471,38 @@ xfs_log_reserve(
* marked as with WANT_SYNC.
*/
+/*
+ * Write out an unmount record using the ticket provided. We have to account for
+ * the data space used in the unmount ticket as this write is not done from a
+ * transaction context that has already done the accounting for us.
+ */
+static int
+xlog_write_unmount(
+ struct xlog *log,
+ struct xlog_ticket *ticket,
+ xfs_lsn_t *lsn,
+ uint flags)
+{
+ /* the data section must be 32 bit size aligned */
+ struct xfs_unmount_log_format magic = {
+ .magic = XLOG_UNMOUNT_TYPE,
+ };
+ struct xfs_log_iovec reg = {
+ .i_addr = &magic,
+ .i_len = sizeof(magic),
+ .i_type = XLOG_REG_TYPE_UNMOUNT,
+ };
+ struct xfs_log_vec vec = {
+ .lv_niovecs = 1,
+ .lv_iovecp = ®,
+ };
+
+ /* account for space used by record data */
+ ticket->t_curr_res -= sizeof(magic);
+
+ return xlog_write(log, &vec, ticket, lsn, NULL, flags);
+}
+
static bool
__xlog_state_release_iclog(
struct xlog *log,
@@ -795,31 +827,13 @@ xlog_wait_on_iclog(
}
/*
- * Final log writes as part of unmount.
- *
- * Mark the filesystem clean as unmount happens. Note that during relocation
- * this routine needs to be executed as part of source-bag while the
- * deallocation must not be done until source-end.
+ * Mark the filesystem clean by writing an unmount record to the head of the
+ * log.
*/
-
-/* Actually write the unmount record to disk. */
static void
xfs_log_write_unmount_record(
struct xfs_mount *mp)
{
- /* the data section must be 32 bit size aligned */
- struct xfs_unmount_log_format magic = {
- .magic = XLOG_UNMOUNT_TYPE,
- };
- struct xfs_log_iovec reg = {
- .i_addr = &magic,
- .i_len = sizeof(magic),
- .i_type = XLOG_REG_TYPE_UNMOUNT,
- };
- struct xfs_log_vec vec = {
- .lv_niovecs = 1,
- .lv_iovecp = ®,
- };
struct xlog *log = mp->m_log;
struct xlog_in_core *iclog;
struct xlog_ticket *tic = NULL;
@@ -844,10 +858,7 @@ xfs_log_write_unmount_record(
flags &= ~XLOG_UNMOUNT_TRANS;
}
- /* 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_unmount(log, tic, &lsn, flags);
/*
* At this point, we're umounting anyway, so there's no point in
* transitioning log state to IOERROR. Just continue...
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 7/9] xfs: rename the log unmount writing functions.
2020-03-23 13:06 xfs: clean up log tickets and record writes v2 Christoph Hellwig
` (5 preceding siblings ...)
2020-03-23 13:07 ` [PATCH 6/9] xfs: factor out unmount record writing Christoph Hellwig
@ 2020-03-23 13:07 ` Christoph Hellwig
2020-03-23 13:07 ` [PATCH 8/9] xfs: remove some stale comments from the log code Christoph Hellwig
` (2 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2020-03-23 13:07 UTC (permalink / raw)
To: linux-xfs; +Cc: david, Dave Chinner
From: Dave Chinner <dchinner@redhat.com>
The naming and calling conventions are a bit of a mess. Clean it up
so the call chain looks like:
xfs_log_unmount_write(mp)
xlog_unmount_write(log)
xlog_write_unmount_record(log, ticket)
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_log.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 5a23a84973bb..d0d9f27ef90c 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -477,7 +477,7 @@ xfs_log_reserve(
* transaction context that has already done the accounting for us.
*/
static int
-xlog_write_unmount(
+xlog_write_unmount_record(
struct xlog *log,
struct xlog_ticket *ticket,
xfs_lsn_t *lsn,
@@ -831,10 +831,10 @@ xlog_wait_on_iclog(
* log.
*/
static void
-xfs_log_write_unmount_record(
- struct xfs_mount *mp)
+xlog_unmount_write(
+ struct xlog *log)
{
- struct xlog *log = mp->m_log;
+ struct xfs_mount *mp = log->l_mp;
struct xlog_in_core *iclog;
struct xlog_ticket *tic = NULL;
xfs_lsn_t lsn;
@@ -858,7 +858,7 @@ xfs_log_write_unmount_record(
flags &= ~XLOG_UNMOUNT_TRANS;
}
- error = xlog_write_unmount(log, tic, &lsn, flags);
+ error = xlog_write_unmount_record(log, tic, &lsn, flags);
/*
* At this point, we're umounting anyway, so there's no point in
* transitioning log state to IOERROR. Just continue...
@@ -924,7 +924,7 @@ xfs_log_unmount_write(
if (XLOG_FORCED_SHUTDOWN(log))
return;
xfs_log_unmount_verify_iclog(log);
- xfs_log_write_unmount_record(mp);
+ xlog_unmount_write(log);
}
/*
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 8/9] xfs: remove some stale comments from the log code
2020-03-23 13:06 xfs: clean up log tickets and record writes v2 Christoph Hellwig
` (6 preceding siblings ...)
2020-03-23 13:07 ` [PATCH 7/9] xfs: rename the log unmount writing functions Christoph Hellwig
@ 2020-03-23 13:07 ` Christoph Hellwig
2020-03-23 13:07 ` [PATCH 9/9] xfs: kill XLOG_TIC_INITED Christoph Hellwig
2020-03-24 11:43 ` xfs: clean up log tickets and record writes v2 Brian Foster
9 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2020-03-23 13:07 UTC (permalink / raw)
To: linux-xfs; +Cc: david, Dave Chinner
From: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_log.c | 59 +++++++++++-------------------------------------
1 file changed, 13 insertions(+), 46 deletions(-)
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index d0d9f27ef90c..3eff7038e3ed 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -463,14 +463,6 @@ xfs_log_reserve(
return error;
}
-
-/*
- * NOTES:
- *
- * 1. currblock field gets updated at startup and after in-core logs
- * marked as with WANT_SYNC.
- */
-
/*
* Write out an unmount record using the ticket provided. We have to account for
* the data space used in the unmount ticket as this write is not done from a
@@ -1912,7 +1904,7 @@ xlog_dealloc_log(
log->l_mp->m_log = NULL;
destroy_workqueue(log->l_ioend_workqueue);
kmem_free(log);
-} /* xlog_dealloc_log */
+}
/*
* Update counters atomically now that memcpy is done.
@@ -2456,14 +2448,6 @@ xlog_write(
return error;
}
-
-/*****************************************************************************
- *
- * State Machine functions
- *
- *****************************************************************************
- */
-
static void
xlog_state_activate_iclog(
struct xlog_in_core *iclog,
@@ -2824,7 +2808,7 @@ xlog_state_done_syncing(
*/
wake_up_all(&iclog->ic_write_wait);
spin_unlock(&log->l_icloglock);
- xlog_state_do_callback(log); /* also cleans log */
+ xlog_state_do_callback(log);
}
/*
@@ -2944,13 +2928,14 @@ xlog_state_get_iclog_space(
*logoffsetp = log_offset;
return 0;
-} /* xlog_state_get_iclog_space */
+}
/*
- * The first cnt-1 times through here we don't need to move the grant write head
- * because the permanent reservation has reserved cnt times the unit amount.
- * Release part of current permanent unit reservation and reset current
- * reservation to be one units worth. Also move grant reservation head forward.
+ * The first cnt-1 times a ticket goes through here we don't need to move the
+ * grant write head because the permanent reservation has reserved cnt times the
+ * unit amount. Release part of current permanent unit reservation and reset
+ * current reservation to be one units worth. Also move grant reservation head
+ * forward.
*/
void
xlog_ticket_regrant(
@@ -3032,12 +3017,8 @@ xlog_ticket_done(
}
/*
- * Mark the current iclog in the ring as WANT_SYNC and move the current iclog
- * pointer to the next iclog in the ring.
- *
- * When called from xlog_state_get_iclog_space(), the exact size of the iclog
- * has not yet been determined, all we know is that we have run out of space in
- * the current iclog.
+ * This routine will mark the current iclog in the ring as WANT_SYNC and move
+ * the current iclog pointer to the next iclog in the ring.
*/
STATIC void
xlog_state_switch_iclogs(
@@ -3082,7 +3063,7 @@ xlog_state_switch_iclogs(
}
ASSERT(iclog == log->l_iclog);
log->l_iclog = iclog->ic_next;
-} /* xlog_state_switch_iclogs */
+}
/*
* Write out all data in the in-core log as of this exact moment in time.
@@ -3289,13 +3270,6 @@ xfs_log_force_lsn(
return ret;
}
-/*****************************************************************************
- *
- * TICKET functions
- *
- *****************************************************************************
- */
-
/*
* Free a used ticket when its refcount falls to zero.
*/
@@ -3453,13 +3427,6 @@ xlog_ticket_alloc(
return tic;
}
-
-/******************************************************************************
- *
- * Log debug routines
- *
- ******************************************************************************
- */
#if defined(DEBUG)
/*
* Make sure that the destination ptr is within the valid data region of
@@ -3545,7 +3512,7 @@ xlog_verify_tail_lsn(
if (blocks < BTOBB(iclog->ic_offset) + 1)
xfs_emerg(log->l_mp, "%s: ran out of log space", __func__);
}
-} /* xlog_verify_tail_lsn */
+}
/*
* Perform a number of checks on the iclog before writing to disk.
@@ -3648,7 +3615,7 @@ xlog_verify_iclog(
}
ptr += sizeof(xlog_op_header_t) + op_len;
}
-} /* xlog_verify_iclog */
+}
#endif
/*
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 9/9] xfs: kill XLOG_TIC_INITED
2020-03-23 13:06 xfs: clean up log tickets and record writes v2 Christoph Hellwig
` (7 preceding siblings ...)
2020-03-23 13:07 ` [PATCH 8/9] xfs: remove some stale comments from the log code Christoph Hellwig
@ 2020-03-23 13:07 ` Christoph Hellwig
2020-03-24 11:43 ` xfs: clean up log tickets and record writes v2 Brian Foster
9 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2020-03-23 13:07 UTC (permalink / raw)
To: linux-xfs; +Cc: david, Dave Chinner
From: Dave Chinner <dchinner@redhat.com>
It is not longer used or checked by anything, so remove the last
traces from the log ticket code.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_log.c | 1 -
fs/xfs/xfs_log_priv.h | 6 ++----
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 3eff7038e3ed..f70be8151a59 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -3418,7 +3418,6 @@ xlog_ticket_alloc(
tic->t_ocnt = cnt;
tic->t_tid = prandom_u32();
tic->t_clientid = client;
- tic->t_flags = XLOG_TIC_INITED;
if (permanent)
tic->t_flags |= XLOG_TIC_PERM_RESERV;
diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
index d4f53664ae12..cfcf3f02e30a 100644
--- a/fs/xfs/xfs_log_priv.h
+++ b/fs/xfs/xfs_log_priv.h
@@ -51,13 +51,11 @@ enum xlog_iclog_state {
};
/*
- * Flags to log ticket
+ * Log ticket flags
*/
-#define XLOG_TIC_INITED 0x1 /* has been initialized */
-#define XLOG_TIC_PERM_RESERV 0x2 /* permanent reservation */
+#define XLOG_TIC_PERM_RESERV 0x1 /* permanent reservation */
#define XLOG_TIC_FLAGS \
- { XLOG_TIC_INITED, "XLOG_TIC_INITED" }, \
{ XLOG_TIC_PERM_RESERV, "XLOG_TIC_PERM_RESERV" }
/*
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: xfs: clean up log tickets and record writes v2
2020-03-23 13:06 xfs: clean up log tickets and record writes v2 Christoph Hellwig
` (8 preceding siblings ...)
2020-03-23 13:07 ` [PATCH 9/9] xfs: kill XLOG_TIC_INITED Christoph Hellwig
@ 2020-03-24 11:43 ` Brian Foster
2020-03-24 12:40 ` Christoph Hellwig
9 siblings, 1 reply; 13+ messages in thread
From: Brian Foster @ 2020-03-24 11:43 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, david
On Mon, Mar 23, 2020 at 02:06:57PM +0100, Christoph Hellwig wrote:
> This series follows up on conversions about relogging infrastructure
> and the way xfs_log_done() does two things but only one of several
> callers uses both of those functions. It also pointed out that
> xfs_trans_commit() never writes to the log anymore, so only
> checkpoints pass a ticket to xlog_write() with this flag set and
> no transaction makes multiple calls to xlog_write() calls on the
> same ticket. Hence there's no real need for XLOG_TIC_INITED to track
> whether a ticket has written a start record to the log anymore.
>
> A lot of further cleanups fell out of this. Once we no longer use
> XLOG_TIC_INITED to carry state inside the write loop, the logic
> can be simplified in both xlog_write and xfs_log_done. xfs_log_done
> can be split up, and then the call chain can be flattened because
> xlog_write_done() and xlog_commit_record() are basically the same.
>
> This then leads to cleanups writing both commit and unmount records.
>
> Finally, to complete what started all this, the XLOG_TIC_INITED flag
> is removed.
>
> A git tree is avaiblable here:
>
> git://git.infradead.org/users/hch/xfs.git xlog-ticket-cleanup.2
>
> Gitweb:
>
> http://git.infradead.org/users/hch/xfs.git/shortlog/refs/heads/xlog-ticket-cleanup.2
>
>
> Changes since v1:
> - taking this over from Dave (for now) as he is still injured, an it
> interacts closely with my log error handling bits
> - rebased on top of for-next + the "more log cleanups" series
> - fix an accounting error in xlog_write
> - use a bool for the ticket header in xlog_write
> - add a new patch to split xlog_ticket_done
>
This seems to ignore various bits of (trivial) feedback from v1 as well
as drops all reviews...
Brian
^ permalink raw reply [flat|nested] 13+ messages in thread