From 8e123850863366b738d6dfb9a84045018ff038fc Mon Sep 17 00:00:00 2001 From: Dave Chinner Date: Mon, 8 Mar 2010 11:26:03 +1100 Subject: xfs: remove stale parameter from ->iop_unpin method The staleness of a object being unpinned can be directly derived from the object itself - there is no need to extract it from the object then pass it as a parameter into IOP_UNPIN(). This means we can kill the XFS_LID_BUF_STALE flag - it is set, checked and cleared in the same places XFS_BLI_STALE flag in the xfs_buf_log_item so it is now redundant and hence safe to remove. Signed-off-by: Dave Chinner Reviewed-by: Christoph Hellwig --- fs/xfs/xfs_trans.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/xfs/xfs_trans.c') diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c index f73e358bae8d..6962f2bd3da3 100644 --- a/fs/xfs/xfs_trans.c +++ b/fs/xfs/xfs_trans.c @@ -1451,6 +1451,6 @@ xfs_trans_chunk_committed( * flags, if anyone else stales the buffer we do not * want to pay any attention to it. */ - IOP_UNPIN(lip, lidp->lid_flags & XFS_LID_BUF_STALE); + IOP_UNPIN(lip); } } -- cgit v1.2.2 From 0924378a689ccb05f6d60875742dc28f69bf0129 Mon Sep 17 00:00:00 2001 From: Dave Chinner Date: Mon, 8 Mar 2010 11:28:28 +1100 Subject: xfs: split out iclog writing from xfs_trans_commit() Split the the part of xfs_trans_commit() that deals with writing the transaction into the iclog into a separate function. This isolates the physical commit process from the logical commit operation and makes it easier to insert different transaction commit paths without affecting the existing algorithm adversely. Signed-off-by: Dave Chinner Reviewed-by: Christoph Hellwig --- fs/xfs/xfs_trans.c | 387 +++++++++++++++++++++++++++-------------------------- 1 file changed, 199 insertions(+), 188 deletions(-) (limited to 'fs/xfs/xfs_trans.c') diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c index 6962f2bd3da3..e07b3290b3bf 100644 --- a/fs/xfs/xfs_trans.c +++ b/fs/xfs/xfs_trans.c @@ -47,8 +47,6 @@ STATIC void xfs_trans_apply_sb_deltas(xfs_trans_t *); -STATIC uint xfs_trans_count_vecs(xfs_trans_t *); -STATIC void xfs_trans_fill_vecs(xfs_trans_t *, xfs_log_iovec_t *); STATIC void xfs_trans_uncommit(xfs_trans_t *, uint); STATIC void xfs_trans_committed(xfs_trans_t *, int); STATIC void xfs_trans_chunk_committed(xfs_log_item_chunk_t *, xfs_lsn_t, int); @@ -764,94 +762,126 @@ xfs_trans_unreserve_and_mod_sb( } } - /* - * xfs_trans_commit - * - * Commit the given transaction to the log a/synchronously. - * - * XFS disk error handling mechanism is not based on a typical - * transaction abort mechanism. Logically after the filesystem - * gets marked 'SHUTDOWN', we can't let any new transactions - * be durable - ie. committed to disk - because some metadata might - * be inconsistent. In such cases, this returns an error, and the - * caller may assume that all locked objects joined to the transaction - * have already been unlocked as if the commit had succeeded. - * Do not reference the transaction structure after this call. + * Total up the number of log iovecs needed to commit this + * transaction. The transaction itself needs one for the + * transaction header. Ask each dirty item in turn how many + * it needs to get the total. */ - /*ARGSUSED*/ -int -_xfs_trans_commit( - xfs_trans_t *tp, - uint flags, - int *log_flushed) +static uint +xfs_trans_count_vecs( + xfs_trans_t *tp) { - xfs_log_iovec_t *log_vector; - int nvec; - xfs_mount_t *mp; - xfs_lsn_t commit_lsn; - /* REFERENCED */ - int error; - int log_flags; - int sync; -#define XFS_TRANS_LOGVEC_COUNT 16 - xfs_log_iovec_t log_vector_fast[XFS_TRANS_LOGVEC_COUNT]; - struct xlog_in_core *commit_iclog; - int shutdown; + int nvecs; + xfs_log_item_desc_t *lidp; - commit_lsn = -1; + nvecs = 1; + lidp = xfs_trans_first_item(tp); + ASSERT(lidp != NULL); - /* - * Determine whether this commit is releasing a permanent - * log reservation or not. + /* In the non-debug case we need to start bailing out if we + * didn't find a log_item here, return zero and let trans_commit + * deal with it. */ - if (flags & XFS_TRANS_RELEASE_LOG_RES) { - ASSERT(tp->t_flags & XFS_TRANS_PERM_LOG_RES); - log_flags = XFS_LOG_REL_PERM_RESERV; - } else { - log_flags = 0; + if (lidp == NULL) + return 0; + + while (lidp != NULL) { + /* + * Skip items which aren't dirty in this transaction. + */ + if (!(lidp->lid_flags & XFS_LID_DIRTY)) { + lidp = xfs_trans_next_item(tp, lidp); + continue; + } + lidp->lid_size = IOP_SIZE(lidp->lid_item); + nvecs += lidp->lid_size; + lidp = xfs_trans_next_item(tp, lidp); } - mp = tp->t_mountp; + + return nvecs; +} + +/* + * Fill in the vector with pointers to data to be logged + * by this transaction. The transaction header takes + * the first vector, and then each dirty item takes the + * number of vectors it indicated it needed in xfs_trans_count_vecs(). + * + * As each item fills in the entries it needs, also pin the item + * so that it cannot be flushed out until the log write completes. + */ +static void +xfs_trans_fill_vecs( + struct xfs_trans *tp, + struct xfs_log_iovec *log_vector) +{ + xfs_log_item_desc_t *lidp; + struct xfs_log_iovec *vecp; + uint nitems; /* - * If there is nothing to be logged by the transaction, - * then unlock all of the items associated with the - * transaction and free the transaction structure. - * Also make sure to return any reserved blocks to - * the free pool. + * Skip over the entry for the transaction header, we'll + * fill that in at the end. */ -shut_us_down: - shutdown = XFS_FORCED_SHUTDOWN(mp) ? EIO : 0; - if (!(tp->t_flags & XFS_TRANS_DIRTY) || shutdown) { - xfs_trans_unreserve_and_mod_sb(tp); + vecp = log_vector + 1; + + nitems = 0; + lidp = xfs_trans_first_item(tp); + ASSERT(lidp); + while (lidp) { + /* Skip items which aren't dirty in this transaction. */ + if (!(lidp->lid_flags & XFS_LID_DIRTY)) { + lidp = xfs_trans_next_item(tp, lidp); + continue; + } + /* - * It is indeed possible for the transaction to be - * not dirty but the dqinfo portion to be. All that - * means is that we have some (non-persistent) quota - * reservations that need to be unreserved. + * The item may be marked dirty but not log anything. This can + * be used to get called when a transaction is committed. */ - xfs_trans_unreserve_and_mod_dquots(tp); - if (tp->t_ticket) { - commit_lsn = xfs_log_done(mp, tp->t_ticket, - NULL, log_flags); - if (commit_lsn == -1 && !shutdown) - shutdown = XFS_ERROR(EIO); - } - current_restore_flags_nested(&tp->t_pflags, PF_FSTRANS); - xfs_trans_free_items(tp, shutdown? XFS_TRANS_ABORT : 0); - xfs_trans_free_busy(tp); - xfs_trans_free(tp); - XFS_STATS_INC(xs_trans_empty); - return (shutdown); + if (lidp->lid_size) + nitems++; + IOP_FORMAT(lidp->lid_item, vecp); + vecp += lidp->lid_size; + IOP_PIN(lidp->lid_item); + lidp = xfs_trans_next_item(tp, lidp); } - ASSERT(tp->t_ticket != NULL); /* - * If we need to update the superblock, then do it now. + * Now that we've counted the number of items in this transaction, fill + * in the transaction header. Note that the transaction header does not + * have a log item. */ - if (tp->t_flags & XFS_TRANS_SB_DIRTY) - xfs_trans_apply_sb_deltas(tp); - xfs_trans_apply_dquot_deltas(tp); + tp->t_header.th_magic = XFS_TRANS_HEADER_MAGIC; + tp->t_header.th_type = tp->t_type; + tp->t_header.th_num_items = nitems; + log_vector->i_addr = (xfs_caddr_t)&tp->t_header; + log_vector->i_len = sizeof(xfs_trans_header_t); + log_vector->i_type = XLOG_REG_TYPE_TRANSHDR; +} + +/* + * Format the transaction direct to the iclog. This isolates the physical + * transaction commit operation from the logical operation and hence allows + * other methods to be introduced without affecting the existing commit path. + */ +static int +xfs_trans_commit_iclog( + struct xfs_mount *mp, + struct xfs_trans *tp, + xfs_lsn_t *commit_lsn, + int flags) +{ + int shutdown; + int error; + int log_flags = 0; + struct xlog_in_core *commit_iclog; +#define XFS_TRANS_LOGVEC_COUNT 16 + struct xfs_log_iovec log_vector_fast[XFS_TRANS_LOGVEC_COUNT]; + struct xfs_log_iovec *log_vector; + uint nvec; + /* * Ask each log item how many log_vector entries it will @@ -861,8 +891,7 @@ shut_us_down: */ nvec = xfs_trans_count_vecs(tp); if (nvec == 0) { - xfs_force_shutdown(mp, SHUTDOWN_LOG_IO_ERROR); - goto shut_us_down; + return ENOMEM; /* triggers a shutdown! */ } else if (nvec <= XFS_TRANS_LOGVEC_COUNT) { log_vector = log_vector_fast; } else { @@ -877,6 +906,9 @@ shut_us_down: */ xfs_trans_fill_vecs(tp, log_vector); + if (flags & XFS_TRANS_RELEASE_LOG_RES) + log_flags = XFS_LOG_REL_PERM_RESERV; + error = xfs_log_write(mp, log_vector, nvec, tp->t_ticket, &(tp->t_lsn)); /* @@ -884,18 +916,17 @@ shut_us_down: * at any time after this call. However, all the items associated * with the transaction are still locked and pinned in memory. */ - commit_lsn = xfs_log_done(mp, tp->t_ticket, &commit_iclog, log_flags); + *commit_lsn = xfs_log_done(mp, tp->t_ticket, &commit_iclog, log_flags); - tp->t_commit_lsn = commit_lsn; - if (nvec > XFS_TRANS_LOGVEC_COUNT) { + tp->t_commit_lsn = *commit_lsn; + if (nvec > XFS_TRANS_LOGVEC_COUNT) kmem_free(log_vector); - } /* * If we got a log write error. Unpin the logitems that we * had pinned, clean up, free trans structure, and return error. */ - if (error || commit_lsn == -1) { + if (error || *commit_lsn == -1) { current_restore_flags_nested(&tp->t_pflags, PF_FSTRANS); xfs_trans_uncommit(tp, flags|XFS_TRANS_ABORT); return XFS_ERROR(EIO); @@ -909,8 +940,6 @@ shut_us_down: */ xfs_trans_unreserve_and_mod_sb(tp); - sync = tp->t_flags & XFS_TRANS_SYNC; - /* * Tell the LM to call the transaction completion routine * when the log write with LSN commit_lsn completes (e.g. @@ -953,7 +982,7 @@ shut_us_down: * the commit lsn of this transaction for dependency tracking * purposes. */ - xfs_trans_unlock_items(tp, commit_lsn); + xfs_trans_unlock_items(tp, *commit_lsn); /* * If we detected a log error earlier, finish committing @@ -973,7 +1002,92 @@ shut_us_down: * and the items are released we can finally allow the iclog to * go to disk. */ - error = xfs_log_release_iclog(mp, commit_iclog); + return xfs_log_release_iclog(mp, commit_iclog); +} + + +/* + * xfs_trans_commit + * + * Commit the given transaction to the log a/synchronously. + * + * XFS disk error handling mechanism is not based on a typical + * transaction abort mechanism. Logically after the filesystem + * gets marked 'SHUTDOWN', we can't let any new transactions + * be durable - ie. committed to disk - because some metadata might + * be inconsistent. In such cases, this returns an error, and the + * caller may assume that all locked objects joined to the transaction + * have already been unlocked as if the commit had succeeded. + * Do not reference the transaction structure after this call. + */ + /*ARGSUSED*/ +int +_xfs_trans_commit( + xfs_trans_t *tp, + uint flags, + int *log_flushed) +{ + xfs_mount_t *mp = tp->t_mountp; + xfs_lsn_t commit_lsn = -1; + int error; + int log_flags = 0; + int sync = tp->t_flags & XFS_TRANS_SYNC; + int shutdown; + + /* + * Determine whether this commit is releasing a permanent + * log reservation or not. + */ + if (flags & XFS_TRANS_RELEASE_LOG_RES) { + ASSERT(tp->t_flags & XFS_TRANS_PERM_LOG_RES); + log_flags = XFS_LOG_REL_PERM_RESERV; + } + + /* + * If there is nothing to be logged by the transaction, + * then unlock all of the items associated with the + * transaction and free the transaction structure. + * Also make sure to return any reserved blocks to + * the free pool. + */ +shut_us_down: + shutdown = XFS_FORCED_SHUTDOWN(mp) ? EIO : 0; + if (!(tp->t_flags & XFS_TRANS_DIRTY) || shutdown) { + xfs_trans_unreserve_and_mod_sb(tp); + /* + * It is indeed possible for the transaction to be + * not dirty but the dqinfo portion to be. All that + * means is that we have some (non-persistent) quota + * reservations that need to be unreserved. + */ + xfs_trans_unreserve_and_mod_dquots(tp); + if (tp->t_ticket) { + commit_lsn = xfs_log_done(mp, tp->t_ticket, + NULL, log_flags); + if (commit_lsn == -1 && !shutdown) + shutdown = XFS_ERROR(EIO); + } + current_restore_flags_nested(&tp->t_pflags, PF_FSTRANS); + xfs_trans_free_items(tp, shutdown? XFS_TRANS_ABORT : 0); + xfs_trans_free_busy(tp); + xfs_trans_free(tp); + XFS_STATS_INC(xs_trans_empty); + return (shutdown); + } + ASSERT(tp->t_ticket != NULL); + + /* + * If we need to update the superblock, then do it now. + */ + if (tp->t_flags & XFS_TRANS_SB_DIRTY) + xfs_trans_apply_sb_deltas(tp); + xfs_trans_apply_dquot_deltas(tp); + + error = xfs_trans_commit_iclog(mp, tp, &commit_lsn, flags); + if (error == ENOMEM) { + xfs_force_shutdown(mp, SHUTDOWN_LOG_IO_ERROR); + goto shut_us_down; + } /* * If the transaction needs to be synchronous, then force the @@ -992,47 +1106,6 @@ shut_us_down: return (error); } - -/* - * Total up the number of log iovecs needed to commit this - * transaction. The transaction itself needs one for the - * transaction header. Ask each dirty item in turn how many - * it needs to get the total. - */ -STATIC uint -xfs_trans_count_vecs( - xfs_trans_t *tp) -{ - int nvecs; - xfs_log_item_desc_t *lidp; - - nvecs = 1; - lidp = xfs_trans_first_item(tp); - ASSERT(lidp != NULL); - - /* In the non-debug case we need to start bailing out if we - * didn't find a log_item here, return zero and let trans_commit - * deal with it. - */ - if (lidp == NULL) - return 0; - - while (lidp != NULL) { - /* - * Skip items which aren't dirty in this transaction. - */ - if (!(lidp->lid_flags & XFS_LID_DIRTY)) { - lidp = xfs_trans_next_item(tp, lidp); - continue; - } - lidp->lid_size = IOP_SIZE(lidp->lid_item); - nvecs += lidp->lid_size; - lidp = xfs_trans_next_item(tp, lidp); - } - - return nvecs; -} - /* * Called from the trans_commit code when we notice that * the filesystem is in the middle of a forced shutdown. @@ -1062,68 +1135,6 @@ xfs_trans_uncommit( xfs_trans_free(tp); } -/* - * Fill in the vector with pointers to data to be logged - * by this transaction. The transaction header takes - * the first vector, and then each dirty item takes the - * number of vectors it indicated it needed in xfs_trans_count_vecs(). - * - * As each item fills in the entries it needs, also pin the item - * so that it cannot be flushed out until the log write completes. - */ -STATIC void -xfs_trans_fill_vecs( - xfs_trans_t *tp, - xfs_log_iovec_t *log_vector) -{ - xfs_log_item_desc_t *lidp; - xfs_log_iovec_t *vecp; - uint nitems; - - /* - * Skip over the entry for the transaction header, we'll - * fill that in at the end. - */ - vecp = log_vector + 1; /* pointer arithmetic */ - - nitems = 0; - lidp = xfs_trans_first_item(tp); - ASSERT(lidp != NULL); - while (lidp != NULL) { - /* - * Skip items which aren't dirty in this transaction. - */ - if (!(lidp->lid_flags & XFS_LID_DIRTY)) { - lidp = xfs_trans_next_item(tp, lidp); - continue; - } - /* - * The item may be marked dirty but not log anything. - * This can be used to get called when a transaction - * is committed. - */ - if (lidp->lid_size) { - nitems++; - } - IOP_FORMAT(lidp->lid_item, vecp); - vecp += lidp->lid_size; /* pointer arithmetic */ - IOP_PIN(lidp->lid_item); - lidp = xfs_trans_next_item(tp, lidp); - } - - /* - * Now that we've counted the number of items in this - * transaction, fill in the transaction header. - */ - tp->t_header.th_magic = XFS_TRANS_HEADER_MAGIC; - tp->t_header.th_type = tp->t_type; - tp->t_header.th_num_items = nitems; - log_vector->i_addr = (xfs_caddr_t)&tp->t_header; - log_vector->i_len = sizeof(xfs_trans_header_t); - log_vector->i_type = XLOG_REG_TYPE_TRANSHDR; -} - - /* * Unlock all of the transaction's items and free the transaction. * The transaction must not have modified any of its items, because -- cgit v1.2.2 From a3ccd2ca43d5cdfe0b256be02957dc5f47ec4c39 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Mon, 15 Mar 2010 12:52:49 +1100 Subject: xfs: clean up xfs_trans_commit logic even more > +shut_us_down: > + shutdown = XFS_FORCED_SHUTDOWN(mp) ? EIO : 0; > + if (!(tp->t_flags & XFS_TRANS_DIRTY) || shutdown) { > + xfs_trans_unreserve_and_mod_sb(tp); > + /* This whole area in _xfs_trans_commit is still a complete mess. So while touching this code, unravel this mess as well to make the whole flow of the function simpler and clearer. Signed-off-by: Christoph Hellwig Reviewed-by: Dave Chinner --- fs/xfs/xfs_trans.c | 69 +++++++++++++++++++++++++++++------------------------- 1 file changed, 37 insertions(+), 32 deletions(-) (limited to 'fs/xfs/xfs_trans.c') diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c index e07b3290b3bf..2bff22995127 100644 --- a/fs/xfs/xfs_trans.c +++ b/fs/xfs/xfs_trans.c @@ -1020,19 +1020,17 @@ xfs_trans_commit_iclog( * have already been unlocked as if the commit had succeeded. * Do not reference the transaction structure after this call. */ - /*ARGSUSED*/ int _xfs_trans_commit( - xfs_trans_t *tp, - uint flags, - int *log_flushed) + struct xfs_trans *tp, + uint flags, + int *log_flushed) { - xfs_mount_t *mp = tp->t_mountp; + struct xfs_mount *mp = tp->t_mountp; xfs_lsn_t commit_lsn = -1; - int error; + int error = 0; int log_flags = 0; int sync = tp->t_flags & XFS_TRANS_SYNC; - int shutdown; /* * Determine whether this commit is releasing a permanent @@ -1050,30 +1048,14 @@ _xfs_trans_commit( * Also make sure to return any reserved blocks to * the free pool. */ -shut_us_down: - shutdown = XFS_FORCED_SHUTDOWN(mp) ? EIO : 0; - if (!(tp->t_flags & XFS_TRANS_DIRTY) || shutdown) { - xfs_trans_unreserve_and_mod_sb(tp); - /* - * It is indeed possible for the transaction to be - * not dirty but the dqinfo portion to be. All that - * means is that we have some (non-persistent) quota - * reservations that need to be unreserved. - */ - xfs_trans_unreserve_and_mod_dquots(tp); - if (tp->t_ticket) { - commit_lsn = xfs_log_done(mp, tp->t_ticket, - NULL, log_flags); - if (commit_lsn == -1 && !shutdown) - shutdown = XFS_ERROR(EIO); - } - current_restore_flags_nested(&tp->t_pflags, PF_FSTRANS); - xfs_trans_free_items(tp, shutdown? XFS_TRANS_ABORT : 0); - xfs_trans_free_busy(tp); - xfs_trans_free(tp); - XFS_STATS_INC(xs_trans_empty); - return (shutdown); + if (!(tp->t_flags & XFS_TRANS_DIRTY)) + goto out_unreserve; + + if (XFS_FORCED_SHUTDOWN(mp)) { + error = XFS_ERROR(EIO); + goto out_unreserve; } + ASSERT(tp->t_ticket != NULL); /* @@ -1086,7 +1068,8 @@ shut_us_down: error = xfs_trans_commit_iclog(mp, tp, &commit_lsn, flags); if (error == ENOMEM) { xfs_force_shutdown(mp, SHUTDOWN_LOG_IO_ERROR); - goto shut_us_down; + error = XFS_ERROR(EIO); + goto out_unreserve; } /* @@ -1103,7 +1086,29 @@ shut_us_down: XFS_STATS_INC(xs_trans_async); } - return (error); + return error; + +out_unreserve: + xfs_trans_unreserve_and_mod_sb(tp); + + /* + * It is indeed possible for the transaction to be not dirty but + * the dqinfo portion to be. All that means is that we have some + * (non-persistent) quota reservations that need to be unreserved. + */ + xfs_trans_unreserve_and_mod_dquots(tp); + if (tp->t_ticket) { + commit_lsn = xfs_log_done(mp, tp->t_ticket, NULL, log_flags); + if (commit_lsn == -1 && !error) + error = XFS_ERROR(EIO); + } + current_restore_flags_nested(&tp->t_pflags, PF_FSTRANS); + xfs_trans_free_items(tp, error ? XFS_TRANS_ABORT : 0); + xfs_trans_free_busy(tp); + xfs_trans_free(tp); + + XFS_STATS_INC(xs_trans_empty); + return error; } /* -- cgit v1.2.2 From 8e646a55ac69fe620b9e84034c03dd1e8e16a36b Mon Sep 17 00:00:00 2001 From: Dave Chinner Date: Mon, 8 Mar 2010 15:06:22 +1100 Subject: xfs: update and factor xfs_trans_committed() The function header to xfs-trans_committed has long had this comment: * THIS SHOULD BE REWRITTEN TO USE xfs_trans_next_item() To prepare for different methods of committing items, convert the code to use xfs_trans_next_item() and factor the code into smaller, more digestible chunks. Signed-off-by: Dave Chinner Reviewed-by: Christoph Hellwig --- fs/xfs/xfs_trans.c | 224 +++++++++++++++++++++++------------------------------ 1 file changed, 95 insertions(+), 129 deletions(-) (limited to 'fs/xfs/xfs_trans.c') diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c index 2bff22995127..084bd3a13184 100644 --- a/fs/xfs/xfs_trans.c +++ b/fs/xfs/xfs_trans.c @@ -49,7 +49,6 @@ STATIC void xfs_trans_apply_sb_deltas(xfs_trans_t *); STATIC void xfs_trans_uncommit(xfs_trans_t *, uint); STATIC void xfs_trans_committed(xfs_trans_t *, int); -STATIC void xfs_trans_chunk_committed(xfs_log_item_chunk_t *, xfs_lsn_t, int); STATIC void xfs_trans_free(xfs_trans_t *); kmem_zone_t *xfs_trans_zone; @@ -1301,60 +1300,86 @@ xfs_trans_roll( } /* - * THIS SHOULD BE REWRITTEN TO USE xfs_trans_next_item(). + * The committed item processing consists of calling the committed routine of + * each logged item, updating the item's position in the AIL if necessary, and + * unpinning each item. If the committed routine returns -1, then do nothing + * further with the item because it may have been freed. * - * This is typically called by the LM when a transaction has been fully - * committed to disk. It needs to unpin the items which have - * been logged by the transaction and update their positions - * in the AIL if necessary. - * This also gets called when the transactions didn't get written out - * because of an I/O error. Abortflag & XFS_LI_ABORTED is set then. + * Since items are unlocked when they are copied to the incore log, it is + * possible for two transactions to be completing and manipulating the same + * item simultaneously. The AIL lock will protect the lsn field of each item. + * The value of this field can never go backwards. * - * Call xfs_trans_chunk_committed() to process the items in - * each chunk. + * We unpin the items after repositioning them in the AIL, because otherwise + * they could be immediately flushed and we'd have to race with the flusher + * trying to pull the item from the AIL as we add it. */ -STATIC void -xfs_trans_committed( - xfs_trans_t *tp, - int abortflag) +static void +xfs_trans_item_committed( + xfs_log_item_t *lip, + xfs_lsn_t commit_lsn, + int aborted) { - xfs_log_item_chunk_t *licp; - xfs_log_item_chunk_t *next_licp; - xfs_log_busy_chunk_t *lbcp; - xfs_log_busy_slot_t *lbsp; - int i; + xfs_lsn_t item_lsn; + struct xfs_ail *ailp; + + if (aborted) + lip->li_flags |= XFS_LI_ABORTED; /* - * Call the transaction's completion callback if there - * is one. + * Send in the ABORTED flag to the COMMITTED routine so that it knows + * whether the transaction was aborted or not. */ - if (tp->t_callback != NULL) { - tp->t_callback(tp, tp->t_callarg); - } + item_lsn = IOP_COMMITTED(lip, commit_lsn); /* - * Special case the chunk embedded in the transaction. + * If the committed routine returns -1, item has been freed. */ - licp = &(tp->t_items); - if (!(xfs_lic_are_all_free(licp))) { - xfs_trans_chunk_committed(licp, tp->t_lsn, abortflag); - } + if (XFS_LSN_CMP(item_lsn, (xfs_lsn_t)-1) == 0) + return; /* - * Process the items in each chunk in turn. + * If the returned lsn is greater than what it contained before, update + * the location of the item in the AIL. If it is not, then do nothing. + * Items can never move backwards in the AIL. + * + * While the new lsn should usually be greater, it is possible that a + * later transaction completing simultaneously with an earlier one + * using the same item could complete first with a higher lsn. This + * would cause the earlier transaction to fail the test below. */ - licp = licp->lic_next; - while (licp != NULL) { - ASSERT(!xfs_lic_are_all_free(licp)); - xfs_trans_chunk_committed(licp, tp->t_lsn, abortflag); - next_licp = licp->lic_next; - kmem_free(licp); - licp = next_licp; + ailp = lip->li_ailp; + spin_lock(&ailp->xa_lock); + if (XFS_LSN_CMP(item_lsn, lip->li_lsn) > 0) { + /* + * This will set the item's lsn to item_lsn and update the + * position of the item in the AIL. + * + * xfs_trans_ail_update() drops the AIL lock. + */ + xfs_trans_ail_update(ailp, lip, item_lsn); + } else { + spin_unlock(&ailp->xa_lock); } /* - * Clear all the per-AG busy list items listed in this transaction + * Now that we've repositioned the item in the AIL, unpin it so it can + * be flushed. Pass information about buffer stale state down from the + * log item flags, if anyone else stales the buffer we do not want to + * pay any attention to it. */ + IOP_UNPIN(lip); +} + +/* Clear all the per-AG busy list items listed in this transaction */ +static void +xfs_trans_clear_busy_extents( + struct xfs_trans *tp) +{ + xfs_log_busy_chunk_t *lbcp; + xfs_log_busy_slot_t *lbsp; + int i; + lbcp = &tp->t_busy; while (lbcp != NULL) { for (i = 0, lbsp = lbcp->lbc_busy; i < lbcp->lbc_unused; i++, lbsp++) { @@ -1366,107 +1391,48 @@ xfs_trans_committed( lbcp = lbcp->lbc_next; } xfs_trans_free_busy(tp); - - /* - * That's it for the transaction structure. Free it. - */ - xfs_trans_free(tp); } /* - * This is called to perform the commit processing for each - * item described by the given chunk. - * - * The commit processing consists of unlocking items which were - * held locked with the SYNC_UNLOCK attribute, calling the committed - * routine of each logged item, updating the item's position in the AIL - * if necessary, and unpinning each item. If the committed routine - * returns -1, then do nothing further with the item because it - * may have been freed. - * - * Since items are unlocked when they are copied to the incore - * log, it is possible for two transactions to be completing - * and manipulating the same item simultaneously. The AIL lock - * will protect the lsn field of each item. The value of this - * field can never go backwards. + * This is typically called by the LM when a transaction has been fully + * committed to disk. It needs to unpin the items which have + * been logged by the transaction and update their positions + * in the AIL if necessary. * - * We unpin the items after repositioning them in the AIL, because - * otherwise they could be immediately flushed and we'd have to race - * with the flusher trying to pull the item from the AIL as we add it. + * This also gets called when the transactions didn't get written out + * because of an I/O error. Abortflag & XFS_LI_ABORTED is set then. */ STATIC void -xfs_trans_chunk_committed( - xfs_log_item_chunk_t *licp, - xfs_lsn_t lsn, - int aborted) +xfs_trans_committed( + xfs_trans_t *tp, + int abortflag) { xfs_log_item_desc_t *lidp; - xfs_log_item_t *lip; - xfs_lsn_t item_lsn; - int i; - - lidp = licp->lic_descs; - for (i = 0; i < licp->lic_unused; i++, lidp++) { - struct xfs_ail *ailp; - - if (xfs_lic_isfree(licp, i)) { - continue; - } - - lip = lidp->lid_item; - if (aborted) - lip->li_flags |= XFS_LI_ABORTED; - - /* - * Send in the ABORTED flag to the COMMITTED routine - * so that it knows whether the transaction was aborted - * or not. - */ - item_lsn = IOP_COMMITTED(lip, lsn); + xfs_log_item_chunk_t *licp; + xfs_log_item_chunk_t *next_licp; - /* - * If the committed routine returns -1, make - * no more references to the item. - */ - if (XFS_LSN_CMP(item_lsn, (xfs_lsn_t)-1) == 0) { - continue; - } + /* + * Call the transaction's completion callback if there + * is one. + */ + if (tp->t_callback != NULL) { + tp->t_callback(tp, tp->t_callarg); + } - /* - * If the returned lsn is greater than what it - * contained before, update the location of the - * item in the AIL. If it is not, then do nothing. - * Items can never move backwards in the AIL. - * - * While the new lsn should usually be greater, it - * is possible that a later transaction completing - * simultaneously with an earlier one using the - * same item could complete first with a higher lsn. - * This would cause the earlier transaction to fail - * the test below. - */ - ailp = lip->li_ailp; - spin_lock(&ailp->xa_lock); - if (XFS_LSN_CMP(item_lsn, lip->li_lsn) > 0) { - /* - * This will set the item's lsn to item_lsn - * and update the position of the item in - * the AIL. - * - * xfs_trans_ail_update() drops the AIL lock. - */ - xfs_trans_ail_update(ailp, lip, item_lsn); - } else { - spin_unlock(&ailp->xa_lock); - } + for (lidp = xfs_trans_first_item(tp); + lidp != NULL; + lidp = xfs_trans_next_item(tp, lidp)) { + xfs_trans_item_committed(lidp->lid_item, tp->t_lsn, abortflag); + } - /* - * Now that we've repositioned the item in the AIL, - * unpin it so it can be flushed. Pass information - * about buffer stale state down from the log item - * flags, if anyone else stales the buffer we do not - * want to pay any attention to it. - */ - IOP_UNPIN(lip); + /* free the item chunks, ignoring the embedded chunk */ + licp = tp->t_items.lic_next; + while (licp != NULL) { + next_licp = licp->lic_next; + kmem_free(licp); + licp = next_licp; } + + xfs_trans_clear_busy_extents(tp); + xfs_trans_free(tp); } -- cgit v1.2.2 From b1c1b5b6108ad8e5991a614514f41da436c659d6 Mon Sep 17 00:00:00 2001 From: Dave Chinner Date: Tue, 23 Mar 2010 10:11:05 +1100 Subject: xfs: Clean up xfs_trans_committed code after factoring Now that the code has been factored, clean up all the remaining style cruft, simplify the code and re-order functions so that it doesn't need forward declarations. Also move the remaining functions that require forward declarations (xfs_trans_uncommit, xfs_trans_free) so that all the forward declarations can be removed from the file. Signed-off-by: Dave Chinner Reviewed-by: Christoph Hellwig --- fs/xfs/xfs_trans.c | 356 +++++++++++++++++++++++++---------------------------- 1 file changed, 166 insertions(+), 190 deletions(-) (limited to 'fs/xfs/xfs_trans.c') diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c index 084bd3a13184..be578ecb4af2 100644 --- a/fs/xfs/xfs_trans.c +++ b/fs/xfs/xfs_trans.c @@ -45,20 +45,12 @@ #include "xfs_trans_space.h" #include "xfs_inode_item.h" - -STATIC void xfs_trans_apply_sb_deltas(xfs_trans_t *); -STATIC void xfs_trans_uncommit(xfs_trans_t *, uint); -STATIC void xfs_trans_committed(xfs_trans_t *, int); -STATIC void xfs_trans_free(xfs_trans_t *); - kmem_zone_t *xfs_trans_zone; - /* * Reservation functions here avoid a huge stack in xfs_trans_init * due to register overflow from temporaries in the calculations. */ - STATIC uint xfs_calc_write_reservation(xfs_mount_t *mp) { @@ -257,6 +249,19 @@ _xfs_trans_alloc( return tp; } +/* + * Free the transaction structure. If there is more clean up + * to do when the structure is freed, add it here. + */ +STATIC void +xfs_trans_free( + xfs_trans_t *tp) +{ + atomic_dec(&tp->t_mountp->m_active_trans); + xfs_trans_free_dqinfo(tp); + kmem_zone_free(xfs_trans_zone, tp); +} + /* * This is called to create a new transaction which will share the * permanent log reservation of the given transaction. The remaining @@ -769,7 +774,7 @@ xfs_trans_unreserve_and_mod_sb( */ static uint xfs_trans_count_vecs( - xfs_trans_t *tp) + struct xfs_trans *tp) { int nvecs; xfs_log_item_desc_t *lidp; @@ -860,6 +865,158 @@ xfs_trans_fill_vecs( log_vector->i_type = XLOG_REG_TYPE_TRANSHDR; } +/* + * The committed item processing consists of calling the committed routine of + * each logged item, updating the item's position in the AIL if necessary, and + * unpinning each item. If the committed routine returns -1, then do nothing + * further with the item because it may have been freed. + * + * Since items are unlocked when they are copied to the incore log, it is + * possible for two transactions to be completing and manipulating the same + * item simultaneously. The AIL lock will protect the lsn field of each item. + * The value of this field can never go backwards. + * + * We unpin the items after repositioning them in the AIL, because otherwise + * they could be immediately flushed and we'd have to race with the flusher + * trying to pull the item from the AIL as we add it. + */ +static void +xfs_trans_item_committed( + struct xfs_log_item *lip, + xfs_lsn_t commit_lsn, + int aborted) +{ + xfs_lsn_t item_lsn; + struct xfs_ail *ailp; + + if (aborted) + lip->li_flags |= XFS_LI_ABORTED; + item_lsn = IOP_COMMITTED(lip, commit_lsn); + + /* If the committed routine returns -1, item has been freed. */ + if (XFS_LSN_CMP(item_lsn, (xfs_lsn_t)-1) == 0) + return; + + /* + * If the returned lsn is greater than what it contained before, update + * the location of the item in the AIL. If it is not, then do nothing. + * Items can never move backwards in the AIL. + * + * While the new lsn should usually be greater, it is possible that a + * later transaction completing simultaneously with an earlier one + * using the same item could complete first with a higher lsn. This + * would cause the earlier transaction to fail the test below. + */ + ailp = lip->li_ailp; + spin_lock(&ailp->xa_lock); + if (XFS_LSN_CMP(item_lsn, lip->li_lsn) > 0) { + /* + * This will set the item's lsn to item_lsn and update the + * position of the item in the AIL. + * + * xfs_trans_ail_update() drops the AIL lock. + */ + xfs_trans_ail_update(ailp, lip, item_lsn); + } else { + spin_unlock(&ailp->xa_lock); + } + + /* + * Now that we've repositioned the item in the AIL, unpin it so it can + * be flushed. Pass information about buffer stale state down from the + * log item flags, if anyone else stales the buffer we do not want to + * pay any attention to it. + */ + IOP_UNPIN(lip); +} + +/* Clear all the per-AG busy list items listed in this transaction */ +static void +xfs_trans_clear_busy_extents( + struct xfs_trans *tp) +{ + xfs_log_busy_chunk_t *lbcp; + xfs_log_busy_slot_t *lbsp; + int i; + + for (lbcp = &tp->t_busy; lbcp != NULL; lbcp = lbcp->lbc_next) { + i = 0; + for (lbsp = lbcp->lbc_busy; i < lbcp->lbc_unused; i++, lbsp++) { + if (XFS_LBC_ISFREE(lbcp, i)) + continue; + xfs_alloc_clear_busy(tp, lbsp->lbc_ag, lbsp->lbc_idx); + } + } + xfs_trans_free_busy(tp); +} + +/* + * This is typically called by the LM when a transaction has been fully + * committed to disk. It needs to unpin the items which have + * been logged by the transaction and update their positions + * in the AIL if necessary. + * + * This also gets called when the transactions didn't get written out + * because of an I/O error. Abortflag & XFS_LI_ABORTED is set then. + */ +STATIC void +xfs_trans_committed( + struct xfs_trans *tp, + int abortflag) +{ + xfs_log_item_desc_t *lidp; + xfs_log_item_chunk_t *licp; + xfs_log_item_chunk_t *next_licp; + + /* Call the transaction's completion callback if there is one. */ + if (tp->t_callback != NULL) + tp->t_callback(tp, tp->t_callarg); + + for (lidp = xfs_trans_first_item(tp); + lidp != NULL; + lidp = xfs_trans_next_item(tp, lidp)) { + xfs_trans_item_committed(lidp->lid_item, tp->t_lsn, abortflag); + } + + /* free the item chunks, ignoring the embedded chunk */ + for (licp = tp->t_items.lic_next; licp != NULL; licp = next_licp) { + next_licp = licp->lic_next; + kmem_free(licp); + } + + xfs_trans_clear_busy_extents(tp); + xfs_trans_free(tp); +} + +/* + * Called from the trans_commit code when we notice that + * the filesystem is in the middle of a forced shutdown. + */ +STATIC void +xfs_trans_uncommit( + struct xfs_trans *tp, + uint flags) +{ + xfs_log_item_desc_t *lidp; + + for (lidp = xfs_trans_first_item(tp); + lidp != NULL; + lidp = xfs_trans_next_item(tp, lidp)) { + /* + * Unpin all but those that aren't dirty. + */ + if (lidp->lid_flags & XFS_LID_DIRTY) + IOP_UNPIN_REMOVE(lidp->lid_item, tp); + } + + xfs_trans_unreserve_and_mod_sb(tp); + xfs_trans_unreserve_and_mod_dquots(tp); + + xfs_trans_free_items(tp, flags); + xfs_trans_free_busy(tp); + xfs_trans_free(tp); +} + /* * Format the transaction direct to the iclog. This isolates the physical * transaction commit operation from the logical operation and hence allows @@ -1110,35 +1267,6 @@ out_unreserve: return error; } -/* - * Called from the trans_commit code when we notice that - * the filesystem is in the middle of a forced shutdown. - */ -STATIC void -xfs_trans_uncommit( - xfs_trans_t *tp, - uint flags) -{ - xfs_log_item_desc_t *lidp; - - for (lidp = xfs_trans_first_item(tp); - lidp != NULL; - lidp = xfs_trans_next_item(tp, lidp)) { - /* - * Unpin all but those that aren't dirty. - */ - if (lidp->lid_flags & XFS_LID_DIRTY) - IOP_UNPIN_REMOVE(lidp->lid_item, tp); - } - - xfs_trans_unreserve_and_mod_sb(tp); - xfs_trans_unreserve_and_mod_dquots(tp); - - xfs_trans_free_items(tp, flags); - xfs_trans_free_busy(tp); - xfs_trans_free(tp); -} - /* * Unlock all of the transaction's items and free the transaction. * The transaction must not have modified any of its items, because @@ -1215,20 +1343,6 @@ xfs_trans_cancel( xfs_trans_free(tp); } - -/* - * Free the transaction structure. If there is more clean up - * to do when the structure is freed, add it here. - */ -STATIC void -xfs_trans_free( - xfs_trans_t *tp) -{ - atomic_dec(&tp->t_mountp->m_active_trans); - xfs_trans_free_dqinfo(tp); - kmem_zone_free(xfs_trans_zone, tp); -} - /* * Roll from one trans in the sequence of PERMANENT transactions to * the next: permanent transactions are only flushed out when @@ -1298,141 +1412,3 @@ xfs_trans_roll( xfs_trans_ihold(trans, dp); return 0; } - -/* - * The committed item processing consists of calling the committed routine of - * each logged item, updating the item's position in the AIL if necessary, and - * unpinning each item. If the committed routine returns -1, then do nothing - * further with the item because it may have been freed. - * - * Since items are unlocked when they are copied to the incore log, it is - * possible for two transactions to be completing and manipulating the same - * item simultaneously. The AIL lock will protect the lsn field of each item. - * The value of this field can never go backwards. - * - * We unpin the items after repositioning them in the AIL, because otherwise - * they could be immediately flushed and we'd have to race with the flusher - * trying to pull the item from the AIL as we add it. - */ -static void -xfs_trans_item_committed( - xfs_log_item_t *lip, - xfs_lsn_t commit_lsn, - int aborted) -{ - xfs_lsn_t item_lsn; - struct xfs_ail *ailp; - - if (aborted) - lip->li_flags |= XFS_LI_ABORTED; - - /* - * Send in the ABORTED flag to the COMMITTED routine so that it knows - * whether the transaction was aborted or not. - */ - item_lsn = IOP_COMMITTED(lip, commit_lsn); - - /* - * If the committed routine returns -1, item has been freed. - */ - if (XFS_LSN_CMP(item_lsn, (xfs_lsn_t)-1) == 0) - return; - - /* - * If the returned lsn is greater than what it contained before, update - * the location of the item in the AIL. If it is not, then do nothing. - * Items can never move backwards in the AIL. - * - * While the new lsn should usually be greater, it is possible that a - * later transaction completing simultaneously with an earlier one - * using the same item could complete first with a higher lsn. This - * would cause the earlier transaction to fail the test below. - */ - ailp = lip->li_ailp; - spin_lock(&ailp->xa_lock); - if (XFS_LSN_CMP(item_lsn, lip->li_lsn) > 0) { - /* - * This will set the item's lsn to item_lsn and update the - * position of the item in the AIL. - * - * xfs_trans_ail_update() drops the AIL lock. - */ - xfs_trans_ail_update(ailp, lip, item_lsn); - } else { - spin_unlock(&ailp->xa_lock); - } - - /* - * Now that we've repositioned the item in the AIL, unpin it so it can - * be flushed. Pass information about buffer stale state down from the - * log item flags, if anyone else stales the buffer we do not want to - * pay any attention to it. - */ - IOP_UNPIN(lip); -} - -/* Clear all the per-AG busy list items listed in this transaction */ -static void -xfs_trans_clear_busy_extents( - struct xfs_trans *tp) -{ - xfs_log_busy_chunk_t *lbcp; - xfs_log_busy_slot_t *lbsp; - int i; - - lbcp = &tp->t_busy; - while (lbcp != NULL) { - for (i = 0, lbsp = lbcp->lbc_busy; i < lbcp->lbc_unused; i++, lbsp++) { - if (!XFS_LBC_ISFREE(lbcp, i)) { - xfs_alloc_clear_busy(tp, lbsp->lbc_ag, - lbsp->lbc_idx); - } - } - lbcp = lbcp->lbc_next; - } - xfs_trans_free_busy(tp); -} - -/* - * This is typically called by the LM when a transaction has been fully - * committed to disk. It needs to unpin the items which have - * been logged by the transaction and update their positions - * in the AIL if necessary. - * - * This also gets called when the transactions didn't get written out - * because of an I/O error. Abortflag & XFS_LI_ABORTED is set then. - */ -STATIC void -xfs_trans_committed( - xfs_trans_t *tp, - int abortflag) -{ - xfs_log_item_desc_t *lidp; - xfs_log_item_chunk_t *licp; - xfs_log_item_chunk_t *next_licp; - - /* - * Call the transaction's completion callback if there - * is one. - */ - if (tp->t_callback != NULL) { - tp->t_callback(tp, tp->t_callarg); - } - - for (lidp = xfs_trans_first_item(tp); - lidp != NULL; - lidp = xfs_trans_next_item(tp, lidp)) { - xfs_trans_item_committed(lidp->lid_item, tp->t_lsn, abortflag); - } - - /* free the item chunks, ignoring the embedded chunk */ - licp = tp->t_items.lic_next; - while (licp != NULL) { - next_licp = licp->lic_next; - kmem_free(licp); - licp = next_licp; - } - - xfs_trans_clear_busy_extents(tp); - xfs_trans_free(tp); -} -- cgit v1.2.2