xfs: condense dfops and automatic relogging
v2 cover xfs: condense dfops and automatic relogging
Hi all,
Here's v2 of the condense dfops patches. This has mostly minor changes such as extra error checks, comments and cleanups.
Thoughts, reviews, flames appreciated.
Brian
v2:
- Add Reviewed-by tags.
- Rename container transaction for intent log recovery, add comment.
- More error checks in automatic dfops relogging.
- Assert and warn checks for permanent transaction with deferred ops.
- Function renames/refactors for on-stack pending dfops list patch.
- Drop unnecessary xfs_mount param from tracepoints with transaction.
v1: https://marc.info/?l=linux-xfs&m=153296912912425&w=2
Brian Foster (15):
xfs: refactor internal dfops initialization
xfs: use transaction for intent recovery instead of raw dfops
xfs: remove unused __xfs_defer_cancel() internal helper
xfs: pass transaction to dfops reset/move helpers
xfs: replace dop_low with transaction flag
xfs: add missing defer ijoins for held inodes
xfs: automatic dfops buffer relogging
xfs: automatic dfops inode relogging
xfs: drop dop param from xfs_defer_op_type ->finish_item() callback
xfs: clean out superfluous dfops dop params/vars
xfs: cancel dfops on xfs_defer_finish() error
xfs: replace xfs_defer_ops ->dop_pending with on-stack list
xfs: pass transaction to xfs_defer_add()
xfs: always defer agfl block frees
xfs: fold dfops into the transaction
fs/xfs/libxfs/xfs_alloc.c | 18 +- fs/xfs/libxfs/xfs_attr.c | 26 +-- fs/xfs/libxfs/xfs_attr_remote.c | 6 +- fs/xfs/libxfs/xfs_bmap.c | 93 ++++----- fs/xfs/libxfs/xfs_bmap.h | 25 ++- fs/xfs/libxfs/xfs_bmap_btree.c | 6 +- fs/xfs/libxfs/xfs_btree.h | 1 - fs/xfs/libxfs/xfs_da_btree.h | 1 - fs/xfs/libxfs/xfs_defer.c | 322 ++++++++++++++------------------ fs/xfs/libxfs/xfs_defer.h | 26 +-- fs/xfs/libxfs/xfs_dir2.c | 2 - fs/xfs/libxfs/xfs_dir2.h | 1 - fs/xfs/libxfs/xfs_ialloc.c | 25 +-- fs/xfs/libxfs/xfs_refcount.c | 76 ++++---- fs/xfs/libxfs/xfs_refcount.h | 25 ++- fs/xfs/libxfs/xfs_rmap.c | 53 +++--- fs/xfs/libxfs/xfs_rmap.h | 22 +-- fs/xfs/libxfs/xfs_shared.h | 12 ++ fs/xfs/xfs_bmap_item.c | 20 +- fs/xfs/xfs_bmap_item.h | 3 +- fs/xfs/xfs_bmap_util.c | 18 +- fs/xfs/xfs_dquot.c | 3 +- fs/xfs/xfs_filestream.c | 3 +- fs/xfs/xfs_inode.c | 3 +- fs/xfs/xfs_inode.h | 1 - fs/xfs/xfs_log_recover.c | 51 ++--- fs/xfs/xfs_refcount_item.c | 30 ++- fs/xfs/xfs_refcount_item.h | 3 +- fs/xfs/xfs_reflink.c | 33 ++-- fs/xfs/xfs_symlink.c | 1 - fs/xfs/xfs_trace.h | 51 ++--- fs/xfs/xfs_trans.c | 39 ++-- fs/xfs/xfs_trans.h | 28 +-- fs/xfs/xfs_trans_bmap.c | 6 +- fs/xfs/xfs_trans_extfree.c | 2 - fs/xfs/xfs_trans_refcount.c | 6 +- fs/xfs/xfs_trans_rmap.c | 1 - 37 files changed, 434 insertions(+), 608 deletions(-)
Signature · 1 line
2.17.1
PATCH v2 1/15 xfs: refactor internal dfops initialization
The current transaction allocation code conditionally initializes the ->t_dfops indirection pointer. Transaction commit/cancel check the validity of the pointer to determine whether to finish/cancel the internal dfops.
This disallows the ability to use the internal dfops list as a temporary container (via xfs_trans_alloc_empty()). Refactor transaction allocation to always initialize ->t_dfops and check permanent reservation state on transaction commit/cancel.
--- fs/xfs/xfs_trans.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)
fs/xfs/xfs_trans.c +9 -10
@@ -281,13 +281,7 @@ xfs_trans_alloc(INIT_LIST_HEAD (&tp->t_items);INIT_LIST_HEAD (&tp->t_busy); tp->t_firstblock =NULLFSBLOCK ; -/* -* We only roll transactions with permanent log reservation. Don't init -* ->t_dfops to skip attempts to finish or cancel an empty dfops with a -* non-permanent res. -*/ -if (resp->tr_logflags &XFS_TRANS_PERM_LOG_RES ) -xfs_defer_init (tp, &tp->t_dfops_internal); +xfs_defer_init (tp, &tp->t_dfops_internal); error =xfs_trans_reserve (tp, resp, blocks, rtextents);if (error) {
@@ -931,8 +925,13 @@ __xfs_trans_commit(trace_xfs_trans_commit (tp, _RET_IP_); -/* finish deferred items on final commit */ -if (!regrant && tp->t_dfops) { +/* +* Finish deferred items on final commit. Only permanent transactions +* should ever have deferred ops. +*/ +WARN_ON_ONCE (xfs_defer_has_unfinished_work (tp->t_dfops) && + !(tp->t_flags &XFS_TRANS_PERM_LOG_RES )); +if (!regrant && (tp->t_flags &XFS_TRANS_PERM_LOG_RES )) { error =xfs_defer_finish_noroll (&tp);if (error) {xfs_defer_cancel (tp);
@@ -1029,7 +1028,7 @@ xfs_trans_cancel(trace_xfs_trans_cancel (tp, _RET_IP_); -if (tp->t_dfops) +if (tp->t_flags &XFS_TRANS_PERM_LOG_RES )xfs_defer_cancel (tp);/*
Signature · 1 line
2.17.1
Re: [PATCH v2 01/15] xfs: refactor internal dfops initialization
Quoted 5 lines · Brian Foster
The current transaction allocation code conditionally initializes the ->t_dfops indirection pointer. Transaction commit/cancel check the validity of the pointer to determine whether to finish/cancel the internal dfops.
Quoted 7 lines
This disallows the ability to use the internal dfops list as a temporary container (via
xfs_trans_alloc_empty()). Refactor transaction allocation to always initialize ->t_dfops and check permanent reservation state on transaction commit/cancel.Signed-off-by Brian Foster <[email protected]>Reviewed-by Christoph Hellwig <[email protected]>
Looks ok,
--D
Quoted 55 lines
--- fs/xfs/xfs_trans.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)fs/xfs/xfs_trans.c +9
-10@@ -281,13 +281,7 @@ xfs_trans_alloc(INIT_LIST_HEAD (&tp->t_items);INIT_LIST_HEAD (&tp->t_busy); tp->t_firstblock =NULLFSBLOCK ; -/* -* We only roll transactions with permanent log reservation. Don't init -* ->t_dfops to skip attempts to finish or cancel an empty dfops with a -* non-permanent res. -*/ -if (resp->tr_logflags &XFS_TRANS_PERM_LOG_RES ) -xfs_defer_init (tp, &tp->t_dfops_internal); +xfs_defer_init (tp, &tp->t_dfops_internal); error =xfs_trans_reserve (tp, resp, blocks, rtextents);if (error) {@@ -931,8 +925,13 @@ __xfs_trans_commit(trace_xfs_trans_commit (tp, _RET_IP_); -/* finish deferred items on final commit */ -if (!regrant && tp->t_dfops) { +/* +* Finish deferred items on final commit. Only permanent transactions +* should ever have deferred ops. +*/ +WARN_ON_ONCE (xfs_defer_has_unfinished_work (tp->t_dfops) && + !(tp->t_flags &XFS_TRANS_PERM_LOG_RES )); +if (!regrant && (tp->t_flags &XFS_TRANS_PERM_LOG_RES )) { error =xfs_defer_finish_noroll (&tp);if (error) {xfs_defer_cancel (tp);@@ -1029,7 +1028,7 @@ xfs_trans_cancel(trace_xfs_trans_cancel (tp, _RET_IP_); -if (tp->t_dfops) +if (tp->t_flags &XFS_TRANS_PERM_LOG_RES )xfs_defer_cancel (tp);/* Signature · 6 lines
2.17.1 -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v2 2/15 xfs: use transaction for intent recovery instead of raw dfops
In reply to Brian FosterLog intent recovery is the last user of an external (on-stack) dfops. The pattern exists because the dfops is used to collect additional deferred operations queued during the whole recovery sequence. The dfops is finished with a new transaction after intent recovery completes.
We already have a mechanism to create an empty, container-like transaction to support the scrub infrastructure. We can reuse that mechanism here to drop the final user of external dfops. This facilitates folding dfops state (i.e., dop_low) into the transaction, the elimination of now unused external dfops support and also eliminates the only caller of __xfs_defer_cancel().
Replace the on-stack dfops with an empty transaction and pass it around to the various helpers that queue and finish deferred operations during intent recovery.
--- fs/xfs/xfs_bmap_item.c | 12 ++++----- fs/xfs/xfs_bmap_item.h | 3 +-- fs/xfs/xfs_log_recover.c | 51 ++++++++++++++++++++++---------------- fs/xfs/xfs_refcount_item.c | 12 ++++----- fs/xfs/xfs_refcount_item.h | 3 +-- 5 files changed, 43 insertions(+), 38 deletions(-)
fs/xfs/xfs_bmap_item.c +6 -6
@@ -375,9 +375,8 @@ xfs_bud_init(*/ int xfs_bui_recover ( -struct xfs_mount *mp, -struct xfs_bui_log_item *buip, -struct xfs_defer_ops *dfops) +struct xfs_trans *parent_tp, +struct xfs_bui_log_item *buip) {int error =0 ;unsigned int bui_type;
@@ -393,6 +392,7 @@ xfs_bui_recover(struct xfs_trans *tp;struct xfs_inode *ip =NULL ;struct xfs_bmbt_irec irec; +struct xfs_mount *mp = parent_tp->t_mountp;ASSERT (!test_bit (XFS_BUI_RECOVERED , &buip->bui_flags));
@@ -446,7 +446,7 @@ xfs_bui_recover(* finishes them on completion. Transfer current dfops state to this * transaction and transfer the result back before we return. */ -xfs_defer_move (tp->t_dfops, dfops); +xfs_defer_move (tp->t_dfops, parent_tp->t_dfops); budp =xfs_trans_get_bud (tp, buip);/* Grab the inode. */
@@ -494,7 +494,7 @@ xfs_bui_recover( }set_bit (XFS_BUI_RECOVERED , &buip->bui_flags); -xfs_defer_move (dfops, tp->t_dfops); +xfs_defer_move (parent_tp->t_dfops, tp->t_dfops); error =xfs_trans_commit (tp);xfs_iunlock (ip,XFS_ILOCK_EXCL );xfs_irele (ip);
@@ -502,7 +502,7 @@ xfs_bui_recover(return error; err_inode: -xfs_defer_move (dfops, tp->t_dfops); +xfs_defer_move (parent_tp->t_dfops, tp->t_dfops);xfs_trans_cancel (tp);if (ip) {xfs_iunlock (ip,XFS_ILOCK_EXCL );
fs/xfs/xfs_bmap_item.h +1 -2
@@ -79,7 +79,6 @@ struct xfs_bud_log_item *xfs_bud_init(struct xfs_mount *, struct xfs_bui_log_item *);void xfs_bui_item_free (struct xfs_bui_log_item *);void xfs_bui_release (struct xfs_bui_log_item *); -int xfs_bui_recover (struct xfs_mount *mp,struct xfs_bui_log_item *buip, -struct xfs_defer_ops *dfops); +int xfs_bui_recover (struct xfs_trans *parent_tp,struct xfs_bui_log_item *buip);#endif /* __XFS_BMAP_ITEM_H__ */
fs/xfs/xfs_log_recover.c +29 -22
123 lines not on this page · raw message
fs/xfs/xfs_refcount_item.c +6 -6
@@ -380,9 +380,8 @@ xfs_cud_init(*/ int xfs_cui_recover ( -struct xfs_mount *mp, -struct xfs_cui_log_item *cuip, -struct xfs_defer_ops *dfops) +struct xfs_trans *parent_tp, +struct xfs_cui_log_item *cuip) {int i;int error =0 ;
@@ -398,6 +397,7 @@ xfs_cui_recover(xfs_extlen_t new_len;struct xfs_bmbt_irec irec;bool requeue_only = false; +struct xfs_mount *mp = parent_tp->t_mountp;ASSERT (!test_bit (XFS_CUI_RECOVERED , &cuip->cui_flags));
@@ -457,7 +457,7 @@ xfs_cui_recover(* finishes them on completion. Transfer current dfops state to this * transaction and transfer the result back before we return. */ -xfs_defer_move (tp->t_dfops, dfops); +xfs_defer_move (tp->t_dfops, parent_tp->t_dfops); cudp =xfs_trans_get_cud (tp, cuip);for (i =0 ; i < cuip->cui_format.cui_nextents; i++) {
@@ -522,13 +522,13 @@ xfs_cui_recover(xfs_refcount_finish_one_cleanup (tp, rcur, error);set_bit (XFS_CUI_RECOVERED , &cuip->cui_flags); -xfs_defer_move (dfops, tp->t_dfops); +xfs_defer_move (parent_tp->t_dfops, tp->t_dfops); error =xfs_trans_commit (tp);return error; abort_error:xfs_refcount_finish_one_cleanup (tp, rcur, error); -xfs_defer_move (dfops, tp->t_dfops); +xfs_defer_move (parent_tp->t_dfops, tp->t_dfops);xfs_trans_cancel (tp);return error; }
fs/xfs/xfs_refcount_item.h +1 -2
@@ -82,7 +82,6 @@ struct xfs_cud_log_item *xfs_cud_init(struct xfs_mount *, struct xfs_cui_log_item *);void xfs_cui_item_free (struct xfs_cui_log_item *);void xfs_cui_release (struct xfs_cui_log_item *); -int xfs_cui_recover (struct xfs_mount *mp,struct xfs_cui_log_item *cuip, -struct xfs_defer_ops *dfops); +int xfs_cui_recover (struct xfs_trans *parent_tp,struct xfs_cui_log_item *cuip);#endif /* __XFS_REFCOUNT_ITEM_H__ */
Signature · 1 line
2.17.1
Re: [PATCH v2 02/15] xfs: use transaction for intent recovery instead of raw dfops
Quoted 13 lines · Brian Foster
Log intent recovery is the last user of an external (on-stack) dfops. The pattern exists because the dfops is used to collect additional deferred operations queued during the whole recovery sequence. The dfops is finished with a new transaction after intent recovery completes.
We already have a mechanism to create an empty, container-like transaction to support the scrub infrastructure. We can reuse that mechanism here to drop the final user of external dfops. This facilitates folding dfops state (i.e.,
dop_low) into the transaction, the elimination of now unused external dfops support and also eliminates the only caller of__xfs_defer_cancel().
Quoted 6 lines
Replace the on-stack dfops with an empty transaction and pass it around to the various helpers that queue and finish deferred operations during intent recovery.
Signed-off-by Brian Foster <[email protected]>Reviewed-by Christoph Hellwig <[email protected]>
Looks ok,
--D
Quoted 277 lines
--- fs/xfs/xfs_bmap_item.c | 12 ++++----- fs/xfs/xfs_bmap_item.h | 3 +-- fs/xfs/xfs_log_recover.c | 51 ++++++++++++++++++++++---------------- fs/xfs/xfs_refcount_item.c | 12 ++++----- fs/xfs/xfs_refcount_item.h | 3 +-- 5 files changed, 43 insertions(+), 38 deletions(-)fs/xfs/xfs_bmap_item.c +6
-6@@ -375,9 +375,8 @@ xfs_bud_init(*/ int xfs_bui_recover ( -struct xfs_mount *mp, -struct xfs_bui_log_item *buip, -struct xfs_defer_ops *dfops) +struct xfs_trans *parent_tp, +struct xfs_bui_log_item *buip) {int error =0 ;unsigned int bui_type;@@ -393,6 +392,7 @@ xfs_bui_recover(struct xfs_trans *tp;struct xfs_inode *ip =NULL ;struct xfs_bmbt_irec irec; +struct xfs_mount *mp = parent_tp->t_mountp;ASSERT (!test_bit (XFS_BUI_RECOVERED , &buip->bui_flags));@@ -446,7 +446,7 @@ xfs_bui_recover(* finishes them on completion. Transfer current dfops state to this * transaction and transfer the result back before we return. */ -xfs_defer_move (tp->t_dfops, dfops); +xfs_defer_move (tp->t_dfops, parent_tp->t_dfops); budp =xfs_trans_get_bud (tp, buip);/* Grab the inode. */ @@ -494,7 +494,7 @@ xfs_bui_recover( }set_bit (XFS_BUI_RECOVERED , &buip->bui_flags); -xfs_defer_move (dfops, tp->t_dfops); +xfs_defer_move (parent_tp->t_dfops, tp->t_dfops); error =xfs_trans_commit (tp);xfs_iunlock (ip,XFS_ILOCK_EXCL );xfs_irele (ip);@@ -502,7 +502,7 @@ xfs_bui_recover(return error; err_inode: -xfs_defer_move (dfops, tp->t_dfops); +xfs_defer_move (parent_tp->t_dfops, tp->t_dfops);xfs_trans_cancel (tp);if (ip) {xfs_iunlock (ip,XFS_ILOCK_EXCL );fs/xfs/xfs_bmap_item.h +1
-2@@ -79,7 +79,6 @@ struct xfs_bud_log_item *xfs_bud_init(struct xfs_mount *, struct xfs_bui_log_item *);void xfs_bui_item_free (struct xfs_bui_log_item *);void xfs_bui_release (struct xfs_bui_log_item *); -int xfs_bui_recover (struct xfs_mount *mp,struct xfs_bui_log_item *buip, -struct xfs_defer_ops *dfops); +int xfs_bui_recover (struct xfs_trans *parent_tp,struct xfs_bui_log_item *buip);#endif /* __XFS_BMAP_ITEM_H__ */ fs/xfs/xfs_log_recover.c +29
-22@@ -4733,10 +4733,9 @@ xlog_recover_cancel_rui(/* Recover the CUI if necessary. */ STATIC intxlog_recover_process_cui ( -struct xfs_mount *mp, +struct xfs_trans *parent_tp,struct xfs_ail *ailp, -struct xfs_log_item *lip, -struct xfs_defer_ops *dfops) +struct xfs_log_item *lip) {struct xfs_cui_log_item *cuip;int error;@@ -4749,7 +4748,7 @@ xlog_recover_process_cui(return 0 ;spin_unlock (&ailp->ail_lock); - error =xfs_cui_recover (mp, cuip, dfops); + error =xfs_cui_recover (parent_tp, cuip);spin_lock (&ailp->ail_lock);return error;@@ -4774,10 +4773,9 @@ xlog_recover_cancel_cui(/* Recover the BUI if necessary. */ STATIC intxlog_recover_process_bui ( -struct xfs_mount *mp, +struct xfs_trans *parent_tp,struct xfs_ail *ailp, -struct xfs_log_item *lip, -struct xfs_defer_ops *dfops) +struct xfs_log_item *lip) {struct xfs_bui_log_item *buip;int error;@@ -4790,7 +4788,7 @@ xlog_recover_process_bui(return 0 ;spin_unlock (&ailp->ail_lock); - error =xfs_bui_recover (mp, buip, dfops); + error =xfs_bui_recover (parent_tp, buip);spin_lock (&ailp->ail_lock);return error;@@ -4829,9 +4827,9 @@ static inline bool xlog_item_is_intent(struct xfs_log_item *lip)/* Take all the collected deferred ops and finish them in order. */ static int xlog_finish_defer_ops ( -struct xfs_mount *mp, -struct xfs_defer_ops *dfops) +struct xfs_trans *parent_tp) { +struct xfs_mount *mp = parent_tp->t_mountp;struct xfs_trans *tp;int64_t freeblks;uint resblks;@@ -4855,7 +4853,7 @@ xlog_finish_defer_ops(if (error)return error;/* transfer all collected dfops to this transaction */ -xfs_defer_move (tp->t_dfops, dfops); +xfs_defer_move (tp->t_dfops, parent_tp->t_dfops);return xfs_trans_commit (tp); }@@ -4880,22 +4878,34 @@ STATIC intxlog_recover_process_intents ( struct xlog *log) { -struct xfs_defer_ops dfops; +struct xfs_trans *parent_tp;struct xfs_ail_cursor cur;struct xfs_log_item *lip;struct xfs_ail *ailp; -int error =0 ; +int error;#if defined(DEBUG ) || defined(XFS_WARN )xfs_lsn_t last_lsn;#endif +/* +* The intent recovery handlers commit transactions to complete recovery +* for individual intents, but any new deferred operations that are +* queued during that process are held off until the very end. The +* purpose of this transaction is to serve as a container for deferred +* operations. Each intent recovery handler must transfer dfops here +* before its local transaction commits, and we'll finish the entire +* list below. +*/ + error =xfs_trans_alloc_empty (log->l_mp, &parent_tp); +if (error) +return error; + ailp = log->l_ailp;spin_lock (&ailp->ail_lock); lip =xfs_trans_ail_cursor_first (ailp, &cur,0 );#if defined(DEBUG ) || defined(XFS_WARN ) last_lsn =xlog_assign_lsn (log->l_curr_cycle, log->l_curr_block);#endif -xfs_defer_init (NULL , &dfops);while (lip !=NULL ) {/* * We're done when we see something other than an intent. @@ -4930,12 +4940,10 @@ xlog_recover_process_intents( error =xlog_recover_process_rui (log->l_mp, ailp, lip);break ;case XFS_LI_CUI : - error =xlog_recover_process_cui (log->l_mp, ailp, lip, - &dfops); + error =xlog_recover_process_cui (parent_tp, ailp, lip);break ;case XFS_LI_BUI : - error =xlog_recover_process_bui (log->l_mp, ailp, lip, - &dfops); + error =xlog_recover_process_bui (parent_tp, ailp, lip);break ; }if (error)@@ -4945,10 +4953,9 @@ xlog_recover_process_intents( out:xfs_trans_ail_cursor_done (&cur);spin_unlock (&ailp->ail_lock); -if (error) -__xfs_defer_cancel (&dfops); -else - error =xlog_finish_defer_ops (log->l_mp, &dfops); +if (!error) + error =xlog_finish_defer_ops (parent_tp); +xfs_trans_cancel (parent_tp);return error; }fs/xfs/xfs_refcount_item.c +6
-6@@ -380,9 +380,8 @@ xfs_cud_init(*/ int xfs_cui_recover ( -struct xfs_mount *mp, -struct xfs_cui_log_item *cuip, -struct xfs_defer_ops *dfops) +struct xfs_trans *parent_tp, +struct xfs_cui_log_item *cuip) {int i;int error =0 ;@@ -398,6 +397,7 @@ xfs_cui_recover(xfs_extlen_t new_len;struct xfs_bmbt_irec irec;bool requeue_only = false; +struct xfs_mount *mp = parent_tp->t_mountp;ASSERT (!test_bit (XFS_CUI_RECOVERED , &cuip->cui_flags));@@ -457,7 +457,7 @@ xfs_cui_recover(* finishes them on completion. Transfer current dfops state to this * transaction and transfer the result back before we return. */ -xfs_defer_move (tp->t_dfops, dfops); +xfs_defer_move (tp->t_dfops, parent_tp->t_dfops); cudp =xfs_trans_get_cud (tp, cuip);for (i =0 ; i < cuip->cui_format.cui_nextents; i++) {@@ -522,13 +522,13 @@ xfs_cui_recover(xfs_refcount_finish_one_cleanup (tp, rcur, error);set_bit (XFS_CUI_RECOVERED , &cuip->cui_flags); -xfs_defer_move (dfops, tp->t_dfops); +xfs_defer_move (parent_tp->t_dfops, tp->t_dfops); error =xfs_trans_commit (tp);return error; abort_error:xfs_refcount_finish_one_cleanup (tp, rcur, error); -xfs_defer_move (dfops, tp->t_dfops); +xfs_defer_move (parent_tp->t_dfops, tp->t_dfops);xfs_trans_cancel (tp);return error; }fs/xfs/xfs_refcount_item.h +1
-2@@ -82,7 +82,6 @@ struct xfs_cud_log_item *xfs_cud_init(struct xfs_mount *, struct xfs_cui_log_item *);void xfs_cui_item_free (struct xfs_cui_log_item *);void xfs_cui_release (struct xfs_cui_log_item *); -int xfs_cui_recover (struct xfs_mount *mp,struct xfs_cui_log_item *cuip, -struct xfs_defer_ops *dfops); +int xfs_cui_recover (struct xfs_trans *parent_tp,struct xfs_cui_log_item *cuip);#endif /* __XFS_REFCOUNT_ITEM_H__ */ Signature · 6 lines
2.17.1 -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v2 3/15 xfs: remove unused __xfs_defer_cancel() internal helper
In reply to Brian FosterWith no more external dfops users, there is no need for an xfs_defer_ops cancel wrapper.
--- fs/xfs/libxfs/xfs_defer.c | 5 +++-- fs/xfs/libxfs/xfs_defer.h | 2 +- fs/xfs/xfs_trans.c | 7 ------- fs/xfs/xfs_trans.h | 3 --- 4 files changed, 4 insertions(+), 13 deletions(-)
fs/xfs/libxfs/xfs_defer.c +3 -2
@@ -465,9 +465,10 @@ xfs_defer_finish(* Free up any items left in the list. */ void -__xfs_defer_cancel ( -struct xfs_defer_ops *dop) +xfs_defer_cancel ( +struct xfs_trans *tp) { +struct xfs_defer_ops *dop = tp->t_dfops;struct xfs_defer_pending *dfp;struct xfs_defer_pending *pli;struct list_head *pwi;
fs/xfs/libxfs/xfs_defer.h +1 -1
@@ -50,7 +50,7 @@ void xfs_defer_add(struct xfs_defer_ops *dop, enum xfs_defer_ops_type type, struct list_head *h);int xfs_defer_finish_noroll (struct xfs_trans **tp);int xfs_defer_finish (struct xfs_trans **tp); -void __xfs_defer_cancel (struct xfs_defer_ops *dop); +void xfs_defer_cancel (struct xfs_trans *);void xfs_defer_init (struct xfs_trans *tp,struct xfs_defer_ops *dop);bool xfs_defer_has_unfinished_work (struct xfs_defer_ops *dop);int xfs_defer_ijoin (struct xfs_defer_ops *dop,struct xfs_inode *ip);
fs/xfs/xfs_trans.c +0 -7
@@ -1110,10 +1110,3 @@ xfs_trans_roll( tres.tr_logflags =XFS_TRANS_PERM_LOG_RES ;return xfs_trans_reserve (*tpp, &tres,0 ,0 ); } - -void -xfs_defer_cancel ( -struct xfs_trans *tp) -{ -__xfs_defer_cancel (tp->t_dfops); -}
fs/xfs/xfs_trans.h +0 -3
@@ -214,9 +214,6 @@ xfs_trans_read_buf( flags, bpp, ops); } -/* cancel dfops associated with a transaction */ -void xfs_defer_cancel (struct xfs_trans *); -struct xfs_buf *xfs_trans_getsb (xfs_trans_t *,struct xfs_mount *,int );void xfs_trans_brelse (xfs_trans_t *,struct xfs_buf *);
Signature · 1 line
2.17.1
PATCH v2 4/15 xfs: pass transaction to dfops reset/move helpers
In reply to Brian FosterAll callers pass ->t_dfops of the associated transactions. Refactor the helpers to receive the transactions and facilitate further cleanups between xfs_defer_ops and xfs_trans.
--- fs/xfs/libxfs/xfs_defer.c | 15 ++++++++++----- fs/xfs/libxfs/xfs_defer.h | 2 +- fs/xfs/xfs_bmap_item.c | 6 +++--- fs/xfs/xfs_log_recover.c | 2 +- fs/xfs/xfs_refcount_item.c | 6 +++--- fs/xfs/xfs_trans.c | 2 +- 6 files changed, 19 insertions(+), 14 deletions(-)
fs/xfs/libxfs/xfs_defer.c +10 -5
@@ -324,9 +324,12 @@ xfs_defer_bjoin(*/ static void xfs_defer_reset ( -struct xfs_defer_ops *dop) +struct xfs_trans *tp) { +struct xfs_defer_ops *dop = tp->t_dfops; +ASSERT (!xfs_defer_has_unfinished_work (dop)); + dop->dop_low = false;memset (dop->dop_inodes,0 ,sizeof (dop->dop_inodes));memset (dop->dop_bufs,0 ,sizeof (dop->dop_bufs));
@@ -457,7 +460,7 @@ xfs_defer_finish(if (error)return error; } -xfs_defer_reset ((*tp)->t_dfops); +xfs_defer_reset (*tp);return 0 ; }
@@ -575,9 +578,11 @@ xfs_defer_init(*/ void xfs_defer_move ( -struct xfs_defer_ops *dst, -struct xfs_defer_ops *src) +struct xfs_trans *dtp, +struct xfs_trans *stp) { +struct xfs_defer_ops *dst = dtp->t_dfops; +struct xfs_defer_ops *src = stp->t_dfops;ASSERT (dst != src);list_splice_init (&src->dop_intake, &dst->dop_intake);
@@ -587,5 +592,5 @@ xfs_defer_move(memcpy (dst->dop_bufs, src->dop_bufs,sizeof (dst->dop_bufs)); dst->dop_low = src->dop_low; -xfs_defer_reset (src); +xfs_defer_reset (stp); }
fs/xfs/libxfs/xfs_defer.h +1 -1
@@ -55,7 +55,7 @@ void xfs_defer_init(struct xfs_trans *tp, struct xfs_defer_ops *dop);bool xfs_defer_has_unfinished_work (struct xfs_defer_ops *dop);int xfs_defer_ijoin (struct xfs_defer_ops *dop,struct xfs_inode *ip);int xfs_defer_bjoin (struct xfs_defer_ops *dop,struct xfs_buf *bp); -void xfs_defer_move (struct xfs_defer_ops *dst,struct xfs_defer_ops *src); +void xfs_defer_move (struct xfs_trans *dtp,struct xfs_trans *stp);/* Description of a deferred type. */ struct xfs_defer_op_type {
fs/xfs/xfs_bmap_item.c +3 -3
@@ -446,7 +446,7 @@ xfs_bui_recover(* finishes them on completion. Transfer current dfops state to this * transaction and transfer the result back before we return. */ -xfs_defer_move (tp->t_dfops, parent_tp->t_dfops); +xfs_defer_move (tp, parent_tp); budp =xfs_trans_get_bud (tp, buip);/* Grab the inode. */
@@ -494,7 +494,7 @@ xfs_bui_recover( }set_bit (XFS_BUI_RECOVERED , &buip->bui_flags); -xfs_defer_move (parent_tp->t_dfops, tp->t_dfops); +xfs_defer_move (parent_tp, tp); error =xfs_trans_commit (tp);xfs_iunlock (ip,XFS_ILOCK_EXCL );xfs_irele (ip);
@@ -502,7 +502,7 @@ xfs_bui_recover(return error; err_inode: -xfs_defer_move (parent_tp->t_dfops, tp->t_dfops); +xfs_defer_move (parent_tp, tp);xfs_trans_cancel (tp);if (ip) {xfs_iunlock (ip,XFS_ILOCK_EXCL );
fs/xfs/xfs_log_recover.c +1 -1
@@ -4853,7 +4853,7 @@ xlog_finish_defer_ops(if (error)return error;/* transfer all collected dfops to this transaction */ -xfs_defer_move (tp->t_dfops, parent_tp->t_dfops); +xfs_defer_move (tp, parent_tp);return xfs_trans_commit (tp); }
fs/xfs/xfs_refcount_item.c +3 -3
@@ -457,7 +457,7 @@ xfs_cui_recover(* finishes them on completion. Transfer current dfops state to this * transaction and transfer the result back before we return. */ -xfs_defer_move (tp->t_dfops, parent_tp->t_dfops); +xfs_defer_move (tp, parent_tp); cudp =xfs_trans_get_cud (tp, cuip);for (i =0 ; i < cuip->cui_format.cui_nextents; i++) {
@@ -522,13 +522,13 @@ xfs_cui_recover(xfs_refcount_finish_one_cleanup (tp, rcur, error);set_bit (XFS_CUI_RECOVERED , &cuip->cui_flags); -xfs_defer_move (parent_tp->t_dfops, tp->t_dfops); +xfs_defer_move (parent_tp, tp); error =xfs_trans_commit (tp);return error; abort_error:xfs_refcount_finish_one_cleanup (tp, rcur, error); -xfs_defer_move (parent_tp->t_dfops, tp->t_dfops); +xfs_defer_move (parent_tp, tp);xfs_trans_cancel (tp);return error; }
fs/xfs/xfs_trans.c +1 -1
@@ -125,7 +125,7 @@ xfs_trans_dup(if (tp->t_dfops != &tp->t_dfops_internal) ntp->t_dfops = tp->t_dfops;else -xfs_defer_move (ntp->t_dfops, tp->t_dfops); +xfs_defer_move (ntp, tp);xfs_trans_dup_dqinfo (tp, ntp);
Signature · 1 line
2.17.1
PATCH v2 5/15 xfs: replace dop_low with transaction flag
In reply to Brian FosterThe dop_low field enables the low free space allocation mode when a previous allocation has detected difficulty allocating blocks. It has historically been part of the xfs_defer_ops structure, which means if enabled, it remains enabled across a set of transactions until the deferred operations have completed and the dfops is reset.
Now that the dfops is embedded in the transaction, we can save a bit more space by using a transaction flag rather than a standalone boolean. Drop the ->dop_low field and replace it with a transaction flag that is set at the same points, carried across rolling transactions and cleared on completion of deferred operations. This essentially emulates the behavior of ->dop_low and so should not change behavior.
--- fs/xfs/libxfs/xfs_bmap.c | 8 ++++---- fs/xfs/libxfs/xfs_bmap_btree.c | 4 ++-- fs/xfs/libxfs/xfs_defer.c | 16 ++++++++++++++-- fs/xfs/libxfs/xfs_defer.h | 11 ----------- fs/xfs/libxfs/xfs_shared.h | 12 ++++++++++++ fs/xfs/xfs_filestream.c | 3 ++- fs/xfs/xfs_trace.h | 10 ++-------- fs/xfs/xfs_trans.h | 2 -- 8 files changed, 36 insertions(+), 30 deletions(-)
fs/xfs/libxfs/xfs_bmap.c +4 -4
@@ -700,7 +700,7 @@ xfs_bmap_extents_to_btree(if (tp->t_firstblock ==NULLFSBLOCK ) { args.type =XFS_ALLOCTYPE_START_BNO ; args.fsbno =XFS_INO_TO_FSB (mp, ip->i_ino); - }else if (tp->t_dfops->dop_low) { + }else if (tp->t_flags &XFS_TRANS_LOWMODE ) { args.type =XFS_ALLOCTYPE_START_BNO ; args.fsbno = tp->t_firstblock; }else {
@@ -3449,7 +3449,7 @@ xfs_bmap_btalloc( error =xfs_bmap_btalloc_nullfb (ap, &args, &blen);if (error)return error; - }else if (ap->tp->t_dfops->dop_low) { + }else if (ap->tp->t_flags &XFS_TRANS_LOWMODE ) {if (xfs_inode_is_filestream (ap->ip)) args.type =XFS_ALLOCTYPE_FIRST_AG ;else
@@ -3484,7 +3484,7 @@ xfs_bmap_btalloc(* is >= the stripe unit and the allocation offset is * at the end of file. */ -if (!ap->tp->t_dfops->dop_low && ap->aeof) { +if (!(ap->tp->t_flags &XFS_TRANS_LOWMODE ) && ap->aeof) {if (!ap->offset) { args.alignment = stripe_align; atype = args.type;
@@ -3576,7 +3576,7 @@ xfs_bmap_btalloc( args.total = ap->minlen;if ((error =xfs_alloc_vextent (&args)))return error; - ap->tp->t_dfops->dop_low = true; + ap->tp->t_flags |=XFS_TRANS_LOWMODE ; }if (args.fsbno !=NULLFSBLOCK ) {/*
fs/xfs/libxfs/xfs_bmap_btree.c +2 -2
@@ -226,7 +226,7 @@ xfs_bmbt_alloc_block(* block allocation here and corrupt the filesystem. */ args.minleft = args.tp->t_blk_res; - }else if (cur->bc_tp->t_dfops->dop_low) { + }else if (cur->bc_tp->t_flags &XFS_TRANS_LOWMODE ) { args.type =XFS_ALLOCTYPE_START_BNO ; }else { args.type =XFS_ALLOCTYPE_NEAR_BNO ;
@@ -253,7 +253,7 @@ xfs_bmbt_alloc_block( error =xfs_alloc_vextent (&args);if (error) goto error0; - cur->bc_tp->t_dfops->dop_low = true; + cur->bc_tp->t_flags |=XFS_TRANS_LOWMODE ; }if (WARN_ON_ONCE (args.fsbno ==NULLFSBLOCK )) { *stat =0 ;
fs/xfs/libxfs/xfs_defer.c +14 -2
@@ -330,9 +330,14 @@ xfs_defer_reset(ASSERT (!xfs_defer_has_unfinished_work (dop)); - dop->dop_low = false;memset (dop->dop_inodes,0 ,sizeof (dop->dop_inodes));memset (dop->dop_bufs,0 ,sizeof (dop->dop_bufs)); + +/* +* Low mode state transfers across transaction rolls to mirror dfops +* lifetime. Clear it now that dfops is reset. +*/ + tp->t_flags &= ~XFS_TRANS_LOWMODE ; }/*
@@ -590,7 +595,14 @@ xfs_defer_move(memcpy (dst->dop_inodes, src->dop_inodes,sizeof (dst->dop_inodes));memcpy (dst->dop_bufs, src->dop_bufs,sizeof (dst->dop_bufs)); - dst->dop_low = src->dop_low; + +/* +* Low free space mode was historically controlled by a dfops field. +* This meant that low mode state potentially carried across multiple +* transaction rolls. Transfer low mode on a dfops move to preserve +* that behavior. +*/ + dtp->t_flags |= (stp->t_flags &XFS_TRANS_LOWMODE );xfs_defer_reset (stp); }
fs/xfs/libxfs/xfs_defer.h +0 -11
@@ -25,17 +25,6 @@ struct xfs_defer_pending {/* * Header for deferred operation list. -* -* dop_low is used by the allocator to activate the lowspace algorithm - -* when free space is running low the extent allocator may choose to -* allocate an extent from an AG without leaving sufficient space for -* a btree split when inserting the new extent. In this case the allocator -* will enable the lowspace algorithm which is supposed to allow further -* allocations (such as btree splits and newroots) to allocate from -* sequential AGs. In order to avoid locking AGs out of order the lowspace -* algorithm will start searching for free space from AG 0. If the correct -* transaction reservations have been made then this algorithm will eventually -* find all the space it needs. */ enum xfs_defer_ops_type {XFS_DEFER_OPS_TYPE_BMAP ,
fs/xfs/libxfs/xfs_shared.h +12 -0
@@ -64,6 +64,18 @@ void xfs_log_get_max_trans_res(struct xfs_mount *mp,#define XFS_TRANS_RESERVE 0x20/* OK to use reserved data blocks */ #define XFS_TRANS_NO_WRITECOUNT 0x40/* do not elevate SB writecount */ #define XFS_TRANS_NOFS 0x80/* pass KM_NOFS to kmem_alloc */ +/* +* LOWMODE is used by the allocator to activate the lowspace algorithm - when +* free space is running low the extent allocator may choose to allocate an +* extent from an AG without leaving sufficient space for a btree split when +* inserting the new extent. In this case the allocator will enable the +* lowspace algorithm which is supposed to allow further allocations (such as +* btree splits and newroots) to allocate from sequential AGs. In order to +* avoid locking AGs out of order the lowspace algorithm will start searching +* for free space from AG 0. If the correct transaction reservations have been +* made then this algorithm will eventually find all the space it needs. +*/ +#define XFS_TRANS_LOWMODE 0x100/* allocate in low space mode */ /* * Field values for xfs_trans_mod_sb.
fs/xfs/xfs_filestream.c +2 -1
@@ -20,6 +20,7 @@#include "xfs_trace.h" #include "xfs_ag_resv.h" #include "xfs_trans.h" +#include "xfs_shared.h" struct xfs_fstrm_item {struct xfs_mru_cache_elem mru;
@@ -378,7 +379,7 @@ xfs_filestream_new_ag(if (xfs_alloc_is_userdata (ap->datatype)) flags |=XFS_PICK_USERDATA ; -if (ap->tp->t_dfops->dop_low) +if (ap->tp->t_flags &XFS_TRANS_LOWMODE ) flags |=XFS_PICK_LOWSPACE ; err =xfs_filestream_pick_ag (pip, startag, agp, flags, minlen);
fs/xfs/xfs_trace.h +2 -8
@@ -2223,19 +2223,16 @@ DECLARE_EVENT_CLASS(xfs_defer_class, __field(dev_t, dev) __field(void *, dop) __field(char, committed) - __field(char, low) __field(unsigned long, caller_ip) ),TP_fast_assign ( __entry->dev = mp ? mp->m_super->s_dev : 0; __entry->dop = dop; - __entry->low = dop->dop_low; __entry->caller_ip = caller_ip; ), - TP_printk("dev %d:%d ops %p low %d, caller %pS", + TP_printk("dev %d:%d ops %p caller %pS", MAJOR(__entry->dev), MINOR(__entry->dev), __entry->dop, - __entry->low, (char *)__entry->caller_ip) )#define DEFINE_DEFER_EVENT (name) \
@@ -2251,19 +2248,16 @@ DECLARE_EVENT_CLASS(xfs_defer_error_class, __field(dev_t, dev) __field(void *, dop) __field(char, committed) - __field(char, low) __field(int, error) ),TP_fast_assign ( __entry->dev = mp ? mp->m_super->s_dev : 0; __entry->dop = dop; - __entry->low = dop->dop_low; __entry->error = error; ), - TP_printk("dev %d:%d ops %p low %d err %d", + TP_printk("dev %d:%d ops %p err %d", MAJOR(__entry->dev), MINOR(__entry->dev), __entry->dop, - __entry->low, __entry->error) )#define DEFINE_DEFER_ERROR_EVENT (name) \
fs/xfs/xfs_trans.h +0 -2
@@ -101,8 +101,6 @@ struct xfs_defer_ops {/* relog these with each roll */ struct xfs_inode *dop_inodes[XFS_DEFER_OPS_NR_INODES ];struct xfs_buf *dop_bufs[XFS_DEFER_OPS_NR_BUFS ]; - -bool dop_low;/* alloc in low mode */ };/*
Signature · 1 line
2.17.1
PATCH v2 6/15 xfs: add missing defer ijoins for held inodes
In reply to Brian FosterLog items that require relogging during deferred operations processing are explicitly joined to the associated dfops via the xfs_defer_*join() helpers. These calls imply that the associated object is "held" by the transaction such that when rolled, the item can be immediately joined to a follow up transaction. For buffers, this means the buffer remains locked and held after each roll. For inodes, this means that the inode remains locked.
Failure to join a held item to the dfops structure means the associated object pins the tail of the log while dfops processing completes, because the item never relogs and is not unlocked or released until deferred processing completes.
Currently, all buffers that are held in transactions (XFS_BLI_HOLD) with deferred operations are explicitly joined to the dfops. This is not the case for inodes, however, as various contexts defer operations to transactions with held inodes without explicit joins to the associated dfops (and thus not relogging).
While this is not a catastrophic problem, it is not ideal. Given that we want to eventually relog such items automatically during dfops processing, start by explicitly adding these missing xfs_defer_ijoin() calls. A call is added everywhere an inode is joined to a transaction without transferring lock ownership and said transaction runs deferred operations.
All xfs_defer_ijoin() calls will eventually be replaced by automatic dfops inode relogging. This patch essentially implements the behavior change that would otherwise occur due to automatic inode dfops relogging.
--- fs/xfs/libxfs/xfs_bmap.c | 1 + fs/xfs/xfs_bmap_util.c | 1 + fs/xfs/xfs_inode.c | 1 + fs/xfs/xfs_iomap.c | 3 +++ fs/xfs/xfs_reflink.c | 1 + 5 files changed, 7 insertions(+)
fs/xfs/libxfs/xfs_bmap.c +1 -0
@@ -1119,6 +1119,7 @@ xfs_bmap_add_attrfork(xfs_log_sb (tp); } +xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_trans_commit (tp);xfs_iunlock (ip,XFS_ILOCK_EXCL );return error;
fs/xfs/xfs_bmap_util.c +1 -0
@@ -979,6 +979,7 @@ xfs_alloc_file_space(/* * Complete the transaction */ +xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_trans_commit (tp);xfs_iunlock (ip,XFS_ILOCK_EXCL );if (error)
fs/xfs/xfs_inode.c +1 -0
@@ -1810,6 +1810,7 @@ xfs_inactive_ifree(* Just ignore errors at this point. There is nothing we can do except * to try to keep going. Make sure it's not a silent error. */ +xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_trans_commit (tp);if (error) xfs_notice(mp, "%s: xfs_trans_commit returned error %d",
fs/xfs/xfs_iomap.c +3 -0
@@ -261,6 +261,7 @@ xfs_iomap_write_direct(/* * Complete the transaction */ +xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_trans_commit (tp);if (error) goto out_unlock;
@@ -763,6 +764,7 @@ xfs_iomap_write_allocate(if (error) goto trans_cancel; +xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_trans_commit (tp);if (error) goto error0;
@@ -882,6 +884,7 @@ xfs_iomap_write_unwritten(xfs_trans_log_inode (tp, ip,XFS_ILOG_CORE ); } +xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_trans_commit (tp);xfs_iunlock (ip,XFS_ILOCK_EXCL );if (error)
fs/xfs/xfs_reflink.c +1 -0
@@ -435,6 +435,7 @@ xfs_reflink_allocate_cow(xfs_inode_set_cowblocks_tag (ip);/* Finish up. */ +xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_trans_commit (tp);if (error)return error;
Signature · 1 line
2.17.1
PATCH v2 7/15 xfs: automatic dfops buffer relogging
In reply to Brian FosterBuffers that are held across deferred operations are explicitly joined to the dfops structure to ensure appropriate relogging. While buffers are currently joined explicitly, we can detect the conditions that require relogging at dfops finish time by inspecting the transaction item list for held buffers.
Replace the xfs_defer_bjoin() infrastructure with such detection and automatic relogging of held buffers. This eliminates the need for the per-dfops buffer list, replaced by an on-stack variant in xfs_defer_trans_roll().
--- fs/xfs/libxfs/xfs_attr.c | 1 - fs/xfs/libxfs/xfs_defer.c | 58 ++++++++++++++++++--------------------- fs/xfs/libxfs/xfs_defer.h | 1 - fs/xfs/xfs_dquot.c | 1 - fs/xfs/xfs_trans.h | 1 - 5 files changed, 26 insertions(+), 36 deletions(-)
fs/xfs/libxfs/xfs_attr.c +0 -1
@@ -320,7 +320,6 @@ xfs_attr_set(* buffer and run into problems with the write verifier. */ xfs_trans_bhold (args.trans, leaf_bp); -xfs_defer_bjoin (args.trans->t_dfops, leaf_bp);xfs_defer_ijoin (args.trans->t_dfops, dp); error =xfs_defer_finish (&args.trans);if (error)
fs/xfs/libxfs/xfs_defer.c +26 -32
100 lines not on this page · raw message
fs/xfs/libxfs/xfs_defer.h +0 -1
@@ -43,7 +43,6 @@ void xfs_defer_cancel(struct xfs_trans *);void xfs_defer_init (struct xfs_trans *tp,struct xfs_defer_ops *dop);bool xfs_defer_has_unfinished_work (struct xfs_defer_ops *dop);int xfs_defer_ijoin (struct xfs_defer_ops *dop,struct xfs_inode *ip); -int xfs_defer_bjoin (struct xfs_defer_ops *dop,struct xfs_buf *bp);void xfs_defer_move (struct xfs_trans *dtp,struct xfs_trans *stp);/* Description of a deferred type. */
fs/xfs/xfs_dquot.c +0 -1
@@ -362,7 +362,6 @@ xfs_dquot_disk_alloc(* manually or by committing the transaction. */ xfs_trans_bhold (tp, bp); - error =xfs_defer_bjoin (tp->t_dfops, bp);if (error) {xfs_trans_bhold_release (tp, bp);xfs_trans_brelse (tp, bp);
fs/xfs/xfs_trans.h +0 -1
@@ -100,7 +100,6 @@ struct xfs_defer_ops {/* relog these with each roll */ struct xfs_inode *dop_inodes[XFS_DEFER_OPS_NR_INODES ]; -struct xfs_buf *dop_bufs[XFS_DEFER_OPS_NR_BUFS ]; };/*
Signature · 1 line
2.17.1
Re: [PATCH v2 07/15] xfs: automatic dfops buffer relogging
Quoted 6 lines · Brian Foster
Buffers that are held across deferred operations are explicitly joined to the dfops structure to ensure appropriate relogging. While buffers are currently joined explicitly, we can detect the conditions that require relogging at dfops finish time by inspecting the transaction item list for held buffers.
Quoted 7 lines
Replace the
xfs_defer_bjoin()infrastructure with such detection and automatic relogging of held buffers. This eliminates the need for the per-dfops buffer list, replaced by an on-stack variant inxfs_defer_trans_roll().Signed-off-by Brian Foster <[email protected]>Reviewed-by Christoph Hellwig <[email protected]>
Looks ok,
--D
Quoted 174 lines
--- fs/xfs/libxfs/xfs_attr.c | 1 - fs/xfs/libxfs/xfs_defer.c | 58 ++++++++++++++++++--------------------- fs/xfs/libxfs/xfs_defer.h | 1 - fs/xfs/xfs_dquot.c | 1 - fs/xfs/xfs_trans.h | 1 - 5 files changed, 26 insertions(+), 36 deletions(-)fs/xfs/libxfs/xfs_attr.c +0
-1@@ -320,7 +320,6 @@ xfs_attr_set(* buffer and run into problems with the write verifier. */ xfs_trans_bhold (args.trans, leaf_bp); -xfs_defer_bjoin (args.trans->t_dfops, leaf_bp);xfs_defer_ijoin (args.trans->t_dfops, dp); error =xfs_defer_finish (&args.trans);if (error)fs/xfs/libxfs/xfs_defer.c +26
-32@@ -14,6 +14,7 @@#include "xfs_mount.h" #include "xfs_defer.h" #include "xfs_trans.h" +#include "xfs_buf_item.h" #include "xfs_trace.h" /* @@ -228,6 +229,10 @@ xfs_defer_trans_roll( struct xfs_trans **tp) {struct xfs_defer_ops *dop = (*tp)->t_dfops; +struct xfs_buf_log_item *bli; +struct xfs_log_item *lip; +struct xfs_buf *bplist[XFS_DEFER_OPS_NR_BUFS ]; +int bpcount =0 ;int i;int error;@@ -235,9 +240,24 @@ xfs_defer_trans_roll(for (i =0 ; i <XFS_DEFER_OPS_NR_INODES && dop->dop_inodes[i]; i++)xfs_trans_log_inode (*tp, dop->dop_inodes[i],XFS_ILOG_CORE ); -/* Hold the (previously bjoin'd) buffer locked across the roll. */ -for (i =0 ; i <XFS_DEFER_OPS_NR_BUFS && dop->dop_bufs[i]; i++) -xfs_trans_dirty_buf (*tp, dop->dop_bufs[i]); +list_for_each_entry (lip, &(*tp)->t_items, li_trans) { +switch (lip->li_type) { +case XFS_LI_BUF : + bli = container_of(lip, struct xfs_buf_log_item, + bli_item); +if (bli->bli_flags &XFS_BLI_HOLD ) { +if (bpcount >=XFS_DEFER_OPS_NR_BUFS ) { +ASSERT (0 ); +return -EFSCORRUPTED ; + } +xfs_trans_dirty_buf (*tp, bli->bli_buf); + bplist[bpcount++] = bli->bli_buf; + } +break ; +default : +break ; + } + }trace_xfs_defer_trans_roll ((*tp)->t_mountp, dop, _RET_IP_);@@ -255,9 +275,9 @@ xfs_defer_trans_roll(xfs_trans_ijoin (*tp, dop->dop_inodes[i],0 );/* Rejoin the buffers and dirty them so the log moves forward. */ -for (i =0 ; i <XFS_DEFER_OPS_NR_BUFS && dop->dop_bufs[i]; i++) { -xfs_trans_bjoin (*tp, dop->dop_bufs[i]); -xfs_trans_bhold (*tp, dop->dop_bufs[i]); +for (i =0 ; i < bpcount; i++) { +xfs_trans_bjoin (*tp, bplist[i]); +xfs_trans_bhold (*tp, bplist[i]); }return error;@@ -295,30 +315,6 @@ xfs_defer_ijoin(return -EFSCORRUPTED ; } -/* -* Add this buffer to the deferred op. Each joined buffer is relogged -* each time we roll the transaction. -*/ -int -xfs_defer_bjoin ( -struct xfs_defer_ops *dop, -struct xfs_buf *bp) -{ -int i; - -for (i =0 ; i <XFS_DEFER_OPS_NR_BUFS ; i++) { -if (dop->dop_bufs[i] == bp) -return 0 ; -else if (dop->dop_bufs[i] ==NULL ) { - dop->dop_bufs[i] = bp; -return 0 ; - } - } - -ASSERT (0 ); -return -EFSCORRUPTED ; -} -/* * Reset an already used dfops after finish. */ @@ -331,7 +327,6 @@ xfs_defer_reset(ASSERT (!xfs_defer_has_unfinished_work (dop));memset (dop->dop_inodes,0 ,sizeof (dop->dop_inodes)); -memset (dop->dop_bufs,0 ,sizeof (dop->dop_bufs));/* * Low mode state transfers across transaction rolls to mirror dfops @@ -594,7 +589,6 @@ xfs_defer_move(list_splice_init (&src->dop_pending, &dst->dop_pending);memcpy (dst->dop_inodes, src->dop_inodes,sizeof (dst->dop_inodes)); -memcpy (dst->dop_bufs, src->dop_bufs,sizeof (dst->dop_bufs));/* * Low free space mode was historically controlled by a dfops field. fs/xfs/libxfs/xfs_defer.h +0
-1@@ -43,7 +43,6 @@ void xfs_defer_cancel(struct xfs_trans *);void xfs_defer_init (struct xfs_trans *tp,struct xfs_defer_ops *dop);bool xfs_defer_has_unfinished_work (struct xfs_defer_ops *dop);int xfs_defer_ijoin (struct xfs_defer_ops *dop,struct xfs_inode *ip); -int xfs_defer_bjoin (struct xfs_defer_ops *dop,struct xfs_buf *bp);void xfs_defer_move (struct xfs_trans *dtp,struct xfs_trans *stp);/* Description of a deferred type. */ fs/xfs/xfs_dquot.c +0
-1@@ -362,7 +362,6 @@ xfs_dquot_disk_alloc(* manually or by committing the transaction. */ xfs_trans_bhold (tp, bp); - error =xfs_defer_bjoin (tp->t_dfops, bp);if (error) {xfs_trans_bhold_release (tp, bp);xfs_trans_brelse (tp, bp);fs/xfs/xfs_trans.h +0
-1@@ -100,7 +100,6 @@ struct xfs_defer_ops {/* relog these with each roll */ struct xfs_inode *dop_inodes[XFS_DEFER_OPS_NR_INODES ]; -struct xfs_buf *dop_bufs[XFS_DEFER_OPS_NR_BUFS ]; };/* Signature · 6 lines
2.17.1 -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v2 8/15 xfs: automatic dfops inode relogging
In reply to Brian FosterInodes that are held across deferred operations are explicitly joined to the dfops structure to ensure appropriate relogging. While inodes are currently joined explicitly, we can detect the conditions that require relogging at dfops finish time by inspecting the transaction item list for inodes with ili_lock_flags == 0.
Replace the xfs_defer_ijoin() infrastructure with such detection and automatic relogging of held inodes. This eliminates the need for the per-dfops inode list, replaced by an on-stack variant in xfs_defer_trans_roll().
--- fs/xfs/libxfs/xfs_attr.c | 9 ----- fs/xfs/libxfs/xfs_attr_remote.c | 2 -- fs/xfs/libxfs/xfs_bmap.c | 8 ----- fs/xfs/libxfs/xfs_defer.c | 59 ++++++++++++--------------------- fs/xfs/libxfs/xfs_defer.h | 1 - fs/xfs/xfs_bmap_util.c | 4 --- fs/xfs/xfs_inode.c | 2 -- fs/xfs/xfs_iomap.c | 3 -- fs/xfs/xfs_reflink.c | 4 --- fs/xfs/xfs_symlink.c | 1 - fs/xfs/xfs_trans.h | 3 -- 11 files changed, 21 insertions(+), 75 deletions(-)
fs/xfs/libxfs/xfs_attr.c +0 -9
@@ -320,7 +320,6 @@ xfs_attr_set(* buffer and run into problems with the write verifier. */ xfs_trans_bhold (args.trans, leaf_bp); -xfs_defer_ijoin (args.trans->t_dfops, dp); error =xfs_defer_finish (&args.trans);if (error) goto out;
@@ -589,7 +588,6 @@ xfs_attr_leaf_addname( error =xfs_attr3_leaf_to_node (args);if (error) goto out_defer_cancel; -xfs_defer_ijoin (args->trans->t_dfops, dp); error =xfs_defer_finish (&args->trans);if (error) goto out_defer_cancel;
@@ -678,7 +676,6 @@ xfs_attr_leaf_addname(/* bp is gone due to xfs_da_shrink_inode */ if (error) goto out_defer_cancel; -xfs_defer_ijoin (args->trans->t_dfops, dp); error =xfs_defer_finish (&args->trans);if (error) goto out_defer_cancel;
@@ -742,7 +739,6 @@ xfs_attr_leaf_removename(/* bp is gone due to xfs_da_shrink_inode */ if (error) goto out_defer_cancel; -xfs_defer_ijoin (args->trans->t_dfops, dp); error =xfs_defer_finish (&args->trans);if (error) goto out_defer_cancel;
@@ -869,7 +865,6 @@ xfs_attr_node_addname( error =xfs_attr3_leaf_to_node (args);if (error) goto out_defer_cancel; -xfs_defer_ijoin (args->trans->t_dfops, dp); error =xfs_defer_finish (&args->trans);if (error) goto out_defer_cancel;
@@ -894,7 +889,6 @@ xfs_attr_node_addname( error =xfs_da3_split (state);if (error) goto out_defer_cancel; -xfs_defer_ijoin (args->trans->t_dfops, dp); error =xfs_defer_finish (&args->trans);if (error) goto out_defer_cancel;
@@ -991,7 +985,6 @@ xfs_attr_node_addname( error =xfs_da3_join (state);if (error) goto out_defer_cancel; -xfs_defer_ijoin (args->trans->t_dfops, dp); error =xfs_defer_finish (&args->trans);if (error) goto out_defer_cancel;
@@ -1115,7 +1108,6 @@ xfs_attr_node_removename( error =xfs_da3_join (state);if (error) goto out_defer_cancel; -xfs_defer_ijoin (args->trans->t_dfops, dp); error =xfs_defer_finish (&args->trans);if (error) goto out_defer_cancel;
@@ -1147,7 +1139,6 @@ xfs_attr_node_removename(/* bp is gone due to xfs_da_shrink_inode */ if (error) goto out_defer_cancel; -xfs_defer_ijoin (args->trans->t_dfops, dp); error =xfs_defer_finish (&args->trans);if (error) goto out_defer_cancel;
fs/xfs/libxfs/xfs_attr_remote.c +0 -2
@@ -486,7 +486,6 @@ xfs_attr_rmtval_set( &nmap);if (error) goto out_defer_cancel; -xfs_defer_ijoin (args->trans->t_dfops, dp); error =xfs_defer_finish (&args->trans);if (error) goto out_defer_cancel;
@@ -627,7 +626,6 @@ xfs_attr_rmtval_remove( XFS_BMAPI_ATTRFORK, 1, &done);if (error) goto out_defer_cancel; -xfs_defer_ijoin (args->trans->t_dfops, args->dp); error =xfs_defer_finish (&args->trans);if (error) goto out_defer_cancel;
fs/xfs/libxfs/xfs_bmap.c +0 -8
@@ -1119,7 +1119,6 @@ xfs_bmap_add_attrfork(xfs_log_sb (tp); } -xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_trans_commit (tp);xfs_iunlock (ip,XFS_ILOCK_EXCL );return error;
@@ -5987,7 +5986,6 @@ __xfs_bmap_add(int whichfork, struct xfs_bmbt_irec *bmap) { - int error; struct xfs_bmap_intent *bi; trace_xfs_bmap_defer(mp,
@@ -6006,12 +6004,6 @@ __xfs_bmap_add( bi->bi_whichfork = whichfork; bi->bi_bmap = *bmap; - error =xfs_defer_ijoin (dfops, bi->bi_owner); -if (error) { -kmem_free (bi); -return error; - } -xfs_defer_add (dfops,XFS_DEFER_OPS_TYPE_BMAP , &bi->bi_list);return 0 ; }
fs/xfs/libxfs/xfs_defer.c +21 -38
106 lines not on this page · raw message
fs/xfs/libxfs/xfs_defer.h +0 -1
@@ -42,7 +42,6 @@ int xfs_defer_finish(struct xfs_trans **tp);void xfs_defer_cancel (struct xfs_trans *);void xfs_defer_init (struct xfs_trans *tp,struct xfs_defer_ops *dop);bool xfs_defer_has_unfinished_work (struct xfs_defer_ops *dop); -int xfs_defer_ijoin (struct xfs_defer_ops *dop,struct xfs_inode *ip);void xfs_defer_move (struct xfs_trans *dtp,struct xfs_trans *stp);/* Description of a deferred type. */
fs/xfs/xfs_bmap_util.c +0 -4
@@ -979,7 +979,6 @@ xfs_alloc_file_space(/* * Complete the transaction */ -xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_trans_commit (tp);xfs_iunlock (ip,XFS_ILOCK_EXCL );if (error)
@@ -1037,8 +1036,6 @@ xfs_unmap_extent(if (error) goto out_trans_cancel; -xfs_defer_ijoin (tp->t_dfops, ip); - error =xfs_trans_commit (tp); out_unlock:xfs_iunlock (ip,XFS_ILOCK_EXCL );
@@ -1624,7 +1621,6 @@ xfs_swap_extent_rmap(if (error) goto out_defer; -xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_defer_finish (tpp); tp = *tpp;if (error)
fs/xfs/xfs_inode.c +0 -2
@@ -1569,7 +1569,6 @@ xfs_itruncate_extents_flags(* Duplicate the transaction that has the permanent * reservation and commit the old transaction. */ -xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_defer_finish (&tp);if (error) goto out_bmap_cancel;
@@ -1810,7 +1809,6 @@ xfs_inactive_ifree(* Just ignore errors at this point. There is nothing we can do except * to try to keep going. Make sure it's not a silent error. */ -xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_trans_commit (tp);if (error) xfs_notice(mp, "%s: xfs_trans_commit returned error %d",
fs/xfs/xfs_iomap.c +0 -3
@@ -261,7 +261,6 @@ xfs_iomap_write_direct(/* * Complete the transaction */ -xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_trans_commit (tp);if (error) goto out_unlock;
@@ -764,7 +763,6 @@ xfs_iomap_write_allocate(if (error) goto trans_cancel; -xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_trans_commit (tp);if (error) goto error0;
@@ -884,7 +882,6 @@ xfs_iomap_write_unwritten(xfs_trans_log_inode (tp, ip,XFS_ILOG_CORE ); } -xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_trans_commit (tp);xfs_iunlock (ip,XFS_ILOCK_EXCL );if (error)
fs/xfs/xfs_reflink.c +0 -4
@@ -435,7 +435,6 @@ xfs_reflink_allocate_cow(xfs_inode_set_cowblocks_tag (ip);/* Finish up. */ -xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_trans_commit (tp);if (error)return error;
@@ -518,7 +517,6 @@ xfs_reflink_cancel_cow_blocks( NULL);/* Roll the transaction */ -xfs_defer_ijoin ((*tpp)->t_dfops, ip); error =xfs_defer_finish (tpp);if (error) {xfs_defer_cancel (*tpp);
@@ -716,7 +714,6 @@ xfs_reflink_end_cow(/* Remove the mapping from the CoW fork. */ xfs_bmap_del_extent_cow (ip, &icur, &got, &del); -xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_defer_finish (&tp);if (error) goto out_cancel;
@@ -1077,7 +1074,6 @@ xfs_reflink_remap_extent( next_extent:/* Process all the deferred stuff. */ -xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_defer_finish (&tp);if (error) goto out_cancel;
fs/xfs/xfs_symlink.c +0 -1
@@ -454,7 +454,6 @@ xfs_inactive_symlink_rmt(* Commit the transaction. This first logs the EFI and the inode, then * rolls and commits the transaction that frees the extents. */ -xfs_defer_ijoin (tp->t_dfops, ip);xfs_trans_log_inode (tp, ip,XFS_ILOG_CORE ); error =xfs_trans_commit (tp);if (error) {
fs/xfs/xfs_trans.h +0 -3
@@ -97,9 +97,6 @@ void xfs_log_item_init(struct xfs_mount *mp, struct xfs_log_item *item,struct xfs_defer_ops {struct list_head dop_intake;/* unlogged pending work */ struct list_head dop_pending;/* logged pending work */ - -/* relog these with each roll */ -struct xfs_inode *dop_inodes[XFS_DEFER_OPS_NR_INODES ]; };/*
Signature · 1 line
2.17.1
Re: [PATCH v2 08/15] xfs: automatic dfops inode relogging
Quoted 6 lines · Brian Foster
Inodes that are held across deferred operations are explicitly joined to the dfops structure to ensure appropriate relogging. While inodes are currently joined explicitly, we can detect the conditions that require relogging at dfops finish time by inspecting the transaction item list for inodes with
ili_lock_flags== 0.
Quoted 7 lines
Replace the
xfs_defer_ijoin()infrastructure with such detection and automatic relogging of held inodes. This eliminates the need for the per-dfops inode list, replaced by an on-stack variant inxfs_defer_trans_roll().Signed-off-by Brian Foster <[email protected]>Reviewed-by Christoph Hellwig <[email protected]>
Looks ok,
--D
Quoted 418 lines
--- fs/xfs/libxfs/xfs_attr.c | 9 ----- fs/xfs/libxfs/xfs_attr_remote.c | 2 -- fs/xfs/libxfs/xfs_bmap.c | 8 ----- fs/xfs/libxfs/xfs_defer.c | 59 ++++++++++++--------------------- fs/xfs/libxfs/xfs_defer.h | 1 - fs/xfs/xfs_bmap_util.c | 4 --- fs/xfs/xfs_inode.c | 2 -- fs/xfs/xfs_iomap.c | 3 -- fs/xfs/xfs_reflink.c | 4 --- fs/xfs/xfs_symlink.c | 1 - fs/xfs/xfs_trans.h | 3 -- 11 files changed, 21 insertions(+), 75 deletions(-)fs/xfs/libxfs/xfs_attr.c +0
-9@@ -320,7 +320,6 @@ xfs_attr_set(* buffer and run into problems with the write verifier. */ xfs_trans_bhold (args.trans, leaf_bp); -xfs_defer_ijoin (args.trans->t_dfops, dp); error =xfs_defer_finish (&args.trans);if (error) goto out;@@ -589,7 +588,6 @@ xfs_attr_leaf_addname( error =xfs_attr3_leaf_to_node (args);if (error) goto out_defer_cancel; -xfs_defer_ijoin (args->trans->t_dfops, dp); error =xfs_defer_finish (&args->trans);if (error) goto out_defer_cancel;@@ -678,7 +676,6 @@ xfs_attr_leaf_addname(/* bp is gone due to xfs_da_shrink_inode */ if (error) goto out_defer_cancel; -xfs_defer_ijoin (args->trans->t_dfops, dp); error =xfs_defer_finish (&args->trans);if (error) goto out_defer_cancel;@@ -742,7 +739,6 @@ xfs_attr_leaf_removename(/* bp is gone due to xfs_da_shrink_inode */ if (error) goto out_defer_cancel; -xfs_defer_ijoin (args->trans->t_dfops, dp); error =xfs_defer_finish (&args->trans);if (error) goto out_defer_cancel;@@ -869,7 +865,6 @@ xfs_attr_node_addname( error =xfs_attr3_leaf_to_node (args);if (error) goto out_defer_cancel; -xfs_defer_ijoin (args->trans->t_dfops, dp); error =xfs_defer_finish (&args->trans);if (error) goto out_defer_cancel;@@ -894,7 +889,6 @@ xfs_attr_node_addname( error =xfs_da3_split (state);if (error) goto out_defer_cancel; -xfs_defer_ijoin (args->trans->t_dfops, dp); error =xfs_defer_finish (&args->trans);if (error) goto out_defer_cancel;@@ -991,7 +985,6 @@ xfs_attr_node_addname( error =xfs_da3_join (state);if (error) goto out_defer_cancel; -xfs_defer_ijoin (args->trans->t_dfops, dp); error =xfs_defer_finish (&args->trans);if (error) goto out_defer_cancel;@@ -1115,7 +1108,6 @@ xfs_attr_node_removename( error =xfs_da3_join (state);if (error) goto out_defer_cancel; -xfs_defer_ijoin (args->trans->t_dfops, dp); error =xfs_defer_finish (&args->trans);if (error) goto out_defer_cancel;@@ -1147,7 +1139,6 @@ xfs_attr_node_removename(/* bp is gone due to xfs_da_shrink_inode */ if (error) goto out_defer_cancel; -xfs_defer_ijoin (args->trans->t_dfops, dp); error =xfs_defer_finish (&args->trans);if (error) goto out_defer_cancel;fs/xfs/libxfs/xfs_attr_remote.c +0
-2@@ -486,7 +486,6 @@ xfs_attr_rmtval_set( &nmap);if (error) goto out_defer_cancel; -xfs_defer_ijoin (args->trans->t_dfops, dp); error =xfs_defer_finish (&args->trans);if (error) goto out_defer_cancel;@@ -627,7 +626,6 @@ xfs_attr_rmtval_remove( XFS_BMAPI_ATTRFORK, 1, &done);if (error) goto out_defer_cancel; -xfs_defer_ijoin (args->trans->t_dfops, args->dp); error =xfs_defer_finish (&args->trans);if (error) goto out_defer_cancel;fs/xfs/libxfs/xfs_bmap.c +0
-8@@ -1119,7 +1119,6 @@ xfs_bmap_add_attrfork(xfs_log_sb (tp); } -xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_trans_commit (tp);xfs_iunlock (ip,XFS_ILOCK_EXCL );return error;@@ -5987,7 +5986,6 @@ __xfs_bmap_add(int whichfork, struct xfs_bmbt_irec *bmap) { - int error; struct xfs_bmap_intent *bi; trace_xfs_bmap_defer(mp,@@ -6006,12 +6004,6 @@ __xfs_bmap_add( bi->bi_whichfork = whichfork; bi->bi_bmap = *bmap; - error =xfs_defer_ijoin (dfops, bi->bi_owner); -if (error) { -kmem_free (bi); -return error; - } -xfs_defer_add (dfops,XFS_DEFER_OPS_TYPE_BMAP , &bi->bi_list);return 0 ; }fs/xfs/libxfs/xfs_defer.c +21
-38@@ -15,6 +15,8 @@#include "xfs_defer.h" #include "xfs_trans.h" #include "xfs_buf_item.h" +#include "xfs_inode.h" +#include "xfs_inode_item.h" #include "xfs_trace.h" /* @@ -230,16 +232,14 @@ xfs_defer_trans_roll( {struct xfs_defer_ops *dop = (*tp)->t_dfops;struct xfs_buf_log_item *bli; +struct xfs_inode_log_item *ili;struct xfs_log_item *lip;struct xfs_buf *bplist[XFS_DEFER_OPS_NR_BUFS ]; -int bpcount =0 ; +struct xfs_inode *iplist[XFS_DEFER_OPS_NR_INODES ]; +int bpcount =0 , ipcount =0 ;int i;int error; -/* Log all the joined inodes. */ -for (i =0 ; i <XFS_DEFER_OPS_NR_INODES && dop->dop_inodes[i]; i++) -xfs_trans_log_inode (*tp, dop->dop_inodes[i],XFS_ILOG_CORE ); -list_for_each_entry (lip, &(*tp)->t_items, li_trans) {switch (lip->li_type) {case XFS_LI_BUF :@@ -254,6 +254,19 @@ xfs_defer_trans_roll( bplist[bpcount++] = bli->bli_buf; }break ; +case XFS_LI_INODE : + ili = container_of(lip, struct xfs_inode_log_item, + ili_item); +if (ili->ili_lock_flags ==0 ) { +if (ipcount >=XFS_DEFER_OPS_NR_INODES ) { +ASSERT (0 ); +return -EFSCORRUPTED ; + } +xfs_trans_log_inode (*tp, ili->ili_inode, +XFS_ILOG_CORE ); + iplist[ipcount++] = ili->ili_inode; + } +break ;default :break ; }@@ -271,8 +284,8 @@ xfs_defer_trans_roll( }/* Rejoin the joined inodes. */ -for (i =0 ; i <XFS_DEFER_OPS_NR_INODES && dop->dop_inodes[i]; i++) -xfs_trans_ijoin (*tp, dop->dop_inodes[i],0 ); +for (i =0 ; i < ipcount; i++) +xfs_trans_ijoin (*tp, iplist[i],0 );/* Rejoin the buffers and dirty them so the log moves forward. */ for (i =0 ; i < bpcount; i++) {@@ -291,30 +304,6 @@ xfs_defer_has_unfinished_work(return !list_empty (&dop->dop_pending) || !list_empty (&dop->dop_intake); } -/* -* Add this inode to the deferred op. Each joined inode is relogged -* each time we roll the transaction. -*/ -int -xfs_defer_ijoin ( -struct xfs_defer_ops *dop, -struct xfs_inode *ip) -{ -int i; - -for (i =0 ; i <XFS_DEFER_OPS_NR_INODES ; i++) { -if (dop->dop_inodes[i] == ip) -return 0 ; -else if (dop->dop_inodes[i] ==NULL ) { - dop->dop_inodes[i] = ip; -return 0 ; - } - } - -ASSERT (0 ); -return -EFSCORRUPTED ; -} -/* * Reset an already used dfops after finish. */ @@ -322,11 +311,7 @@ static voidxfs_defer_reset ( struct xfs_trans *tp) { -struct xfs_defer_ops *dop = tp->t_dfops; - -ASSERT (!xfs_defer_has_unfinished_work (dop)); - -memset (dop->dop_inodes,0 ,sizeof (dop->dop_inodes)); +ASSERT (!xfs_defer_has_unfinished_work (tp->t_dfops));/* * Low mode state transfers across transaction rolls to mirror dfops @@ -588,8 +573,6 @@ xfs_defer_move(list_splice_init (&src->dop_intake, &dst->dop_intake);list_splice_init (&src->dop_pending, &dst->dop_pending); -memcpy (dst->dop_inodes, src->dop_inodes,sizeof (dst->dop_inodes)); -/* * Low free space mode was historically controlled by a dfops field. * This meant that low mode state potentially carried across multiple fs/xfs/libxfs/xfs_defer.h +0
-1@@ -42,7 +42,6 @@ int xfs_defer_finish(struct xfs_trans **tp);void xfs_defer_cancel (struct xfs_trans *);void xfs_defer_init (struct xfs_trans *tp,struct xfs_defer_ops *dop);bool xfs_defer_has_unfinished_work (struct xfs_defer_ops *dop); -int xfs_defer_ijoin (struct xfs_defer_ops *dop,struct xfs_inode *ip);void xfs_defer_move (struct xfs_trans *dtp,struct xfs_trans *stp);/* Description of a deferred type. */ fs/xfs/xfs_bmap_util.c +0
-4@@ -979,7 +979,6 @@ xfs_alloc_file_space(/* * Complete the transaction */ -xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_trans_commit (tp);xfs_iunlock (ip,XFS_ILOCK_EXCL );if (error)@@ -1037,8 +1036,6 @@ xfs_unmap_extent(if (error) goto out_trans_cancel; -xfs_defer_ijoin (tp->t_dfops, ip); - error =xfs_trans_commit (tp); out_unlock:xfs_iunlock (ip,XFS_ILOCK_EXCL );@@ -1624,7 +1621,6 @@ xfs_swap_extent_rmap(if (error) goto out_defer; -xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_defer_finish (tpp); tp = *tpp;if (error)fs/xfs/xfs_inode.c +0
-2@@ -1569,7 +1569,6 @@ xfs_itruncate_extents_flags(* Duplicate the transaction that has the permanent * reservation and commit the old transaction. */ -xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_defer_finish (&tp);if (error) goto out_bmap_cancel;@@ -1810,7 +1809,6 @@ xfs_inactive_ifree(* Just ignore errors at this point. There is nothing we can do except * to try to keep going. Make sure it's not a silent error. */ -xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_trans_commit (tp);if (error) xfs_notice(mp, "%s: xfs_trans_commit returned error %d",fs/xfs/xfs_iomap.c +0
-3@@ -261,7 +261,6 @@ xfs_iomap_write_direct(/* * Complete the transaction */ -xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_trans_commit (tp);if (error) goto out_unlock;@@ -764,7 +763,6 @@ xfs_iomap_write_allocate(if (error) goto trans_cancel; -xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_trans_commit (tp);if (error) goto error0;@@ -884,7 +882,6 @@ xfs_iomap_write_unwritten(xfs_trans_log_inode (tp, ip,XFS_ILOG_CORE ); } -xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_trans_commit (tp);xfs_iunlock (ip,XFS_ILOCK_EXCL );if (error)fs/xfs/xfs_reflink.c +0
-4@@ -435,7 +435,6 @@ xfs_reflink_allocate_cow(xfs_inode_set_cowblocks_tag (ip);/* Finish up. */ -xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_trans_commit (tp);if (error)return error;@@ -518,7 +517,6 @@ xfs_reflink_cancel_cow_blocks( NULL);/* Roll the transaction */ -xfs_defer_ijoin ((*tpp)->t_dfops, ip); error =xfs_defer_finish (tpp);if (error) {xfs_defer_cancel (*tpp);@@ -716,7 +714,6 @@ xfs_reflink_end_cow(/* Remove the mapping from the CoW fork. */ xfs_bmap_del_extent_cow (ip, &icur, &got, &del); -xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_defer_finish (&tp);if (error) goto out_cancel;@@ -1077,7 +1074,6 @@ xfs_reflink_remap_extent( next_extent:/* Process all the deferred stuff. */ -xfs_defer_ijoin (tp->t_dfops, ip); error =xfs_defer_finish (&tp);if (error) goto out_cancel;fs/xfs/xfs_symlink.c +0
-1@@ -454,7 +454,6 @@ xfs_inactive_symlink_rmt(* Commit the transaction. This first logs the EFI and the inode, then * rolls and commits the transaction that frees the extents. */ -xfs_defer_ijoin (tp->t_dfops, ip);xfs_trans_log_inode (tp, ip,XFS_ILOG_CORE ); error =xfs_trans_commit (tp);if (error) {fs/xfs/xfs_trans.h +0
-3@@ -97,9 +97,6 @@ void xfs_log_item_init(struct xfs_mount *mp, struct xfs_log_item *item,struct xfs_defer_ops {struct list_head dop_intake;/* unlogged pending work */ struct list_head dop_pending;/* logged pending work */ - -/* relog these with each roll */ -struct xfs_inode *dop_inodes[XFS_DEFER_OPS_NR_INODES ]; };/* Signature · 6 lines
2.17.1 -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v2 9/15 xfs: drop dop param from xfs_defer_op_type ->finish_item() callback
In reply to Brian FosterThe dfops infrastructure ->finish_item() callback passes the transaction and dfops as separate parameters. Since dfops is always part of a transaction, the latter parameter is no longer necessary. Remove it from the various callbacks.
--- fs/xfs/libxfs/xfs_bmap.c | 2 -- fs/xfs/libxfs/xfs_bmap.h | 6 +++--- fs/xfs/libxfs/xfs_defer.c | 2 +- fs/xfs/libxfs/xfs_defer.h | 4 ++-- fs/xfs/libxfs/xfs_refcount.c | 2 +- fs/xfs/libxfs/xfs_refcount.h | 7 +++---- fs/xfs/xfs_bmap_item.c | 5 ++--- fs/xfs/xfs_refcount_item.c | 4 ++-- fs/xfs/xfs_trans.h | 10 +++++----- fs/xfs/xfs_trans_bmap.c | 6 ++---- fs/xfs/xfs_trans_extfree.c | 2 -- fs/xfs/xfs_trans_refcount.c | 6 ++---- fs/xfs/xfs_trans_rmap.c | 1 - 13 files changed, 23 insertions(+), 34 deletions(-)
fs/xfs/libxfs/xfs_bmap.c +0 -2
@@ -6045,7 +6045,6 @@ xfs_bmap_unmap_extent( int xfs_bmap_finish_one( struct xfs_trans *tp, - struct xfs_defer_ops *dfops, struct xfs_inode *ip, enum xfs_bmap_intent_type type, int whichfork,
@@ -6072,7 +6071,6 @@ xfs_bmap_finish_one(switch (type) {case XFS_BMAP_MAP : -ASSERT (dfops == tp->t_dfops); error =xfs_bmapi_remap (tp, ip, startoff, *blockcount, startblock,0 ); *blockcount =0 ;
fs/xfs/libxfs/xfs_bmap.h +3 -3
@@ -252,9 +252,9 @@ struct xfs_bmap_intent {struct xfs_bmbt_irec bi_bmap; }; -int xfs_bmap_finish_one (struct xfs_trans *tp,struct xfs_defer_ops *dfops, -struct xfs_inode *ip,enum xfs_bmap_intent_type type, -int whichfork,xfs_fileoff_t startoff,xfs_fsblock_t startblock, +int xfs_bmap_finish_one (struct xfs_trans *tp,struct xfs_inode *ip, +enum xfs_bmap_intent_type type,int whichfork, +xfs_fileoff_t startoff,xfs_fsblock_t startblock,xfs_filblks_t *blockcount,xfs_exntst_t state);int xfs_bmap_map_extent (struct xfs_mount *mp,struct xfs_defer_ops *dfops,struct xfs_inode *ip,struct xfs_bmbt_irec *imap);
fs/xfs/libxfs/xfs_defer.c +1 -1
@@ -371,7 +371,7 @@ xfs_defer_finish_noroll(list_for_each_safe (li, n, &dfp->dfp_work) {list_del (li); dfp->dfp_count--; - error = dfp->dfp_type->finish_item (*tp, dop, li, + error = dfp->dfp_type->finish_item (*tp, li, dfp->dfp_done, &state);if (error == -EAGAIN ) {/*
fs/xfs/libxfs/xfs_defer.h +2 -2
@@ -50,8 +50,8 @@ struct xfs_defer_op_type {unsigned int max_items;void (*abort_intent)(void *);void *(*create_done)(struct xfs_trans *,void *,unsigned int ); -int (*finish_item)(struct xfs_trans *,struct xfs_defer_ops *, -struct list_head *,void *,void **); +int (*finish_item)(struct xfs_trans *,struct list_head *,void *, +void **);void (*finish_cleanup)(struct xfs_trans *,void *,int );void (*cancel_item)(struct list_head *);int (*diff_items)(void *,struct list_head *,struct list_head *);
fs/xfs/libxfs/xfs_refcount.c +1 -1
@@ -1082,7 +1082,6 @@ xfs_refcount_finish_one_cleanup( int xfs_refcount_finish_one( struct xfs_trans *tp, - struct xfs_defer_ops *dfops, enum xfs_refcount_intent_type type, xfs_fsblock_t startblock, xfs_extlen_t blockcount,
@@ -1091,6 +1090,7 @@ xfs_refcount_finish_one( struct xfs_btree_cur **pcur) {struct xfs_mount *mp = tp->t_mountp; +struct xfs_defer_ops *dfops = tp->t_dfops;struct xfs_btree_cur *rcur;struct xfs_buf *agbp =NULL ;int error =0 ;
fs/xfs/libxfs/xfs_refcount.h +3 -4
@@ -37,10 +37,9 @@ extern int xfs_refcount_decrease_extent(struct xfs_mount *mp,extern void xfs_refcount_finish_one_cleanup (struct xfs_trans *tp,struct xfs_btree_cur *rcur,int error);extern int xfs_refcount_finish_one (struct xfs_trans *tp, -struct xfs_defer_ops *dfops,enum xfs_refcount_intent_type type, -xfs_fsblock_t startblock,xfs_extlen_t blockcount, -xfs_fsblock_t *new_fsb,xfs_extlen_t *new_len, -struct xfs_btree_cur **pcur); +enum xfs_refcount_intent_type type,xfs_fsblock_t startblock, +xfs_extlen_t blockcount,xfs_fsblock_t *new_fsb, +xfs_extlen_t *new_len,struct xfs_btree_cur **pcur); extern int xfs_refcount_find_shared(struct xfs_btree_cur *cur, xfs_agblock_t agbno, xfs_extlen_t aglen, xfs_agblock_t *fbno,
fs/xfs/xfs_bmap_item.c +2 -3
@@ -475,9 +475,8 @@ xfs_bui_recover(xfs_trans_ijoin (tp, ip,0 ); count = bmap->me_len; - error =xfs_trans_log_finish_bmap_update (tp, budp, tp->t_dfops, type, - ip, whichfork, bmap->me_startoff, - bmap->me_startblock, &count, state); + error =xfs_trans_log_finish_bmap_update (tp, budp, type, ip, whichfork, + bmap->me_startoff, bmap->me_startblock, &count, state);if (error) goto err_inode;
fs/xfs/xfs_refcount_item.c +2 -2
@@ -479,8 +479,8 @@ xfs_cui_recover( new_len = refc->pe_len; }else error =xfs_trans_log_finish_refcount_update (tp, cudp, - tp->t_dfops, type, refc->pe_startblock, - refc->pe_len, &new_fsb, &new_len, &rcur); + type, refc->pe_startblock, refc->pe_len, + &new_fsb, &new_len, &rcur);if (error) goto abort_error;
fs/xfs/xfs_trans.h +5 -5
@@ -268,7 +268,7 @@ void xfs_refcount_update_init_defer_op(void);struct xfs_cud_log_item *xfs_trans_get_cud (struct xfs_trans *tp,struct xfs_cui_log_item *cuip);int xfs_trans_log_finish_refcount_update (struct xfs_trans *tp, -struct xfs_cud_log_item *cudp,struct xfs_defer_ops *dfops, +struct xfs_cud_log_item *cudp,enum xfs_refcount_intent_type type,xfs_fsblock_t startblock,xfs_extlen_t blockcount,xfs_fsblock_t *new_fsb,xfs_extlen_t *new_len,struct xfs_btree_cur **pcur);
@@ -280,9 +280,9 @@ void xfs_bmap_update_init_defer_op(void);struct xfs_bud_log_item *xfs_trans_get_bud (struct xfs_trans *tp,struct xfs_bui_log_item *buip);int xfs_trans_log_finish_bmap_update (struct xfs_trans *tp, -struct xfs_bud_log_item *rudp,struct xfs_defer_ops *dfops, -enum xfs_bmap_intent_type type,struct xfs_inode *ip, -int whichfork,xfs_fileoff_t startoff,xfs_fsblock_t startblock, -xfs_filblks_t *blockcount,xfs_exntst_t state); +struct xfs_bud_log_item *rudp,enum xfs_bmap_intent_type type, +struct xfs_inode *ip,int whichfork,xfs_fileoff_t startoff, +xfs_fsblock_t startblock,xfs_filblks_t *blockcount, +xfs_exntst_t state);#endif /* __XFS_TRANS_H__ */
fs/xfs/xfs_trans_bmap.c +2 -4
@@ -43,7 +43,6 @@ intxfs_trans_log_finish_bmap_update ( struct xfs_trans *tp, struct xfs_bud_log_item *budp, - struct xfs_defer_ops *dop, enum xfs_bmap_intent_type type, struct xfs_inode *ip, int whichfork,
@@ -54,7 +53,7 @@ xfs_trans_log_finish_bmap_update( {int error; - error =xfs_bmap_finish_one (tp, dop, ip, type, whichfork, startoff, + error =xfs_bmap_finish_one (tp, ip, type, whichfork, startoff, startblock, blockcount, state);/*
@@ -176,7 +175,6 @@ xfs_bmap_update_create_done( STATIC intxfs_bmap_update_finish_item (struct xfs_trans *tp, -struct xfs_defer_ops *dop,struct list_head *item,void *done_item,void **state)
@@ -187,7 +185,7 @@ xfs_bmap_update_finish_item( bmap = container_of(item, struct xfs_bmap_intent, bi_list); count = bmap->bi_bmap.br_blockcount; - error = xfs_trans_log_finish_bmap_update(tp, done_item, dop, + error = xfs_trans_log_finish_bmap_update(tp, done_item, bmap->bi_type, bmap->bi_owner, bmap->bi_whichfork, bmap->bi_bmap.br_startoff,
fs/xfs/xfs_trans_extfree.c +0 -2
@@ -171,7 +171,6 @@ xfs_extent_free_create_done( STATIC intxfs_extent_free_finish_item (struct xfs_trans *tp, -struct xfs_defer_ops *dop,struct list_head *item,void *done_item,void **state)
@@ -226,7 +225,6 @@ static const struct xfs_defer_op_type xfs_extent_free_defer_type = { STATIC intxfs_agfl_free_finish_item (struct xfs_trans *tp, -struct xfs_defer_ops *dop,struct list_head *item,void *done_item,void **state)
fs/xfs/xfs_trans_refcount.c +2 -4
@@ -42,7 +42,6 @@ intxfs_trans_log_finish_refcount_update ( struct xfs_trans *tp, struct xfs_cud_log_item *cudp, - struct xfs_defer_ops *dop, enum xfs_refcount_intent_type type, xfs_fsblock_t startblock, xfs_extlen_t blockcount,
@@ -52,7 +51,7 @@ xfs_trans_log_finish_refcount_update( {int error; - error =xfs_refcount_finish_one (tp, dop, type, startblock, + error =xfs_refcount_finish_one (tp, type, startblock, blockcount, new_fsb, new_len, pcur);/*
@@ -169,7 +168,6 @@ xfs_refcount_update_create_done( STATIC intxfs_refcount_update_finish_item (struct xfs_trans *tp, -struct xfs_defer_ops *dop,struct list_head *item,void *done_item,void **state)
@@ -180,7 +178,7 @@ xfs_refcount_update_finish_item(int error; refc = container_of(item, struct xfs_refcount_intent, ri_list); - error = xfs_trans_log_finish_refcount_update(tp, done_item, dop, + error = xfs_trans_log_finish_refcount_update(tp, done_item, refc->ri_type, refc->ri_startblock, refc->ri_blockcount,
fs/xfs/xfs_trans_rmap.c +0 -1
@@ -193,7 +193,6 @@ xfs_rmap_update_create_done( STATIC intxfs_rmap_update_finish_item (struct xfs_trans *tp, -struct xfs_defer_ops *dop,struct list_head *item,void *done_item,void **state)
Signature · 1 line
2.17.1
PATCH v2 10/15 xfs: clean out superfluous dfops dop params/vars
In reply to Brian FosterThe dfops code still passes around the xfs_defer_ops pointer superfluously in a few places. Clean this up wherever the transaction will suffice.
--- fs/xfs/libxfs/xfs_defer.c | 43 +++++++++++++++++++-------------------- fs/xfs/libxfs/xfs_defer.h | 2 +- 2 files changed, 22 insertions(+), 23 deletions(-)
fs/xfs/libxfs/xfs_defer.c +21 -22
@@ -181,9 +181,9 @@ static const struct xfs_defer_op_type *defer_op_types[XFS_DEFER_OPS_TYPE_MAX];*/ STATIC voidxfs_defer_intake_work ( -struct xfs_trans *tp, -struct xfs_defer_ops *dop) +struct xfs_trans *tp) { +struct xfs_defer_ops *dop = tp->t_dfops;struct list_head *li;struct xfs_defer_pending *dfp;
@@ -204,9 +204,9 @@ xfs_defer_intake_work( STATIC voidxfs_defer_trans_abort (struct xfs_trans *tp, -struct xfs_defer_ops *dop,int error) { +struct xfs_defer_ops *dop = tp->t_dfops;struct xfs_defer_pending *dfp;trace_xfs_defer_trans_abort (tp->t_mountp, dop, _RET_IP_);
@@ -230,7 +230,6 @@ STATIC intxfs_defer_trans_roll ( struct xfs_trans **tp) { -struct xfs_defer_ops *dop = (*tp)->t_dfops;struct xfs_buf_log_item *bli;struct xfs_inode_log_item *ili;struct xfs_log_item *lip;
@@ -272,14 +271,14 @@ xfs_defer_trans_roll( } } -trace_xfs_defer_trans_roll ((*tp)->t_mountp, dop, _RET_IP_); +trace_xfs_defer_trans_roll ((*tp)->t_mountp, (*tp)->t_dfops, _RET_IP_);/* Roll the transaction. */ error =xfs_trans_roll (tp); - dop = (*tp)->t_dfops;if (error) { -trace_xfs_defer_trans_roll_error ((*tp)->t_mountp, dop, error); -xfs_defer_trans_abort (*tp, dop, error); +trace_xfs_defer_trans_roll_error ((*tp)->t_mountp, + (*tp)->t_dfops, error); +xfs_defer_trans_abort (*tp, error);return error; }
@@ -299,9 +298,10 @@ xfs_defer_trans_roll(/* Do we have any work items to finish? */ bool xfs_defer_has_unfinished_work ( -struct xfs_defer_ops *dop) +struct xfs_trans *tp) { -return !list_empty (&dop->dop_pending) || !list_empty (&dop->dop_intake); +return !list_empty (&tp->t_dfops->dop_pending) || + !list_empty (&tp->t_dfops->dop_intake); }/*
@@ -311,7 +311,7 @@ static voidxfs_defer_reset ( struct xfs_trans *tp) { -ASSERT (!xfs_defer_has_unfinished_work (tp->t_dfops)); +ASSERT (!xfs_defer_has_unfinished_work (tp));/* * Low mode state transfers across transaction rolls to mirror dfops
@@ -332,7 +332,6 @@ intxfs_defer_finish_noroll ( struct xfs_trans **tp) { -struct xfs_defer_ops *dop = (*tp)->t_dfops;struct xfs_defer_pending *dfp;struct list_head *li;struct list_head *n;
@@ -342,24 +341,22 @@ xfs_defer_finish_noroll(ASSERT ((*tp)->t_flags &XFS_TRANS_PERM_LOG_RES ); -trace_xfs_defer_finish ((*tp)->t_mountp, dop, _RET_IP_); +trace_xfs_defer_finish ((*tp)->t_mountp, (*tp)->t_dfops, _RET_IP_);/* Until we run out of pending work to finish... */ -while (xfs_defer_has_unfinished_work (dop)) { +while (xfs_defer_has_unfinished_work (*tp)) {/* Log intents for work items sitting in the intake. */ -xfs_defer_intake_work (*tp, dop); +xfs_defer_intake_work (*tp);/* -* Roll the transaction and update dop in case dfops was -* embedded in the transaction. +* Roll the transaction. */ error =xfs_defer_trans_roll (tp);if (error) goto out; - dop = (*tp)->t_dfops;/* Log an intent-done item for the first pending item. */ - dfp =list_first_entry (&dop->dop_pending, + dfp =list_first_entry (&(*tp)->t_dfops->dop_pending, struct xfs_defer_pending, dfp_list);trace_xfs_defer_pending_finish ((*tp)->t_mountp, dfp); dfp->dfp_done = dfp->dfp_type->create_done(*tp, dfp->dfp_intent,
@@ -390,7 +387,7 @@ xfs_defer_finish_noroll(*/ if (cleanup_fn)cleanup_fn (*tp, state, error); -xfs_defer_trans_abort (*tp, dop, error); +xfs_defer_trans_abort (*tp, error); goto out; } }
@@ -420,9 +417,11 @@ xfs_defer_finish_noroll( out:if (error) -trace_xfs_defer_finish_error ((*tp)->t_mountp, dop, error); +trace_xfs_defer_finish_error ((*tp)->t_mountp, (*tp)->t_dfops, + error);else -trace_xfs_defer_finish_done ((*tp)->t_mountp, dop, _RET_IP_); +trace_xfs_defer_finish_done ((*tp)->t_mountp, (*tp)->t_dfops, + _RET_IP_);return error; }
fs/xfs/libxfs/xfs_defer.h +1 -1
@@ -41,7 +41,7 @@ int xfs_defer_finish_noroll(struct xfs_trans **tp);int xfs_defer_finish (struct xfs_trans **tp);void xfs_defer_cancel (struct xfs_trans *);void xfs_defer_init (struct xfs_trans *tp,struct xfs_defer_ops *dop); -bool xfs_defer_has_unfinished_work (struct xfs_defer_ops *dop); +bool xfs_defer_has_unfinished_work (struct xfs_trans *tp);void xfs_defer_move (struct xfs_trans *dtp,struct xfs_trans *stp);/* Description of a deferred type. */
Signature · 1 line
2.17.1
Re: [PATCH v2 10/15] xfs: clean out superfluous dfops dop params/vars
Quoted 67 lines · Brian Foster
The dfops code still passes around the
xfs_defer_opspointer superfluously in a few places. Clean this up wherever the transaction will suffice.Signed-off-by Brian Foster <[email protected]>Reviewed-by Darrick J. Wong <[email protected]>Reviewed-by Christoph Hellwig <[email protected]>--- fs/xfs/libxfs/xfs_defer.c | 43 +++++++++++++++++++-------------------- fs/xfs/libxfs/xfs_defer.h | 2 +- 2 files changed, 22 insertions(+), 23 deletions(-)fs/xfs/libxfs/xfs_defer.c +7
-8@@ -181,9 +181,9 @@ static const struct xfs_defer_op_type *defer_op_types[XFS_DEFER_OPS_TYPE_MAX];*/ STATIC voidxfs_defer_intake_work ( -struct xfs_trans *tp, -struct xfs_defer_ops *dop) +struct xfs_trans *tp) { +struct xfs_defer_ops *dop = tp->t_dfops;struct list_head *li;struct xfs_defer_pending *dfp;@@ -204,9 +204,9 @@ xfs_defer_intake_work( STATIC voidxfs_defer_trans_abort (struct xfs_trans *tp, -struct xfs_defer_ops *dop,int error) { +struct xfs_defer_ops *dop = tp->t_dfops;struct xfs_defer_pending *dfp;trace_xfs_defer_trans_abort (tp->t_mountp, dop, _RET_IP_);@@ -230,7 +230,6 @@ STATIC intxfs_defer_trans_roll ( struct xfs_trans **tp) { -struct xfs_defer_ops *dop = (*tp)->t_dfops;struct xfs_buf_log_item *bli;struct xfs_inode_log_item *ili;struct xfs_log_item *lip;@@ -272,14 +271,14 @@ xfs_defer_trans_roll( } } -trace_xfs_defer_trans_roll ((*tp)->t_mountp, dop, _RET_IP_); +trace_xfs_defer_trans_roll ((*tp)->t_mountp, (*tp)->t_dfops, _RET_IP_);/* Roll the transaction. */ error =xfs_trans_roll (tp); - dop = (*tp)->t_dfops;if (error) { -trace_xfs_defer_trans_roll_error ((*tp)->t_mountp, dop, error); -xfs_defer_trans_abort (*tp, dop, error); +trace_xfs_defer_trans_roll_error ((*tp)->t_mountp, + (*tp)->t_dfops, error); +xfs_defer_trans_abort (*tp, error);return error; }
fs/xfs/libxfs/xfs_defer.c xfs_defer_trans_roll( · line 301
Quoted 5 lines
fs/xfs/libxfs/xfs_defer.c +1
-1@@ -299,9 +298,10 @@ xfs_defer_trans_roll(/* Do we have any work items to finish? */ bool xfs_defer_has_unfinished_work ( -struct xfs_defer_ops *dop) +struct xfs_trans *tp)
The WARN_ON_ONCE in __xfs_trans_commit needs to be converted too. Will fix that on the way in.
--D
Quoted 97 lines
fs/xfs/libxfs/xfs_defer.c +13
-13{ -return !list_empty (&dop->dop_pending) || !list_empty (&dop->dop_intake); +return !list_empty (&tp->t_dfops->dop_pending) || + !list_empty (&tp->t_dfops->dop_intake); }/* @@ -311,7 +311,7 @@ static voidxfs_defer_reset ( struct xfs_trans *tp) { -ASSERT (!xfs_defer_has_unfinished_work (tp->t_dfops)); +ASSERT (!xfs_defer_has_unfinished_work (tp));/* * Low mode state transfers across transaction rolls to mirror dfops @@ -332,7 +332,6 @@ intxfs_defer_finish_noroll ( struct xfs_trans **tp) { -struct xfs_defer_ops *dop = (*tp)->t_dfops;struct xfs_defer_pending *dfp;struct list_head *li;struct list_head *n;@@ -342,24 +341,22 @@ xfs_defer_finish_noroll(ASSERT ((*tp)->t_flags &XFS_TRANS_PERM_LOG_RES ); -trace_xfs_defer_finish ((*tp)->t_mountp, dop, _RET_IP_); +trace_xfs_defer_finish ((*tp)->t_mountp, (*tp)->t_dfops, _RET_IP_);/* Until we run out of pending work to finish... */ -while (xfs_defer_has_unfinished_work (dop)) { +while (xfs_defer_has_unfinished_work (*tp)) {/* Log intents for work items sitting in the intake. */ -xfs_defer_intake_work (*tp, dop); +xfs_defer_intake_work (*tp);/* -* Roll the transaction and update dop in case dfops was -* embedded in the transaction. +* Roll the transaction. */ error =xfs_defer_trans_roll (tp);if (error) goto out; - dop = (*tp)->t_dfops;/* Log an intent-done item for the first pending item. */ - dfp =list_first_entry (&dop->dop_pending, + dfp =list_first_entry (&(*tp)->t_dfops->dop_pending, struct xfs_defer_pending, dfp_list);trace_xfs_defer_pending_finish ((*tp)->t_mountp, dfp); dfp->dfp_done = dfp->dfp_type->create_done(*tp, dfp->dfp_intent,@@ -390,7 +387,7 @@ xfs_defer_finish_noroll(*/ if (cleanup_fn)cleanup_fn (*tp, state, error); -xfs_defer_trans_abort (*tp, dop, error); +xfs_defer_trans_abort (*tp, error); goto out; } }@@ -420,9 +417,11 @@ xfs_defer_finish_noroll( out:if (error) -trace_xfs_defer_finish_error ((*tp)->t_mountp, dop, error); +trace_xfs_defer_finish_error ((*tp)->t_mountp, (*tp)->t_dfops, + error);else -trace_xfs_defer_finish_done ((*tp)->t_mountp, dop, _RET_IP_); +trace_xfs_defer_finish_done ((*tp)->t_mountp, (*tp)->t_dfops, + _RET_IP_);return error; }fs/xfs/libxfs/xfs_defer.h +1
-1@@ -41,7 +41,7 @@ int xfs_defer_finish_noroll(struct xfs_trans **tp);int xfs_defer_finish (struct xfs_trans **tp);void xfs_defer_cancel (struct xfs_trans *);void xfs_defer_init (struct xfs_trans *tp,struct xfs_defer_ops *dop); -bool xfs_defer_has_unfinished_work (struct xfs_defer_ops *dop); +bool xfs_defer_has_unfinished_work (struct xfs_trans *tp);void xfs_defer_move (struct xfs_trans *dtp,struct xfs_trans *stp);/* Description of a deferred type. */ Signature · 6 lines
2.17.1 -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v2 11/15 xfs: cancel dfops on xfs_defer_finish() error
In reply to Brian FosterThe current semantics of xfs_defer_finish() require the caller to call xfs_defer_cancel() on error. This is slightly inconsistent with transaction commit error handling where a failed commit cleans up the transaction before returning.
More significantly, the only requirement for exposure of ->dop_pending outside of xfs_defer_finish() is so that xfs_defer_cancel() can drain it on error. Since the only recourse of xfs_defer_finish() errors is cancellation, mirror the transaction logic and cancel remaining dfops before returning from xfs_defer_finish() with an error.
Beside simplifying xfs_defer_finish() semantics, this ensures that xfs_defer_finish() always returns with an empty ->dop_pending and thus facilitates removal of the list from xfs_defer_ops.
--- fs/xfs/libxfs/xfs_attr.c | 16 ++++++++-------- fs/xfs/libxfs/xfs_attr_remote.c | 4 ++-- fs/xfs/libxfs/xfs_defer.c | 11 ++++++----- fs/xfs/xfs_bmap_util.c | 2 +- fs/xfs/xfs_dquot.c | 2 +- fs/xfs/xfs_inode.c | 2 +- fs/xfs/xfs_reflink.c | 4 +--- fs/xfs/xfs_trans.c | 4 +--- 8 files changed, 21 insertions(+), 24 deletions(-)
fs/xfs/libxfs/xfs_attr.c +8 -8
@@ -590,7 +590,7 @@ xfs_attr_leaf_addname( goto out_defer_cancel; error =xfs_defer_finish (&args->trans);if (error) - goto out_defer_cancel; +return error;/* * Commit the current trans (including the inode) and start
@@ -678,7 +678,7 @@ xfs_attr_leaf_addname( goto out_defer_cancel; error =xfs_defer_finish (&args->trans);if (error) - goto out_defer_cancel; +return error; }/*
@@ -741,7 +741,7 @@ xfs_attr_leaf_removename( goto out_defer_cancel; error =xfs_defer_finish (&args->trans);if (error) - goto out_defer_cancel; +return error; }return 0 ; out_defer_cancel:
@@ -867,7 +867,7 @@ xfs_attr_node_addname( goto out_defer_cancel; error =xfs_defer_finish (&args->trans);if (error) - goto out_defer_cancel; + goto out;/* * Commit the node conversion and start the next
@@ -891,7 +891,7 @@ xfs_attr_node_addname( goto out_defer_cancel; error =xfs_defer_finish (&args->trans);if (error) - goto out_defer_cancel; + goto out; }else {/* * Addition succeeded, update Btree hashvals.
@@ -987,7 +987,7 @@ xfs_attr_node_addname( goto out_defer_cancel; error =xfs_defer_finish (&args->trans);if (error) - goto out_defer_cancel; + goto out; }/*
@@ -1110,7 +1110,7 @@ xfs_attr_node_removename( goto out_defer_cancel; error =xfs_defer_finish (&args->trans);if (error) - goto out_defer_cancel; + goto out;/* * Commit the Btree join operation and start a new trans. */
@@ -1141,7 +1141,7 @@ xfs_attr_node_removename( goto out_defer_cancel; error =xfs_defer_finish (&args->trans);if (error) - goto out_defer_cancel; + goto out; }else xfs_trans_brelse (args->trans, bp); }
fs/xfs/libxfs/xfs_attr_remote.c +2 -2
@@ -488,7 +488,7 @@ xfs_attr_rmtval_set( goto out_defer_cancel; error =xfs_defer_finish (&args->trans);if (error) - goto out_defer_cancel; +return error;ASSERT (nmap ==1 ); ASSERT((map.br_startblock != DELAYSTARTBLOCK) &&
@@ -628,7 +628,7 @@ xfs_attr_rmtval_remove( goto out_defer_cancel; error =xfs_defer_finish (&args->trans);if (error) - goto out_defer_cancel; +return error;/* * Close out trans and start the next one in the chain.
fs/xfs/libxfs/xfs_defer.c +6 -5
@@ -416,14 +416,15 @@ xfs_defer_finish_noroll( } out: -if (error) +if (error) {trace_xfs_defer_finish_error ((*tp)->t_mountp, (*tp)->t_dfops, error); -else -trace_xfs_defer_finish_done ((*tp)->t_mountp, (*tp)->t_dfops, - _RET_IP_); +xfs_defer_cancel (*tp); +return error; + } -return error; +trace_xfs_defer_finish_done ((*tp)->t_mountp, (*tp)->t_dfops, _RET_IP_); +return 0 ; }int
fs/xfs/xfs_bmap_util.c +1 -1
@@ -1624,7 +1624,7 @@ xfs_swap_extent_rmap( error =xfs_defer_finish (tpp); tp = *tpp;if (error) - goto out_defer; + goto out; tirec.br_startoff += rlen; if (tirec.br_startblock != HOLESTARTBLOCK &&
fs/xfs/xfs_dquot.c +1 -1
@@ -371,7 +371,7 @@ xfs_dquot_disk_alloc( tp = *tpp;if (error) {xfs_buf_relse (bp); - goto error1; + goto error0; } *bpp = bp;return 0 ;
fs/xfs/xfs_inode.c +1 -1
@@ -1571,7 +1571,7 @@ xfs_itruncate_extents_flags(*/ error =xfs_defer_finish (&tp);if (error) - goto out_bmap_cancel; + goto out; error =xfs_trans_roll_inode (&tp, ip);if (error)
fs/xfs/xfs_reflink.c +1 -3
@@ -518,10 +518,8 @@ xfs_reflink_cancel_cow_blocks(/* Roll the transaction */ error =xfs_defer_finish (tpp); -if (error) { -xfs_defer_cancel (*tpp); +if (error)break ; - }/* Remove the mapping from the CoW fork. */ xfs_bmap_del_extent_cow (ip, &icur, &got, &del);
fs/xfs/xfs_trans.c +1 -3
@@ -933,10 +933,8 @@ __xfs_trans_commit( !(tp->t_flags & XFS_TRANS_PERM_LOG_RES));if (!regrant && (tp->t_flags &XFS_TRANS_PERM_LOG_RES )) { error =xfs_defer_finish_noroll (&tp); -if (error) { -xfs_defer_cancel (tp); +if (error) goto out_unreserve; - } }/*
Signature · 1 line
2.17.1
PATCH v2 12/15 xfs: replace xfs_defer_ops ->dop_pending with on-stack list
In reply to Brian FosterThe xfs_defer_ops ->dop_pending list is used to track active deferred operations once intents are logged. These items must be aborted in the event of an error. The list is populated as intents are logged and items are removed as they complete (or are aborted).
Now that xfs_defer_finish() cancels on error, there is no need to ever access ->dop_pending outside of xfs_defer_finish(). The list is only ever populated after xfs_defer_finish() begins and is either completed or cancelled before it returns.
Remove ->dop_pending from xfs_defer_ops and replace it with a local list in the xfs_defer_finish() path. Pass the local list to the various helpers now that it is not accessible via dfops. Note that we have to check for NULL in the abort case as the final tx roll occurs outside of the scope of the new local list (once the dfops has completed and thus drained the list).
--- fs/xfs/libxfs/xfs_defer.c | 129 ++++++++++++++++++-------------------- fs/xfs/libxfs/xfs_defer.h | 1 - fs/xfs/xfs_trace.h | 5 +- fs/xfs/xfs_trans.c | 2 +- fs/xfs/xfs_trans.h | 1 - 5 files changed, 64 insertions(+), 74 deletions(-)
fs/xfs/libxfs/xfs_defer.c +61 -68
@@ -180,7 +180,7 @@ static const struct xfs_defer_op_type *defer_op_types[XFS_DEFER_OPS_TYPE_MAX];* the pending list. */ STATIC void -xfs_defer_intake_work ( +xfs_defer_create_intents (struct xfs_trans *tp) {struct xfs_defer_ops *dop = tp->t_dfops;
@@ -190,20 +190,19 @@ xfs_defer_intake_work(list_for_each_entry (dfp, &dop->dop_intake, dfp_list) { dfp->dfp_intent = dfp->dfp_type->create_intent (tp, dfp->dfp_count); -trace_xfs_defer_intake_work (tp->t_mountp, dfp); +trace_xfs_defer_create_intent (tp->t_mountp, dfp);list_sort (tp->t_mountp, &dfp->dfp_work, dfp->dfp_type->diff_items);list_for_each (li, &dfp->dfp_work) dfp->dfp_type->log_item (tp, dfp->dfp_intent, li); } - -list_splice_tail_init (&dop->dop_intake, &dop->dop_pending); }/* Abort all the intents that were committed. */ STATIC voidxfs_defer_trans_abort (struct xfs_trans *tp, +struct list_head *dop_pending,int error) {struct xfs_defer_ops *dop = tp->t_dfops;
@@ -212,11 +211,13 @@ xfs_defer_trans_abort(trace_xfs_defer_trans_abort (tp->t_mountp, dop, _RET_IP_);/* Abort intent items that don't have a done item. */ -list_for_each_entry (dfp, &dop->dop_pending, dfp_list) { -trace_xfs_defer_pending_abort (tp->t_mountp, dfp); -if (dfp->dfp_intent && !dfp->dfp_done) { - dfp->dfp_type->abort_intent (dfp->dfp_intent); - dfp->dfp_intent =NULL ; +if (dop_pending) { +list_for_each_entry (dfp, dop_pending, dfp_list) { +trace_xfs_defer_pending_abort (tp->t_mountp, dfp); +if (dfp->dfp_intent && !dfp->dfp_done) { + dfp->dfp_type->abort_intent (dfp->dfp_intent); + dfp->dfp_intent =NULL ; + } } }
@@ -228,7 +229,8 @@ xfs_defer_trans_abort(/* Roll a transaction so we can do some deferred op processing. */ STATIC intxfs_defer_trans_roll ( -struct xfs_trans **tp) +struct xfs_trans **tp, +struct list_head *dop_pending) {struct xfs_buf_log_item *bli;struct xfs_inode_log_item *ili;
@@ -278,7 +280,7 @@ xfs_defer_trans_roll(if (error) {trace_xfs_defer_trans_roll_error ((*tp)->t_mountp, (*tp)->t_dfops, error); -xfs_defer_trans_abort (*tp, error); +xfs_defer_trans_abort (*tp, dop_pending, error);return error; }
@@ -295,15 +297,6 @@ xfs_defer_trans_roll(return error; } -/* Do we have any work items to finish? */ -bool -xfs_defer_has_unfinished_work ( -struct xfs_trans *tp) -{ -return !list_empty (&tp->t_dfops->dop_pending) || - !list_empty (&tp->t_dfops->dop_intake); -} -/* * Reset an already used dfops after finish. */
@@ -311,7 +304,7 @@ static voidxfs_defer_reset ( struct xfs_trans *tp) { -ASSERT (!xfs_defer_has_unfinished_work (tp)); +ASSERT (list_empty (&tp->t_dfops->dop_intake));/* * Low mode state transfers across transaction rolls to mirror dfops
@@ -320,6 +313,36 @@ xfs_defer_reset( tp->t_flags &= ~XFS_TRANS_LOWMODE ; } +/* +* Free up any items left in the list. +*/ +static void +xfs_defer_cancel_list ( +struct xfs_mount *mp, +struct list_head *dop_list) +{ +struct xfs_defer_pending *dfp; +struct xfs_defer_pending *pli; +struct list_head *pwi; +struct list_head *n; + +/* +* Free the pending items. Caller should already have arranged +* for the intent items to be released. +*/ +list_for_each_entry_safe (dfp, pli, dop_list, dfp_list) { +trace_xfs_defer_cancel_list (mp, dfp); +list_del (&dfp->dfp_list); +list_for_each_safe (pwi, n, &dfp->dfp_work) { +list_del (pwi); + dfp->dfp_count--; + dfp->dfp_type->cancel_item (pwi); + } +ASSERT (dfp->dfp_count ==0 ); +kmem_free (dfp); + } +} +/* * Finish all the pending work. This involves logging intent items for * any work items that wandered in since the last transaction roll (if
@@ -338,26 +361,30 @@ xfs_defer_finish_noroll(void *state;int error =0 ;void (*cleanup_fn)(struct xfs_trans *,void *,int ); +LIST_HEAD (dop_pending);ASSERT ((*tp)->t_flags &XFS_TRANS_PERM_LOG_RES );trace_xfs_defer_finish ((*tp)->t_mountp, (*tp)->t_dfops, _RET_IP_);/* Until we run out of pending work to finish... */ -while (xfs_defer_has_unfinished_work (*tp)) { -/* Log intents for work items sitting in the intake. */ -xfs_defer_intake_work (*tp); +while (!list_empty (&dop_pending) || + !list_empty (&(*tp)->t_dfops->dop_intake)) { +/* log intents and pull in intake items */ +xfs_defer_create_intents (*tp); +list_splice_tail_init (&(*tp)->t_dfops->dop_intake, + &dop_pending);/* * Roll the transaction. */ - error =xfs_defer_trans_roll (tp); + error =xfs_defer_trans_roll (tp, &dop_pending);if (error) goto out;/* Log an intent-done item for the first pending item. */ - dfp =list_first_entry (&(*tp)->t_dfops->dop_pending, - struct xfs_defer_pending, dfp_list); + dfp = list_first_entry(&dop_pending, struct xfs_defer_pending, + dfp_list);trace_xfs_defer_pending_finish ((*tp)->t_mountp, dfp); dfp->dfp_done = dfp->dfp_type->create_done (*tp, dfp->dfp_intent, dfp->dfp_count);
@@ -387,7 +414,7 @@ xfs_defer_finish_noroll(*/ if (cleanup_fn)cleanup_fn (*tp, state, error); -xfs_defer_trans_abort (*tp, error); +xfs_defer_trans_abort (*tp, &dop_pending, error); goto out; } }
@@ -419,6 +446,7 @@ xfs_defer_finish_noroll(if (error) {trace_xfs_defer_finish_error ((*tp)->t_mountp, (*tp)->t_dfops, error); +xfs_defer_cancel_list ((*tp)->t_mountp, &dop_pending);xfs_defer_cancel (*tp);return error; }
@@ -441,7 +469,7 @@ xfs_defer_finish(if (error)return error;if ((*tp)->t_flags &XFS_TRANS_DIRTY ) { - error =xfs_defer_trans_roll (tp); + error =xfs_defer_trans_roll (tp,NULL );if (error)return error; }
@@ -449,47 +477,14 @@ xfs_defer_finish(return 0 ; } -/* -* Free up any items left in the list. -*/ void xfs_defer_cancel ( -struct xfs_trans *tp) +struct xfs_trans *tp) { -struct xfs_defer_ops *dop = tp->t_dfops; -struct xfs_defer_pending *dfp; -struct xfs_defer_pending *pli; -struct list_head *pwi; -struct list_head *n; - -trace_xfs_defer_cancel (NULL , dop, _RET_IP_); +struct xfs_mount *mp = tp->t_mountp; -/* -* Free the pending items. Caller should already have arranged -* for the intent items to be released. -*/ -list_for_each_entry_safe (dfp, pli, &dop->dop_intake, dfp_list) { -trace_xfs_defer_intake_cancel (NULL , dfp); -list_del (&dfp->dfp_list); -list_for_each_safe (pwi, n, &dfp->dfp_work) { -list_del (pwi); - dfp->dfp_count--; - dfp->dfp_type->cancel_item (pwi); - } -ASSERT (dfp->dfp_count ==0 ); -kmem_free (dfp); - } -list_for_each_entry_safe (dfp, pli, &dop->dop_pending, dfp_list) { -trace_xfs_defer_pending_cancel (NULL , dfp); -list_del (&dfp->dfp_list); -list_for_each_safe (pwi, n, &dfp->dfp_work) { -list_del (pwi); - dfp->dfp_count--; - dfp->dfp_type->cancel_item (pwi); - } -ASSERT (dfp->dfp_count ==0 ); -kmem_free (dfp); - } +trace_xfs_defer_cancel (mp, tp->t_dfops, _RET_IP_); +xfs_defer_cancel_list (mp, &tp->t_dfops->dop_intake); }/* Add an item for later deferred processing. */
@@ -547,7 +542,6 @@ xfs_defer_init(memset (dop,0 ,sizeof (struct xfs_defer_ops ));INIT_LIST_HEAD (&dop->dop_intake); -INIT_LIST_HEAD (&dop->dop_pending);if (tp) {ASSERT (tp->t_firstblock ==NULLFSBLOCK ); tp->t_dfops = dop;
@@ -571,7 +565,6 @@ xfs_defer_move(ASSERT (dst != src);list_splice_init (&src->dop_intake, &dst->dop_intake); -list_splice_init (&src->dop_pending, &dst->dop_pending);/* * Low free space mode was historically controlled by a dfops field.
fs/xfs/libxfs/xfs_defer.h +0 -1
@@ -41,7 +41,6 @@ int xfs_defer_finish_noroll(struct xfs_trans **tp);int xfs_defer_finish (struct xfs_trans **tp);void xfs_defer_cancel (struct xfs_trans *);void xfs_defer_init (struct xfs_trans *tp,struct xfs_defer_ops *dop); -bool xfs_defer_has_unfinished_work (struct xfs_trans *tp);void xfs_defer_move (struct xfs_trans *dtp,struct xfs_trans *stp);/* Description of a deferred type. */
fs/xfs/xfs_trace.h +2 -3
@@ -2392,9 +2392,8 @@ DEFINE_DEFER_EVENT(xfs_defer_finish_done);DEFINE_DEFER_ERROR_EVENT (xfs_defer_trans_roll_error);DEFINE_DEFER_ERROR_EVENT (xfs_defer_finish_error); -DEFINE_DEFER_PENDING_EVENT (xfs_defer_intake_work); -DEFINE_DEFER_PENDING_EVENT (xfs_defer_intake_cancel); -DEFINE_DEFER_PENDING_EVENT (xfs_defer_pending_cancel); +DEFINE_DEFER_PENDING_EVENT (xfs_defer_create_intent); +DEFINE_DEFER_PENDING_EVENT (xfs_defer_cancel_list);DEFINE_DEFER_PENDING_EVENT (xfs_defer_pending_finish);DEFINE_DEFER_PENDING_EVENT (xfs_defer_pending_abort);
fs/xfs/xfs_trans.c +1 -1
@@ -929,7 +929,7 @@ __xfs_trans_commit(* Finish deferred items on final commit. Only permanent transactions * should ever have deferred ops. */ -WARN_ON_ONCE (xfs_defer_has_unfinished_work (tp->t_dfops) && +WARN_ON_ONCE (!list_empty (&tp->t_dfops->dop_intake) && !(tp->t_flags &XFS_TRANS_PERM_LOG_RES ));if (!regrant && (tp->t_flags &XFS_TRANS_PERM_LOG_RES )) { error =xfs_defer_finish_noroll (&tp);
fs/xfs/xfs_trans.h +0 -1
@@ -96,7 +96,6 @@ void xfs_log_item_init(struct xfs_mount *mp, struct xfs_log_item *item,#define XFS_DEFER_OPS_NR_BUFS 2/* join up to two buffers */ struct xfs_defer_ops {struct list_head dop_intake;/* unlogged pending work */ -struct list_head dop_pending;/* logged pending work */ };/*
Signature · 1 line
2.17.1
Re: [PATCH v2 12/15] xfs: replace xfs_defer_ops ->dop_pending with on-stack list
Quoted 10 lines · Brian Foster
The
xfs_defer_ops->dop_pending list is used to track active deferred operations once intents are logged. These items must be aborted in the event of an error. The list is populated as intents are logged and items are removed as they complete (or are aborted).Now that
xfs_defer_finish()cancels on error, there is no need to ever access ->dop_pending outside ofxfs_defer_finish(). The list is only ever populated afterxfs_defer_finish()begins and is either completed or cancelled before it returns.
Quoted 8 lines
Remove ->dop_pending from
xfs_defer_opsand replace it with a local list in thexfs_defer_finish()path. Pass the local list to the various helpers now that it is not accessible via dfops. Note that we have to check for NULL in the abort case as the final tx roll occurs outside of the scope of the new local list (once the dfops has completed and thus drained the list).Signed-off-by Brian Foster <[email protected]>
/me wipes the sweat off his brow, bfoster having cleaned up all the weird dfops/firstblock/tp lifetime rules. Thanks a bunch for all this work! :)
(Will send out for testing and report back, esp. since we now have an extra week.)
--D
Quoted 335 lines
--- fs/xfs/libxfs/xfs_defer.c | 129 ++++++++++++++++++-------------------- fs/xfs/libxfs/xfs_defer.h | 1 - fs/xfs/xfs_trace.h | 5 +- fs/xfs/xfs_trans.c | 2 +- fs/xfs/xfs_trans.h | 1 - 5 files changed, 64 insertions(+), 74 deletions(-)fs/xfs/libxfs/xfs_defer.c +61
-68@@ -180,7 +180,7 @@ static const struct xfs_defer_op_type *defer_op_types[XFS_DEFER_OPS_TYPE_MAX];* the pending list. */ STATIC void -xfs_defer_intake_work ( +xfs_defer_create_intents (struct xfs_trans *tp) {struct xfs_defer_ops *dop = tp->t_dfops;@@ -190,20 +190,19 @@ xfs_defer_intake_work(list_for_each_entry (dfp, &dop->dop_intake, dfp_list) { dfp->dfp_intent = dfp->dfp_type->create_intent (tp, dfp->dfp_count); -trace_xfs_defer_intake_work (tp->t_mountp, dfp); +trace_xfs_defer_create_intent (tp->t_mountp, dfp);list_sort (tp->t_mountp, &dfp->dfp_work, dfp->dfp_type->diff_items);list_for_each (li, &dfp->dfp_work) dfp->dfp_type->log_item (tp, dfp->dfp_intent, li); } - -list_splice_tail_init (&dop->dop_intake, &dop->dop_pending); }/* Abort all the intents that were committed. */ STATIC voidxfs_defer_trans_abort (struct xfs_trans *tp, +struct list_head *dop_pending,int error) {struct xfs_defer_ops *dop = tp->t_dfops;@@ -212,11 +211,13 @@ xfs_defer_trans_abort(trace_xfs_defer_trans_abort (tp->t_mountp, dop, _RET_IP_);/* Abort intent items that don't have a done item. */ -list_for_each_entry (dfp, &dop->dop_pending, dfp_list) { -trace_xfs_defer_pending_abort (tp->t_mountp, dfp); -if (dfp->dfp_intent && !dfp->dfp_done) { - dfp->dfp_type->abort_intent (dfp->dfp_intent); - dfp->dfp_intent =NULL ; +if (dop_pending) { +list_for_each_entry (dfp, dop_pending, dfp_list) { +trace_xfs_defer_pending_abort (tp->t_mountp, dfp); +if (dfp->dfp_intent && !dfp->dfp_done) { + dfp->dfp_type->abort_intent (dfp->dfp_intent); + dfp->dfp_intent =NULL ; + } } }@@ -228,7 +229,8 @@ xfs_defer_trans_abort(/* Roll a transaction so we can do some deferred op processing. */ STATIC intxfs_defer_trans_roll ( -struct xfs_trans **tp) +struct xfs_trans **tp, +struct list_head *dop_pending) {struct xfs_buf_log_item *bli;struct xfs_inode_log_item *ili;@@ -278,7 +280,7 @@ xfs_defer_trans_roll(if (error) {trace_xfs_defer_trans_roll_error ((*tp)->t_mountp, (*tp)->t_dfops, error); -xfs_defer_trans_abort (*tp, error); +xfs_defer_trans_abort (*tp, dop_pending, error);return error; }@@ -295,15 +297,6 @@ xfs_defer_trans_roll(return error; } -/* Do we have any work items to finish? */ -bool -xfs_defer_has_unfinished_work ( -struct xfs_trans *tp) -{ -return !list_empty (&tp->t_dfops->dop_pending) || - !list_empty (&tp->t_dfops->dop_intake); -} -/* * Reset an already used dfops after finish. */ @@ -311,7 +304,7 @@ static voidxfs_defer_reset ( struct xfs_trans *tp) { -ASSERT (!xfs_defer_has_unfinished_work (tp)); +ASSERT (list_empty (&tp->t_dfops->dop_intake));/* * Low mode state transfers across transaction rolls to mirror dfops @@ -320,6 +313,36 @@ xfs_defer_reset( tp->t_flags &= ~XFS_TRANS_LOWMODE ; } +/* +* Free up any items left in the list. +*/ +static void +xfs_defer_cancel_list ( +struct xfs_mount *mp, +struct list_head *dop_list) +{ +struct xfs_defer_pending *dfp; +struct xfs_defer_pending *pli; +struct list_head *pwi; +struct list_head *n; + +/* +* Free the pending items. Caller should already have arranged +* for the intent items to be released. +*/ +list_for_each_entry_safe (dfp, pli, dop_list, dfp_list) { +trace_xfs_defer_cancel_list (mp, dfp); +list_del (&dfp->dfp_list); +list_for_each_safe (pwi, n, &dfp->dfp_work) { +list_del (pwi); + dfp->dfp_count--; + dfp->dfp_type->cancel_item (pwi); + } +ASSERT (dfp->dfp_count ==0 ); +kmem_free (dfp); + } +} +/* * Finish all the pending work. This involves logging intent items for * any work items that wandered in since the last transaction roll (if @@ -338,26 +361,30 @@ xfs_defer_finish_noroll(void *state;int error =0 ;void (*cleanup_fn)(struct xfs_trans *,void *,int ); +LIST_HEAD (dop_pending);ASSERT ((*tp)->t_flags &XFS_TRANS_PERM_LOG_RES );trace_xfs_defer_finish ((*tp)->t_mountp, (*tp)->t_dfops, _RET_IP_);/* Until we run out of pending work to finish... */ -while (xfs_defer_has_unfinished_work (*tp)) { -/* Log intents for work items sitting in the intake. */ -xfs_defer_intake_work (*tp); +while (!list_empty (&dop_pending) || + !list_empty (&(*tp)->t_dfops->dop_intake)) { +/* log intents and pull in intake items */ +xfs_defer_create_intents (*tp); +list_splice_tail_init (&(*tp)->t_dfops->dop_intake, + &dop_pending);/* * Roll the transaction. */ - error =xfs_defer_trans_roll (tp); + error =xfs_defer_trans_roll (tp, &dop_pending);if (error) goto out;/* Log an intent-done item for the first pending item. */ - dfp =list_first_entry (&(*tp)->t_dfops->dop_pending, - struct xfs_defer_pending, dfp_list); + dfp = list_first_entry(&dop_pending, struct xfs_defer_pending, + dfp_list);trace_xfs_defer_pending_finish ((*tp)->t_mountp, dfp); dfp->dfp_done = dfp->dfp_type->create_done (*tp, dfp->dfp_intent, dfp->dfp_count);@@ -387,7 +414,7 @@ xfs_defer_finish_noroll(*/ if (cleanup_fn)cleanup_fn (*tp, state, error); -xfs_defer_trans_abort (*tp, error); +xfs_defer_trans_abort (*tp, &dop_pending, error); goto out; } }@@ -419,6 +446,7 @@ xfs_defer_finish_noroll(if (error) {trace_xfs_defer_finish_error ((*tp)->t_mountp, (*tp)->t_dfops, error); +xfs_defer_cancel_list ((*tp)->t_mountp, &dop_pending);xfs_defer_cancel (*tp);return error; }@@ -441,7 +469,7 @@ xfs_defer_finish(if (error)return error;if ((*tp)->t_flags &XFS_TRANS_DIRTY ) { - error =xfs_defer_trans_roll (tp); + error =xfs_defer_trans_roll (tp,NULL );if (error)return error; }@@ -449,47 +477,14 @@ xfs_defer_finish(return 0 ; } -/* -* Free up any items left in the list. -*/ void xfs_defer_cancel ( -struct xfs_trans *tp) +struct xfs_trans *tp) { -struct xfs_defer_ops *dop = tp->t_dfops; -struct xfs_defer_pending *dfp; -struct xfs_defer_pending *pli; -struct list_head *pwi; -struct list_head *n; - -trace_xfs_defer_cancel (NULL , dop, _RET_IP_); +struct xfs_mount *mp = tp->t_mountp; -/* -* Free the pending items. Caller should already have arranged -* for the intent items to be released. -*/ -list_for_each_entry_safe (dfp, pli, &dop->dop_intake, dfp_list) { -trace_xfs_defer_intake_cancel (NULL , dfp); -list_del (&dfp->dfp_list); -list_for_each_safe (pwi, n, &dfp->dfp_work) { -list_del (pwi); - dfp->dfp_count--; - dfp->dfp_type->cancel_item (pwi); - } -ASSERT (dfp->dfp_count ==0 ); -kmem_free (dfp); - } -list_for_each_entry_safe (dfp, pli, &dop->dop_pending, dfp_list) { -trace_xfs_defer_pending_cancel (NULL , dfp); -list_del (&dfp->dfp_list); -list_for_each_safe (pwi, n, &dfp->dfp_work) { -list_del (pwi); - dfp->dfp_count--; - dfp->dfp_type->cancel_item (pwi); - } -ASSERT (dfp->dfp_count ==0 ); -kmem_free (dfp); - } +trace_xfs_defer_cancel (mp, tp->t_dfops, _RET_IP_); +xfs_defer_cancel_list (mp, &tp->t_dfops->dop_intake); }/* Add an item for later deferred processing. */ @@ -547,7 +542,6 @@ xfs_defer_init(memset (dop,0 ,sizeof (struct xfs_defer_ops ));INIT_LIST_HEAD (&dop->dop_intake); -INIT_LIST_HEAD (&dop->dop_pending);if (tp) {ASSERT (tp->t_firstblock ==NULLFSBLOCK ); tp->t_dfops = dop;@@ -571,7 +565,6 @@ xfs_defer_move(ASSERT (dst != src);list_splice_init (&src->dop_intake, &dst->dop_intake); -list_splice_init (&src->dop_pending, &dst->dop_pending);/* * Low free space mode was historically controlled by a dfops field. fs/xfs/libxfs/xfs_defer.h +0
-1@@ -41,7 +41,6 @@ int xfs_defer_finish_noroll(struct xfs_trans **tp);int xfs_defer_finish (struct xfs_trans **tp);void xfs_defer_cancel (struct xfs_trans *);void xfs_defer_init (struct xfs_trans *tp,struct xfs_defer_ops *dop); -bool xfs_defer_has_unfinished_work (struct xfs_trans *tp);void xfs_defer_move (struct xfs_trans *dtp,struct xfs_trans *stp);/* Description of a deferred type. */ fs/xfs/xfs_trace.h +2
-3@@ -2392,9 +2392,8 @@ DEFINE_DEFER_EVENT(xfs_defer_finish_done);DEFINE_DEFER_ERROR_EVENT (xfs_defer_trans_roll_error);DEFINE_DEFER_ERROR_EVENT (xfs_defer_finish_error); -DEFINE_DEFER_PENDING_EVENT (xfs_defer_intake_work); -DEFINE_DEFER_PENDING_EVENT (xfs_defer_intake_cancel); -DEFINE_DEFER_PENDING_EVENT (xfs_defer_pending_cancel); +DEFINE_DEFER_PENDING_EVENT (xfs_defer_create_intent); +DEFINE_DEFER_PENDING_EVENT (xfs_defer_cancel_list);DEFINE_DEFER_PENDING_EVENT (xfs_defer_pending_finish);DEFINE_DEFER_PENDING_EVENT (xfs_defer_pending_abort);fs/xfs/xfs_trans.c +1
-1@@ -929,7 +929,7 @@ __xfs_trans_commit(* Finish deferred items on final commit. Only permanent transactions * should ever have deferred ops. */ -WARN_ON_ONCE (xfs_defer_has_unfinished_work (tp->t_dfops) && +WARN_ON_ONCE (!list_empty (&tp->t_dfops->dop_intake) && !(tp->t_flags &XFS_TRANS_PERM_LOG_RES ));if (!regrant && (tp->t_flags &XFS_TRANS_PERM_LOG_RES )) { error =xfs_defer_finish_noroll (&tp);fs/xfs/xfs_trans.h +0
-1@@ -96,7 +96,6 @@ void xfs_log_item_init(struct xfs_mount *mp, struct xfs_log_item *item,#define XFS_DEFER_OPS_NR_BUFS 2/* join up to two buffers */ struct xfs_defer_ops {struct list_head dop_intake;/* unlogged pending work */ -struct list_head dop_pending;/* logged pending work */ };/* Signature · 6 lines
2.17.1 -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 12/15] xfs: replace xfs_defer_ops ->dop_pending with on-stack list
In reply to Brian FosterQuoted 92 lines · Brian Foster
The
xfs_defer_ops->dop_pending list is used to track active deferred operations once intents are logged. These items must be aborted in the event of an error. The list is populated as intents are logged and items are removed as they complete (or are aborted).Now that
xfs_defer_finish()cancels on error, there is no need to ever access ->dop_pending outside ofxfs_defer_finish(). The list is only ever populated afterxfs_defer_finish()begins and is either completed or cancelled before it returns.Remove ->dop_pending from
xfs_defer_opsand replace it with a local list in thexfs_defer_finish()path. Pass the local list to the various helpers now that it is not accessible via dfops. Note that we have to check for NULL in the abort case as the final tx roll occurs outside of the scope of the new local list (once the dfops has completed and thus drained the list).Signed-off-by Brian Foster <[email protected]>--- fs/xfs/libxfs/xfs_defer.c | 129 ++++++++++++++++++-------------------- fs/xfs/libxfs/xfs_defer.h | 1 - fs/xfs/xfs_trace.h | 5 +- fs/xfs/xfs_trans.c | 2 +- fs/xfs/xfs_trans.h | 1 - 5 files changed, 64 insertions(+), 74 deletions(-)fs/xfs/libxfs/xfs_defer.c +12
-10@@ -180,7 +180,7 @@ static const struct xfs_defer_op_type *defer_op_types[XFS_DEFER_OPS_TYPE_MAX];* the pending list. */ STATIC void -xfs_defer_intake_work ( +xfs_defer_create_intents (struct xfs_trans *tp) {struct xfs_defer_ops *dop = tp->t_dfops;@@ -190,20 +190,19 @@ xfs_defer_intake_work(list_for_each_entry (dfp, &dop->dop_intake, dfp_list) { dfp->dfp_intent = dfp->dfp_type->create_intent (tp, dfp->dfp_count); -trace_xfs_defer_intake_work (tp->t_mountp, dfp); +trace_xfs_defer_create_intent (tp->t_mountp, dfp);list_sort (tp->t_mountp, &dfp->dfp_work, dfp->dfp_type->diff_items);list_for_each (li, &dfp->dfp_work) dfp->dfp_type->log_item (tp, dfp->dfp_intent, li); } - -list_splice_tail_init (&dop->dop_intake, &dop->dop_pending); }/* Abort all the intents that were committed. */ STATIC voidxfs_defer_trans_abort (struct xfs_trans *tp, +struct list_head *dop_pending,int error) {struct xfs_defer_ops *dop = tp->t_dfops;@@ -212,11 +211,13 @@ xfs_defer_trans_abort(trace_xfs_defer_trans_abort (tp->t_mountp, dop, _RET_IP_);/* Abort intent items that don't have a done item. */ -list_for_each_entry (dfp, &dop->dop_pending, dfp_list) { -trace_xfs_defer_pending_abort (tp->t_mountp, dfp); -if (dfp->dfp_intent && !dfp->dfp_done) { - dfp->dfp_type->abort_intent (dfp->dfp_intent); - dfp->dfp_intent =NULL ; +if (dop_pending) { +list_for_each_entry (dfp, dop_pending, dfp_list) { +trace_xfs_defer_pending_abort (tp->t_mountp, dfp); +if (dfp->dfp_intent && !dfp->dfp_done) { + dfp->dfp_type->abort_intent (dfp->dfp_intent); + dfp->dfp_intent =NULL ; + } } }@@ -228,7 +229,8 @@ xfs_defer_trans_abort(/* Roll a transaction so we can do some deferred op processing. */ STATIC intxfs_defer_trans_roll ( -struct xfs_trans **tp) +struct xfs_trans **tp, +struct list_head *dop_pending) {struct xfs_buf_log_item *bli;struct xfs_inode_log_item *ili;
fs/xfs/libxfs/xfs_defer.c xfs_defer_trans_roll( · line 283
Quoted 5 lines
fs/xfs/libxfs/xfs_defer.c +1
-1@@ -278,7 +280,7 @@ xfs_defer_trans_roll(if (error) {trace_xfs_defer_trans_roll_error ((*tp)->t_mountp, (*tp)->t_dfops, error); -xfs_defer_trans_abort (*tp, error); +xfs_defer_trans_abort (*tp, dop_pending, error);
Can you just move this call into the caller? That way we don't need to pass dop_pending at all, xfs_defer_finish can just do the plain shutdown instead of calling xfs_defer_trans_abort, we can remove the NULL dop handling from xfs_defer_trans_abort. In fact we could also move the shutdown out of xfs_defer_trans_abort entirely and stop passing the errno to it.
fs/xfs/libxfs/xfs_defer.c xfs_defer_finish_noroll( · line 376
Quoted 9 lines
fs/xfs/libxfs/xfs_defer.c +6
-3-while (xfs_defer_has_unfinished_work (*tp)) { -/* Log intents for work items sitting in the intake. */ -xfs_defer_intake_work (*tp); +while (!list_empty (&dop_pending) || + !list_empty (&(*tp)->t_dfops->dop_intake)) { +/* log intents and pull in intake items */ +xfs_defer_create_intents (*tp); +list_splice_tail_init (&(*tp)->t_dfops->dop_intake, + &dop_pending);
Btw, it would be nice to rename tp to tpp in this function and have a local tp without the double indirection. Reading the current code is a bit of a pain.
Otherwise looks good:
Re: [PATCH v2 12/15] xfs: replace xfs_defer_ops ->dop_pending with on-stack list
Quoted 31 lines · Christoph Hellwig
Quoted 30 lines · Brian Foster
The
xfs_defer_ops->dop_pending list is used to track active deferred operations once intents are logged. These items must be aborted in the event of an error. The list is populated as intents are logged and items are removed as they complete (or are aborted).Now that
xfs_defer_finish()cancels on error, there is no need to ever access ->dop_pending outside ofxfs_defer_finish(). The list is only ever populated afterxfs_defer_finish()begins and is either completed or cancelled before it returns.Remove ->dop_pending from
xfs_defer_opsand replace it with a local list in thexfs_defer_finish()path. Pass the local list to the various helpers now that it is not accessible via dfops. Note that we have to check for NULL in the abort case as the final tx roll occurs outside of the scope of the new local list (once the dfops has completed and thus drained the list).Signed-off-by Brian Foster <[email protected]>--- fs/xfs/libxfs/xfs_defer.c | 129 ++++++++++++++++++-------------------- fs/xfs/libxfs/xfs_defer.h | 1 - fs/xfs/xfs_trace.h | 5 +- fs/xfs/xfs_trans.c | 2 +- fs/xfs/xfs_trans.h | 1 - 5 files changed, 64 insertions(+), 74 deletions(-)fs/xfs/libxfs/xfs_defer.c +0
-0
...
Quoted 15 lines
Quoted 15 lines
fs/xfs/libxfs/xfs_defer.c +2
-2@@ -228,7 +229,8 @@ xfs_defer_trans_abort(/* Roll a transaction so we can do some deferred op processing. */ STATIC intxfs_defer_trans_roll ( -struct xfs_trans **tp) +struct xfs_trans **tp, +struct list_head *dop_pending) {struct xfs_buf_log_item *bli;struct xfs_inode_log_item *ili;@@ -278,7 +280,7 @@ xfs_defer_trans_roll(if (error) {trace_xfs_defer_trans_roll_error ((*tp)->t_mountp, (*tp)->t_dfops, error); -xfs_defer_trans_abort (*tp, error);
Quoted 9 lines
fs/xfs/libxfs/xfs_defer.c xfs_defer_trans_roll( · line 283
Quoted 1 line
fs/xfs/libxfs/xfs_defer.c +1
-0+xfs_defer_trans_abort (*tp, dop_pending, error);Can you just move this call into the caller? That way we don't need to pass
dop_pendingat all,xfs_defer_finishcan just do the plain shutdown instead of callingxfs_defer_trans_abort, we can remove the NULL dop handling fromxfs_defer_trans_abort. In fact we could also move the shutdown out ofxfs_defer_trans_abortentirely and stop passing the errno to it.
Sure. I dropped the SHUTDOWN_META_IO_ERROR usage as well because it reduces the churn a little bit and outside of here only seems to be used on legitimate I/O errors.
Quoted 5 lines
Quoted 5 lines
fs/xfs/libxfs/xfs_defer.c +2
-3-while (xfs_defer_has_unfinished_work (*tp)) { -/* Log intents for work items sitting in the intake. */ -xfs_defer_intake_work (*tp); +while (!list_empty (&dop_pending) || + !list_empty (&(*tp)->t_dfops->dop_intake)) {
Quoted 9 lines
fs/xfs/libxfs/xfs_defer.c xfs_defer_finish_noroll( · line 376
Quoted 4 lines
fs/xfs/libxfs/xfs_defer.c +4
-0+/* log intents and pull in intake items */ +xfs_defer_create_intents (*tp); +list_splice_tail_init (&(*tp)->t_dfops->dop_intake, + &dop_pending);Btw, it would be nice to rename tp to tpp in this function and have a local tp without the double indirection. Reading the current code is a bit of a pain.
Ok.
Brian
Quoted 7 lines
Otherwise looks good:
Reviewed-by Christoph Hellwig <[email protected]>--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v3 12/15 xfs: replace xfs_defer_ops ->dop_pending with on-stack list
In reply to Brian FosterThe xfs_defer_ops ->dop_pending list is used to track active deferred operations once intents are logged. These items must be aborted in the event of an error. The list is populated as intents are logged and items are removed as they complete (or are aborted).
Now that xfs_defer_finish() cancels on error, there is no need to ever access ->dop_pending outside of xfs_defer_finish(). The list is only ever populated after xfs_defer_finish() begins and is either completed or cancelled before it returns.
Remove ->dop_pending from xfs_defer_ops and replace it with a local list in the xfs_defer_finish() path. Pass the local list to the various helpers now that it is not accessible via dfops. Note that we have to check for NULL in the abort case as the final tx roll occurs outside of the scope of the new local list (once the dfops has completed and thus drained the list).
---
v3:
- Refactor xfs_defer_trans_abort() usage to eliminate NULL dop_pending case.
- Drop use of metadata I/O error shutdown value.
- Refactor trans pointer usage in xfs_defer_trans_roll().
fs/xfs/libxfs/xfs_defer.c | 146 ++++++++++++++++++-------------------- fs/xfs/libxfs/xfs_defer.h | 1 - fs/xfs/xfs_trace.h | 5 +- fs/xfs/xfs_trans.c | 2 +- fs/xfs/xfs_trans.h | 1 - 5 files changed, 71 insertions(+), 84 deletions(-)
fs/xfs/libxfs/xfs_defer.c +68 -78
@@ -180,7 +180,7 @@ static const struct xfs_defer_op_type *defer_op_types[XFS_DEFER_OPS_TYPE_MAX];* the pending list. */ STATIC void -xfs_defer_intake_work ( +xfs_defer_create_intents (struct xfs_trans *tp) {struct xfs_defer_ops *dop = tp->t_dfops;
@@ -190,21 +190,19 @@ xfs_defer_intake_work(list_for_each_entry (dfp, &dop->dop_intake, dfp_list) { dfp->dfp_intent = dfp->dfp_type->create_intent (tp, dfp->dfp_count); -trace_xfs_defer_intake_work (tp->t_mountp, dfp); +trace_xfs_defer_create_intent (tp->t_mountp, dfp);list_sort (tp->t_mountp, &dfp->dfp_work, dfp->dfp_type->diff_items);list_for_each (li, &dfp->dfp_work) dfp->dfp_type->log_item (tp, dfp->dfp_intent, li); } - -list_splice_tail_init (&dop->dop_intake, &dop->dop_pending); }/* Abort all the intents that were committed. */ STATIC voidxfs_defer_trans_abort (struct xfs_trans *tp, -int error) +struct list_head *dop_pending) {struct xfs_defer_ops *dop = tp->t_dfops;struct xfs_defer_pending *dfp;
@@ -212,24 +210,21 @@ xfs_defer_trans_abort(trace_xfs_defer_trans_abort (tp->t_mountp, dop, _RET_IP_);/* Abort intent items that don't have a done item. */ -list_for_each_entry (dfp, &dop->dop_pending, dfp_list) { +list_for_each_entry (dfp, dop_pending, dfp_list) {trace_xfs_defer_pending_abort (tp->t_mountp, dfp);if (dfp->dfp_intent && !dfp->dfp_done) { dfp->dfp_type->abort_intent (dfp->dfp_intent); dfp->dfp_intent =NULL ; } } - -/* Shut down FS. */ -xfs_force_shutdown (tp->t_mountp, (error == -EFSCORRUPTED ) ? -SHUTDOWN_CORRUPT_INCORE :SHUTDOWN_META_IO_ERROR ); }/* Roll a transaction so we can do some deferred op processing. */ STATIC intxfs_defer_trans_roll ( -struct xfs_trans **tp) +struct xfs_trans **tpp) { +struct xfs_trans *tp = *tpp;struct xfs_buf_log_item *bli;struct xfs_inode_log_item *ili;struct xfs_log_item *lip;
@@ -239,7 +234,7 @@ xfs_defer_trans_roll(int i;int error; -list_for_each_entry (lip, &(*tp)->t_items, li_trans) { +list_for_each_entry (lip, &tp->t_items, li_trans) {switch (lip->li_type) {case XFS_LI_BUF : bli = container_of(lip, struct xfs_buf_log_item,
@@ -249,7 +244,7 @@ xfs_defer_trans_roll(ASSERT (0 );return -EFSCORRUPTED ; } -xfs_trans_dirty_buf (*tp, bli->bli_buf); +xfs_trans_dirty_buf (tp, bli->bli_buf); bplist[bpcount++] = bli->bli_buf; }break ;
@@ -261,7 +256,7 @@ xfs_defer_trans_roll(ASSERT (0 );return -EFSCORRUPTED ; } -xfs_trans_log_inode (*tp, ili->ili_inode, +xfs_trans_log_inode (tp, ili->ili_inode,XFS_ILOG_CORE ); iplist[ipcount++] = ili->ili_inode; }
@@ -271,39 +266,30 @@ xfs_defer_trans_roll( } } -trace_xfs_defer_trans_roll ((*tp)->t_mountp, (*tp)->t_dfops, _RET_IP_); +trace_xfs_defer_trans_roll (tp->t_mountp, tp->t_dfops, _RET_IP_);/* Roll the transaction. */ - error =xfs_trans_roll (tp); + error =xfs_trans_roll (tpp); + tp = *tpp;if (error) { -trace_xfs_defer_trans_roll_error ((*tp)->t_mountp, - (*tp)->t_dfops, error); -xfs_defer_trans_abort (*tp, error); +trace_xfs_defer_trans_roll_error (tp->t_mountp, + tp->t_dfops, error);return error; }/* Rejoin the joined inodes. */ for (i =0 ; i < ipcount; i++) -xfs_trans_ijoin (*tp, iplist[i],0 ); +xfs_trans_ijoin (tp, iplist[i],0 );/* Rejoin the buffers and dirty them so the log moves forward. */ for (i =0 ; i < bpcount; i++) { -xfs_trans_bjoin (*tp, bplist[i]); -xfs_trans_bhold (*tp, bplist[i]); +xfs_trans_bjoin (tp, bplist[i]); +xfs_trans_bhold (tp, bplist[i]); }return error; } -/* Do we have any work items to finish? */ -bool -xfs_defer_has_unfinished_work ( -struct xfs_trans *tp) -{ -return !list_empty (&tp->t_dfops->dop_pending) || - !list_empty (&tp->t_dfops->dop_intake); -} -/* * Reset an already used dfops after finish. */
@@ -311,7 +297,7 @@ static voidxfs_defer_reset ( struct xfs_trans *tp) { -ASSERT (!xfs_defer_has_unfinished_work (tp)); +ASSERT (list_empty (&tp->t_dfops->dop_intake));/* * Low mode state transfers across transaction rolls to mirror dfops
@@ -320,6 +306,36 @@ xfs_defer_reset( tp->t_flags &= ~XFS_TRANS_LOWMODE ; } +/* +* Free up any items left in the list. +*/ +static void +xfs_defer_cancel_list ( +struct xfs_mount *mp, +struct list_head *dop_list) +{ +struct xfs_defer_pending *dfp; +struct xfs_defer_pending *pli; +struct list_head *pwi; +struct list_head *n; + +/* +* Free the pending items. Caller should already have arranged +* for the intent items to be released. +*/ +list_for_each_entry_safe (dfp, pli, dop_list, dfp_list) { +trace_xfs_defer_cancel_list (mp, dfp); +list_del (&dfp->dfp_list); +list_for_each_safe (pwi, n, &dfp->dfp_work) { +list_del (pwi); + dfp->dfp_count--; + dfp->dfp_type->cancel_item (pwi); + } +ASSERT (dfp->dfp_count ==0 ); +kmem_free (dfp); + } +} +/* * Finish all the pending work. This involves logging intent items for * any work items that wandered in since the last transaction roll (if
@@ -338,15 +354,19 @@ xfs_defer_finish_noroll(void *state;int error =0 ;void (*cleanup_fn)(struct xfs_trans *,void *,int ); +LIST_HEAD (dop_pending);ASSERT ((*tp)->t_flags &XFS_TRANS_PERM_LOG_RES );trace_xfs_defer_finish ((*tp)->t_mountp, (*tp)->t_dfops, _RET_IP_);/* Until we run out of pending work to finish... */ -while (xfs_defer_has_unfinished_work (*tp)) { -/* Log intents for work items sitting in the intake. */ -xfs_defer_intake_work (*tp); +while (!list_empty (&dop_pending) || + !list_empty (&(*tp)->t_dfops->dop_intake)) { +/* log intents and pull in intake items */ +xfs_defer_create_intents (*tp); +list_splice_tail_init (&(*tp)->t_dfops->dop_intake, + &dop_pending);/* * Roll the transaction.
@@ -356,8 +376,8 @@ xfs_defer_finish_noroll( goto out;/* Log an intent-done item for the first pending item. */ - dfp =list_first_entry (&(*tp)->t_dfops->dop_pending, - struct xfs_defer_pending, dfp_list); + dfp = list_first_entry(&dop_pending, struct xfs_defer_pending, + dfp_list);trace_xfs_defer_pending_finish ((*tp)->t_mountp, dfp); dfp->dfp_done = dfp->dfp_type->create_done (*tp, dfp->dfp_intent, dfp->dfp_count);
@@ -387,7 +407,6 @@ xfs_defer_finish_noroll(*/ if (cleanup_fn)cleanup_fn (*tp, state, error); -xfs_defer_trans_abort (*tp, error); goto out; } }
@@ -417,8 +436,11 @@ xfs_defer_finish_noroll( out:if (error) { +xfs_defer_trans_abort (*tp, &dop_pending); +xfs_force_shutdown ((*tp)->t_mountp,SHUTDOWN_CORRUPT_INCORE );trace_xfs_defer_finish_error ((*tp)->t_mountp, (*tp)->t_dfops, error); +xfs_defer_cancel_list ((*tp)->t_mountp, &dop_pending);xfs_defer_cancel (*tp);return error; }
@@ -442,54 +464,24 @@ xfs_defer_finish(return error;if ((*tp)->t_flags &XFS_TRANS_DIRTY ) { error =xfs_defer_trans_roll (tp); -if (error) +if (error) { +xfs_force_shutdown ((*tp)->t_mountp, +SHUTDOWN_CORRUPT_INCORE );return error; + } }xfs_defer_reset (*tp);return 0 ; } -/* -* Free up any items left in the list. -*/ void xfs_defer_cancel ( -struct xfs_trans *tp) +struct xfs_trans *tp) { -struct xfs_defer_ops *dop = tp->t_dfops; -struct xfs_defer_pending *dfp; -struct xfs_defer_pending *pli; -struct list_head *pwi; -struct list_head *n; +struct xfs_mount *mp = tp->t_mountp; -trace_xfs_defer_cancel (NULL , dop, _RET_IP_); - -/* -* Free the pending items. Caller should already have arranged -* for the intent items to be released. -*/ -list_for_each_entry_safe (dfp, pli, &dop->dop_intake, dfp_list) { -trace_xfs_defer_intake_cancel (NULL , dfp); -list_del (&dfp->dfp_list); -list_for_each_safe (pwi, n, &dfp->dfp_work) { -list_del (pwi); - dfp->dfp_count--; - dfp->dfp_type->cancel_item (pwi); - } -ASSERT (dfp->dfp_count ==0 ); -kmem_free (dfp); - } -list_for_each_entry_safe (dfp, pli, &dop->dop_pending, dfp_list) { -trace_xfs_defer_pending_cancel (NULL , dfp); -list_del (&dfp->dfp_list); -list_for_each_safe (pwi, n, &dfp->dfp_work) { -list_del (pwi); - dfp->dfp_count--; - dfp->dfp_type->cancel_item (pwi); - } -ASSERT (dfp->dfp_count ==0 ); -kmem_free (dfp); - } +trace_xfs_defer_cancel (mp, tp->t_dfops, _RET_IP_); +xfs_defer_cancel_list (mp, &tp->t_dfops->dop_intake); }/* Add an item for later deferred processing. */
@@ -547,7 +539,6 @@ xfs_defer_init(memset (dop,0 ,sizeof (struct xfs_defer_ops ));INIT_LIST_HEAD (&dop->dop_intake); -INIT_LIST_HEAD (&dop->dop_pending);if (tp) {ASSERT (tp->t_firstblock ==NULLFSBLOCK ); tp->t_dfops = dop;
@@ -571,7 +562,6 @@ xfs_defer_move(ASSERT (dst != src);list_splice_init (&src->dop_intake, &dst->dop_intake); -list_splice_init (&src->dop_pending, &dst->dop_pending);/* * Low free space mode was historically controlled by a dfops field.
fs/xfs/libxfs/xfs_defer.h +0 -1
@@ -41,7 +41,6 @@ int xfs_defer_finish_noroll(struct xfs_trans **tp);int xfs_defer_finish (struct xfs_trans **tp);void xfs_defer_cancel (struct xfs_trans *);void xfs_defer_init (struct xfs_trans *tp,struct xfs_defer_ops *dop); -bool xfs_defer_has_unfinished_work (struct xfs_trans *tp);void xfs_defer_move (struct xfs_trans *dtp,struct xfs_trans *stp);/* Description of a deferred type. */
fs/xfs/xfs_trace.h +2 -3
@@ -2392,9 +2392,8 @@ DEFINE_DEFER_EVENT(xfs_defer_finish_done);DEFINE_DEFER_ERROR_EVENT (xfs_defer_trans_roll_error);DEFINE_DEFER_ERROR_EVENT (xfs_defer_finish_error); -DEFINE_DEFER_PENDING_EVENT (xfs_defer_intake_work); -DEFINE_DEFER_PENDING_EVENT (xfs_defer_intake_cancel); -DEFINE_DEFER_PENDING_EVENT (xfs_defer_pending_cancel); +DEFINE_DEFER_PENDING_EVENT (xfs_defer_create_intent); +DEFINE_DEFER_PENDING_EVENT (xfs_defer_cancel_list);DEFINE_DEFER_PENDING_EVENT (xfs_defer_pending_finish);DEFINE_DEFER_PENDING_EVENT (xfs_defer_pending_abort);
fs/xfs/xfs_trans.c +1 -1
@@ -929,7 +929,7 @@ __xfs_trans_commit(* Finish deferred items on final commit. Only permanent transactions * should ever have deferred ops. */ -WARN_ON_ONCE (xfs_defer_has_unfinished_work (tp->t_dfops) && +WARN_ON_ONCE (!list_empty (&tp->t_dfops->dop_intake) && !(tp->t_flags &XFS_TRANS_PERM_LOG_RES ));if (!regrant && (tp->t_flags &XFS_TRANS_PERM_LOG_RES )) { error =xfs_defer_finish_noroll (&tp);
fs/xfs/xfs_trans.h +0 -1
@@ -96,7 +96,6 @@ void xfs_log_item_init(struct xfs_mount *mp, struct xfs_log_item *item,#define XFS_DEFER_OPS_NR_BUFS 2/* join up to two buffers */ struct xfs_defer_ops {struct list_head dop_intake;/* unlogged pending work */ -struct list_head dop_pending;/* logged pending work */ };/*
Signature · 1 line
2.17.1
PATCH v2 13/15 xfs: pass transaction to xfs_defer_add()
In reply to Brian FosterThe majority of remaining references to struct xfs_defer_ops in XFS are associated with xfs_defer_add(). At this point, there are no more external xfs_defer_ops users left. All instances of xfs_defer_ops are embedded in the transaction, which means we can safely pass the transaction down to the dfops add interface.
Update xfs_defer_add() to receive the transaction as a parameter. Various subsystems implement wrappers to allocate and construct the context specific data structures for the associated deferred operation type. Update these to also carry the transaction down as needed and clean up unused dfops parameters along the way.
This removes most of the remaining references to struct xfs_defer_ops throughout the code and facilitates removal of the structure.
--- fs/xfs/libxfs/xfs_alloc.c | 9 ++-- fs/xfs/libxfs/xfs_bmap.c | 75 +++++++++++++++------------------ fs/xfs/libxfs/xfs_bmap.h | 19 ++++----- fs/xfs/libxfs/xfs_bmap_btree.c | 2 +- fs/xfs/libxfs/xfs_defer.c | 5 ++- fs/xfs/libxfs/xfs_defer.h | 2 +- fs/xfs/libxfs/xfs_ialloc.c | 25 +++++------ fs/xfs/libxfs/xfs_refcount.c | 76 ++++++++++++++-------------------- fs/xfs/libxfs/xfs_refcount.h | 18 ++++---- fs/xfs/libxfs/xfs_rmap.c | 53 ++++++++++++------------ fs/xfs/libxfs/xfs_rmap.h | 22 ++++------ fs/xfs/xfs_bmap_item.c | 3 +- fs/xfs/xfs_bmap_util.c | 13 ++---- fs/xfs/xfs_refcount_item.c | 14 ++----- fs/xfs/xfs_reflink.c | 21 ++++------ 15 files changed, 158 insertions(+), 199 deletions(-)
fs/xfs/libxfs/xfs_alloc.c +4 -5
@@ -2198,12 +2198,12 @@ xfs_agfl_reset(*/ STATIC voidxfs_defer_agfl_block ( -struct xfs_mount *mp, -struct xfs_defer_ops *dfops, +struct xfs_trans *tp,xfs_agnumber_t agno,xfs_fsblock_t agbno,struct xfs_owner_info *oinfo) { +struct xfs_mount *mp = tp->t_mountp;struct xfs_extent_free_item *new;/* new element */ ASSERT (xfs_bmap_free_item_zone !=NULL );
@@ -2216,7 +2216,7 @@ xfs_defer_agfl_block(trace_xfs_agfl_free_defer (mp, agno,0 , agbno,1 ); -xfs_defer_add (dfops,XFS_DEFER_OPS_TYPE_AGFL_FREE , &new->xefi_list); +xfs_defer_add (tp,XFS_DEFER_OPS_TYPE_AGFL_FREE , &new->xefi_list); }/*
@@ -2325,8 +2325,7 @@ xfs_alloc_fix_freelist(/* defer agfl frees if dfops is provided */ if (tp->t_dfops) { -xfs_defer_agfl_block (mp, tp->t_dfops, args->agno, - bno, &targs.oinfo); +xfs_defer_agfl_block (tp, args->agno, bno, &targs.oinfo); }else { error =xfs_free_agfl_block (tp, args->agno, bno, agbp, &targs.oinfo);
fs/xfs/libxfs/xfs_bmap.c +33 -42
247 lines not on this page · raw message
fs/xfs/libxfs/xfs_bmap.h +9 -10
@@ -184,9 +184,9 @@ void xfs_trim_extent(struct xfs_bmbt_irec *irec, xfs_fileoff_t bno,void xfs_trim_extent_eof (struct xfs_bmbt_irec *,struct xfs_inode *);int xfs_bmap_add_attrfork (struct xfs_inode *ip,int size,int rsvd);void xfs_bmap_local_to_extents_empty (struct xfs_inode *ip,int whichfork); -void __xfs_bmap_add_free (struct xfs_mount *mp,struct xfs_defer_ops *dfops, -xfs_fsblock_t bno,xfs_filblks_t len, -struct xfs_owner_info *oinfo,bool skip_discard); +void __xfs_bmap_add_free (struct xfs_trans *tp,xfs_fsblock_t bno, +xfs_filblks_t len,struct xfs_owner_info *oinfo, +bool skip_discard);void xfs_bmap_compute_maxlevels (struct xfs_mount *mp,int whichfork);int xfs_bmap_first_unused (struct xfs_trans *tp,struct xfs_inode *ip,xfs_extlen_t len,xfs_fileoff_t *unused,int whichfork);
@@ -230,13 +230,12 @@ int xfs_bmapi_reserve_delalloc(struct xfs_inode *ip, int whichfork,static inline void xfs_bmap_add_free ( -struct xfs_mount *mp, -struct xfs_defer_ops *dfops, +struct xfs_trans *tp,xfs_fsblock_t bno,xfs_filblks_t len,struct xfs_owner_info *oinfo) { -__xfs_bmap_add_free (mp, dfops, bno, len, oinfo, false); +__xfs_bmap_add_free (tp, bno, len, oinfo, false); }enum xfs_bmap_intent_type {
@@ -256,10 +255,10 @@ int xfs_bmap_finish_one(struct xfs_trans *tp, struct xfs_inode *ip, enum xfs_bmap_intent_type type, int whichfork, xfs_fileoff_t startoff, xfs_fsblock_t startblock, xfs_filblks_t *blockcount, xfs_exntst_t state); -int xfs_bmap_map_extent (struct xfs_mount *mp,struct xfs_defer_ops *dfops, -struct xfs_inode *ip,struct xfs_bmbt_irec *imap); -int xfs_bmap_unmap_extent (struct xfs_mount *mp,struct xfs_defer_ops *dfops, -struct xfs_inode *ip,struct xfs_bmbt_irec *imap); +int xfs_bmap_map_extent (struct xfs_trans *tp,struct xfs_inode *ip, +struct xfs_bmbt_irec *imap); +int xfs_bmap_unmap_extent (struct xfs_trans *tp,struct xfs_inode *ip, +struct xfs_bmbt_irec *imap);static inline int xfs_bmap_fork_to_state (int whichfork) {
fs/xfs/libxfs/xfs_bmap_btree.c +1 -1
@@ -289,7 +289,7 @@ xfs_bmbt_free_block(struct xfs_owner_info oinfo;xfs_rmap_ino_bmbt_owner (&oinfo, ip->i_ino, cur->bc_private.b.whichfork); -xfs_bmap_add_free (mp, cur->bc_tp->t_dfops, fsbno,1 , &oinfo); +xfs_bmap_add_free (cur->bc_tp, fsbno,1 , &oinfo); ip->i_d.di_nblocks--;xfs_trans_log_inode (tp, ip,XFS_ILOG_CORE );
fs/xfs/libxfs/xfs_defer.c +4 -1
@@ -490,12 +490,15 @@ xfs_defer_cancel(/* Add an item for later deferred processing. */ void xfs_defer_add ( -struct xfs_defer_ops *dop, +struct xfs_trans *tp,enum xfs_defer_ops_type type,struct list_head *li) { +struct xfs_defer_ops *dop = tp->t_dfops;struct xfs_defer_pending *dfp =NULL ; +ASSERT (tp->t_flags &XFS_TRANS_PERM_LOG_RES ); +/* * Add the item to a pending item at the end of the intake list. * If the last pending item has the same type, reuse it. Else,
fs/xfs/libxfs/xfs_defer.h +1 -1
@@ -35,7 +35,7 @@ enum xfs_defer_ops_type { XFS_DEFER_OPS_TYPE_MAX, }; -void xfs_defer_add (struct xfs_defer_ops *dop,enum xfs_defer_ops_type type, +void xfs_defer_add (struct xfs_trans *tp,enum xfs_defer_ops_type type,struct list_head *h);int xfs_defer_finish_noroll (struct xfs_trans **tp);int xfs_defer_finish (struct xfs_trans **tp);
fs/xfs/libxfs/xfs_ialloc.c +13 -12
@@ -1838,23 +1838,24 @@ xfs_dialloc(*/ STATIC voidxfs_difree_inode_chunk ( -struct xfs_mount *mp, +struct xfs_trans *tp,xfs_agnumber_t agno, -struct xfs_inobt_rec_incore *rec, -struct xfs_defer_ops *dfops) +struct xfs_inobt_rec_incore *rec) { -xfs_agblock_t sagbno =XFS_AGINO_TO_AGBNO (mp, rec->ir_startino); -int startidx, endidx; -int nextbit; -xfs_agblock_t agbno; -int contigblk; -struct xfs_owner_info oinfo; +struct xfs_mount *mp = tp->t_mountp; +xfs_agblock_t sagbno =XFS_AGINO_TO_AGBNO (mp, + rec->ir_startino); +int startidx, endidx; +int nextbit; +xfs_agblock_t agbno; +int contigblk; +struct xfs_owner_info oinfo;DECLARE_BITMAP (holemask,XFS_INOBT_HOLEMASK_BITS );xfs_rmap_ag_owner (&oinfo,XFS_RMAP_OWN_INODES );if (!xfs_inobt_issparse (rec->ir_holemask)) {/* not sparse, calculate extent info directly */ -xfs_bmap_add_free (mp, dfops,XFS_AGB_TO_FSB (mp, agno, sagbno), +xfs_bmap_add_free (tp,XFS_AGB_TO_FSB (mp, agno, sagbno), mp->m_ialloc_blks, &oinfo);return ; }
@@ -1898,7 +1899,7 @@ xfs_difree_inode_chunk(ASSERT (agbno % mp->m_sb.sb_spino_align ==0 );ASSERT (contigblk % mp->m_sb.sb_spino_align ==0 ); -xfs_bmap_add_free (mp, dfops,XFS_AGB_TO_FSB (mp, agno, agbno), +xfs_bmap_add_free (tp,XFS_AGB_TO_FSB (mp, agno, agbno), contigblk, &oinfo);/* reset range to current bit and carry on... */
@@ -2002,7 +2003,7 @@ xfs_difree_inobt( goto error0; } -xfs_difree_inode_chunk (mp, agno, &rec, tp->t_dfops); +xfs_difree_inode_chunk (tp, agno, &rec); }else { xic->deleted = false;
fs/xfs/libxfs/xfs_refcount.c +32 -44
234 lines not on this page · raw message
fs/xfs/libxfs/xfs_refcount.h +8 -10
@@ -29,10 +29,10 @@ struct xfs_refcount_intent {xfs_extlen_t ri_blockcount; }; -extern int xfs_refcount_increase_extent (struct xfs_mount *mp, -struct xfs_defer_ops *dfops,struct xfs_bmbt_irec *irec); -extern int xfs_refcount_decrease_extent (struct xfs_mount *mp, -struct xfs_defer_ops *dfops,struct xfs_bmbt_irec *irec); +extern int xfs_refcount_increase_extent (struct xfs_trans *tp, +struct xfs_bmbt_irec *irec); +extern int xfs_refcount_decrease_extent (struct xfs_trans *tp, +struct xfs_bmbt_irec *irec);extern void xfs_refcount_finish_one_cleanup (struct xfs_trans *tp,struct xfs_btree_cur *rcur,int error);
@@ -45,12 +45,10 @@ extern int xfs_refcount_find_shared(struct xfs_btree_cur *cur, xfs_agblock_t agbno, xfs_extlen_t aglen, xfs_agblock_t *fbno, xfs_extlen_t *flen, bool find_end_of_shared); -extern int xfs_refcount_alloc_cow_extent (struct xfs_mount *mp, -struct xfs_defer_ops *dfops,xfs_fsblock_t fsb, -xfs_extlen_t len); -extern int xfs_refcount_free_cow_extent (struct xfs_mount *mp, -struct xfs_defer_ops *dfops,xfs_fsblock_t fsb, -xfs_extlen_t len); +extern int xfs_refcount_alloc_cow_extent (struct xfs_trans *tp, +xfs_fsblock_t fsb,xfs_extlen_t len); +extern int xfs_refcount_free_cow_extent (struct xfs_trans *tp, +xfs_fsblock_t fsb,xfs_extlen_t len);extern int xfs_refcount_recover_cow_leftovers (struct xfs_mount *mp,xfs_agnumber_t agno);
fs/xfs/libxfs/xfs_rmap.c +26 -27
142 lines not on this page · raw message
fs/xfs/libxfs/xfs_rmap.h +9 -13
@@ -185,21 +185,17 @@ struct xfs_rmap_intent { };/* functions for updating the rmapbt based on bmbt map/unmap operations */ -int xfs_rmap_map_extent (struct xfs_mount *mp,struct xfs_defer_ops *dfops, +int xfs_rmap_map_extent (struct xfs_trans *tp,struct xfs_inode *ip, +int whichfork,struct xfs_bmbt_irec *imap); +int xfs_rmap_unmap_extent (struct xfs_trans *tp,struct xfs_inode *ip, +int whichfork,struct xfs_bmbt_irec *imap); +int xfs_rmap_convert_extent (struct xfs_mount *mp,struct xfs_trans *tp,struct xfs_inode *ip,int whichfork,struct xfs_bmbt_irec *imap); -int xfs_rmap_unmap_extent (struct xfs_mount *mp,struct xfs_defer_ops *dfops, -struct xfs_inode *ip,int whichfork, -struct xfs_bmbt_irec *imap); -int xfs_rmap_convert_extent (struct xfs_mount *mp,struct xfs_defer_ops *dfops, -struct xfs_inode *ip,int whichfork, -struct xfs_bmbt_irec *imap); -int xfs_rmap_alloc_extent (struct xfs_mount *mp,struct xfs_defer_ops *dfops, -xfs_agnumber_t agno,xfs_agblock_t bno,xfs_extlen_t len, -uint64_t owner); -int xfs_rmap_free_extent (struct xfs_mount *mp,struct xfs_defer_ops *dfops, -xfs_agnumber_t agno,xfs_agblock_t bno,xfs_extlen_t len, -uint64_t owner); +int xfs_rmap_alloc_extent (struct xfs_trans *tp,xfs_agnumber_t agno, +xfs_agblock_t bno,xfs_extlen_t len,uint64_t owner); +int xfs_rmap_free_extent (struct xfs_trans *tp,xfs_agnumber_t agno, +xfs_agblock_t bno,xfs_extlen_t len,uint64_t owner);void xfs_rmap_finish_one_cleanup (struct xfs_trans *tp,struct xfs_btree_cur *rcur,int error);
fs/xfs/xfs_bmap_item.c +1 -2
@@ -486,8 +486,7 @@ xfs_bui_recover( irec.br_blockcount = count; irec.br_startoff = bmap->me_startoff; irec.br_state = state; - error =xfs_bmap_unmap_extent (tp->t_mountp, tp->t_dfops, ip, - &irec); + error =xfs_bmap_unmap_extent (tp, ip, &irec);if (error) goto err_inode; }
fs/xfs/xfs_bmap_util.c +4 -9
@@ -1534,7 +1534,6 @@ xfs_swap_extent_rmap( struct xfs_inode *tip) {struct xfs_trans *tp = *tpp; -struct xfs_mount *mp = tp->t_mountp;struct xfs_bmbt_irec irec;struct xfs_bmbt_irec uirec;struct xfs_bmbt_irec tirec;
@@ -1598,26 +1597,22 @@ xfs_swap_extent_rmap(trace_xfs_swap_extent_rmap_remap_piece (tip, &uirec);/* Remove the mapping from the donor file. */ - error =xfs_bmap_unmap_extent (mp, tp->t_dfops, tip, - &uirec); + error =xfs_bmap_unmap_extent (tp, tip, &uirec);if (error) goto out_defer;/* Remove the mapping from the source file. */ - error =xfs_bmap_unmap_extent (mp, tp->t_dfops, ip, - &irec); + error =xfs_bmap_unmap_extent (tp, ip, &irec);if (error) goto out_defer;/* Map the donor file's blocks into the source file. */ - error =xfs_bmap_map_extent (mp, tp->t_dfops, ip, - &uirec); + error =xfs_bmap_map_extent (tp, ip, &uirec);if (error) goto out_defer;/* Map the source file's blocks into the donor file. */ - error =xfs_bmap_map_extent (mp, tp->t_dfops, tip, - &irec); + error =xfs_bmap_map_extent (tp, tip, &irec);if (error) goto out_defer;
fs/xfs/xfs_refcount_item.c +4 -10
@@ -490,24 +490,18 @@ xfs_cui_recover( irec.br_blockcount = new_len;switch (type) {case XFS_REFCOUNT_INCREASE : - error =xfs_refcount_increase_extent ( - tp->t_mountp, tp->t_dfops, - &irec); + error =xfs_refcount_increase_extent (tp, &irec);break ;case XFS_REFCOUNT_DECREASE : - error =xfs_refcount_decrease_extent ( - tp->t_mountp, tp->t_dfops, - &irec); + error =xfs_refcount_decrease_extent (tp, &irec);break ;case XFS_REFCOUNT_ALLOC_COW : - error =xfs_refcount_alloc_cow_extent ( - tp->t_mountp, tp->t_dfops, + error =xfs_refcount_alloc_cow_extent (tp, irec.br_startblock, irec.br_blockcount);break ;case XFS_REFCOUNT_FREE_COW : - error =xfs_refcount_free_cow_extent ( - tp->t_mountp, tp->t_dfops, + error =xfs_refcount_free_cow_extent (tp, irec.br_startblock, irec.br_blockcount);break ;
fs/xfs/xfs_reflink.c +9 -12
@@ -506,15 +506,13 @@ xfs_reflink_cancel_cow_blocks(ASSERT ((*tpp)->t_firstblock ==NULLFSBLOCK );/* Free the CoW orphan record. */ - error =xfs_refcount_free_cow_extent (ip->i_mount, - (*tpp)->t_dfops, del.br_startblock, - del.br_blockcount); + error =xfs_refcount_free_cow_extent (*tpp, + del.br_startblock, del.br_blockcount);if (error)break ; -xfs_bmap_add_free (ip->i_mount, (*tpp)->t_dfops, - del.br_startblock, del.br_blockcount, -NULL ); +xfs_bmap_add_free (*tpp, del.br_startblock, + del.br_blockcount,NULL );/* Roll the transaction */ error =xfs_defer_finish (tpp);
@@ -694,14 +692,13 @@ xfs_reflink_end_cow(trace_xfs_reflink_cow_remap (ip, &del);/* Free the CoW orphan record. */ - error =xfs_refcount_free_cow_extent (tp->t_mountp, tp->t_dfops, - del.br_startblock, del.br_blockcount); + error =xfs_refcount_free_cow_extent (tp, del.br_startblock, + del.br_blockcount);if (error) goto out_cancel;/* Map the new blocks into the data fork. */ - error =xfs_bmap_map_extent (tp->t_mountp, tp->t_dfops, ip, - &del); + error =xfs_bmap_map_extent (tp, ip, &del);if (error) goto out_cancel;
@@ -1046,12 +1043,12 @@ xfs_reflink_remap_extent( uirec.br_blockcount, uirec.br_startblock);/* Update the refcount tree */ - error =xfs_refcount_increase_extent (mp, tp->t_dfops, &uirec); + error =xfs_refcount_increase_extent (tp, &uirec);if (error) goto out_cancel;/* Map the new blocks into the data fork. */ - error =xfs_bmap_map_extent (mp, tp->t_dfops, ip, &uirec); + error =xfs_bmap_map_extent (tp, ip, &uirec);if (error) goto out_cancel;
Signature · 1 line
2.17.1
Re: [PATCH v2 13/15] xfs: pass transaction to xfs_defer_add()
Quoted 12 lines · Brian Foster
The majority of remaining references to struct
xfs_defer_opsin XFS are associated withxfs_defer_add(). At this point, there are no more externalxfs_defer_opsusers left. All instances ofxfs_defer_opsare embedded in the transaction, which means we can safely pass the transaction down to the dfops add interface.Update
xfs_defer_add()to receive the transaction as a parameter. Various subsystems implement wrappers to allocate and construct the context specific data structures for the associated deferred operation type. Update these to also carry the transaction down as needed and clean up unused dfops parameters along the way.
Quoted 7 lines
This removes most of the remaining references to struct
xfs_defer_opsthroughout the code and facilitates removal of the structure.Signed-off-by Brian Foster <[email protected]>Reviewed-by Darrick J. Wong <[email protected]>Reviewed-by Christoph Hellwig <[email protected]>
FWIW I've made the following edits to this patch to get rid of the compiler warnings in xfsprogs and with CONFIG_FTRACE=n because our tracepoints don't do anything with the *mp local variable. There aren't any functional changes.
--D
fs/xfs/libxfs/xfs_bmap.c +9 -9
@@ -539,11 +539,11 @@ __xfs_bmap_add_free( struct xfs_owner_info *oinfo, bool skip_discard) { -struct xfs_mount *mp = tp->t_mountp;struct xfs_extent_free_item *new;/* new element */ #ifdef DEBUG -xfs_agnumber_t agno; -xfs_agblock_t agbno; +struct xfs_mount *mp = tp->t_mountp; +xfs_agnumber_t agno; +xfs_agblock_t agbno;ASSERT (bno !=NULLFSBLOCK );ASSERT (len >0 );
@@ -566,8 +566,9 @@ __xfs_bmap_add_free(else xfs_rmap_skip_owner_update (&new->xefi_oinfo); new->xefi_skip_discard = skip_discard; -trace_xfs_bmap_free_defer (mp,XFS_FSB_TO_AGNO (mp, bno),0 , -XFS_FSB_TO_AGBNO (mp, bno), len); +trace_xfs_bmap_free_defer (tp->t_mountp, +XFS_FSB_TO_AGNO (tp->t_mountp, bno),0 , +XFS_FSB_TO_AGBNO (tp->t_mountp, bno), len);xfs_defer_add (tp,XFS_DEFER_OPS_TYPE_FREE , &new->xefi_list); }
@@ -5980,13 +5981,12 @@ __xfs_bmap_add(int whichfork, struct xfs_bmbt_irec *bmap) { - struct xfs_mount *mp = tp->t_mountp; struct xfs_bmap_intent *bi; - trace_xfs_bmap_defer(mp, - XFS_FSB_TO_AGNO(mp, bmap->br_startblock), + trace_xfs_bmap_defer(tp->t_mountp, + XFS_FSB_TO_AGNO(tp->t_mountp, bmap->br_startblock), type, - XFS_FSB_TO_AGBNO(mp, bmap->br_startblock), + XFS_FSB_TO_AGBNO(tp->t_mountp, bmap->br_startblock), ip->i_ino, whichfork, bmap->br_startoff, bmap->br_blockcount,
fs/xfs/libxfs/xfs_refcount.c +3 -3
@@ -1183,11 +1183,11 @@ __xfs_refcount_add( xfs_fsblock_t startblock, xfs_extlen_t blockcount) { -struct xfs_mount *mp = tp->t_mountp;struct xfs_refcount_intent *ri; -trace_xfs_refcount_defer (mp,XFS_FSB_TO_AGNO (mp, startblock), - type,XFS_FSB_TO_AGBNO (mp, startblock), +trace_xfs_refcount_defer (tp->t_mountp, +XFS_FSB_TO_AGNO (tp->t_mountp, startblock), + type,XFS_FSB_TO_AGBNO (tp->t_mountp, startblock), blockcount); ri = kmem_alloc(sizeof(struct xfs_refcount_intent),
fs/xfs/libxfs/xfs_rmap.c +3 -3
@@ -2283,12 +2283,12 @@ __xfs_rmap_add(int whichfork, struct xfs_bmbt_irec *bmap) { - struct xfs_mount *mp = tp->t_mountp; struct xfs_rmap_intent *ri; - trace_xfs_rmap_defer(mp, XFS_FSB_TO_AGNO(mp, bmap->br_startblock), + trace_xfs_rmap_defer(tp->t_mountp, + XFS_FSB_TO_AGNO(tp->t_mountp, bmap->br_startblock), type, - XFS_FSB_TO_AGBNO(mp, bmap->br_startblock), + XFS_FSB_TO_AGBNO(tp->t_mountp, bmap->br_startblock), owner, whichfork, bmap->br_startoff, bmap->br_blockcount,
PATCH v2 14/15 xfs: always defer agfl block frees
In reply to Brian FosterThe AGFL fixup code conditionally defers block frees from the free list based on whether the current transaction has an associated xfs_defer_ops structure. Now that dfops is embedded in the transaction and the internal dfops is used unconditionally, this invariant is always true.
Remove the now dead logic to check for ->t_dfops in xfs_alloc_fix_freelist() and unconditionally defer AGFL block frees.
--- fs/xfs/libxfs/xfs_alloc.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-)
fs/xfs/libxfs/xfs_alloc.c +2 -9
@@ -2323,15 +2323,8 @@ xfs_alloc_fix_freelist(if (error) goto out_agbp_relse; -/* defer agfl frees if dfops is provided */ -if (tp->t_dfops) { -xfs_defer_agfl_block (tp, args->agno, bno, &targs.oinfo); - }else { - error =xfs_free_agfl_block (tp, args->agno, bno, agbp, - &targs.oinfo); -if (error) - goto out_agbp_relse; - } +/* defer agfl frees */ +xfs_defer_agfl_block (tp, args->agno, bno, &targs.oinfo); } targs.tp = tp;
Signature · 1 line
2.17.1
PATCH v2 15/15 xfs: fold dfops into the transaction
In reply to Brian Fosterstruct xfs_defer_ops has now been reduced to a single list_head. The external dfops mechanism is unused and thus everywhere a (permanent) transaction is accessible the associated dfops structure is as well.
Remove the xfs_defer_ops structure and fold the list_head into the transaction. Also remove the last remnant of external dfops in xfs_trans_dup().
--- fs/xfs/libxfs/xfs_bmap.c | 1 - fs/xfs/libxfs/xfs_btree.h | 1 - fs/xfs/libxfs/xfs_da_btree.h | 1 - fs/xfs/libxfs/xfs_defer.c | 67 ++++++++++-------------------------- fs/xfs/libxfs/xfs_defer.h | 2 -- fs/xfs/libxfs/xfs_dir2.c | 2 -- fs/xfs/libxfs/xfs_dir2.h | 1 - fs/xfs/xfs_inode.h | 1 - fs/xfs/xfs_reflink.c | 5 ++- fs/xfs/xfs_trace.h | 40 ++++++++++----------- fs/xfs/xfs_trans.c | 13 +++---- fs/xfs/xfs_trans.h | 8 ++--- 12 files changed, 46 insertions(+), 96 deletions(-)
fs/xfs/libxfs/xfs_bmap.c +0 -1
@@ -4285,7 +4285,6 @@ xfs_bmapi_write( bma.ip = ip; bma.total = total; bma.datatype =0 ; -ASSERT (!tp || tp->t_dfops);while (bno < end && n < *nmap) {bool need_alloc = false, wasdelay = false;
fs/xfs/libxfs/xfs_btree.h +0 -1
@@ -7,7 +7,6 @@#define __XFS_BTREE_H__struct xfs_buf ; -struct xfs_defer_ops ;struct xfs_inode ;struct xfs_mount ;struct xfs_trans ;
fs/xfs/libxfs/xfs_da_btree.h +0 -1
@@ -7,7 +7,6 @@#ifndef __XFS_DA_BTREE_H__#define __XFS_DA_BTREE_H__ -struct xfs_defer_ops ;struct xfs_inode ;struct xfs_trans ;struct zone ;
fs/xfs/libxfs/xfs_defer.c +19 -48
158 lines not on this page · raw message
fs/xfs/libxfs/xfs_defer.h +0 -2
@@ -7,7 +7,6 @@#define __XFS_DEFER_H__struct xfs_defer_op_type ; -struct xfs_defer_ops ;/* * Save a log intent item and a list of extents, so that we can replay
@@ -40,7 +39,6 @@ void xfs_defer_add(struct xfs_trans *tp, enum xfs_defer_ops_type type,int xfs_defer_finish_noroll (struct xfs_trans **tp);int xfs_defer_finish (struct xfs_trans **tp);void xfs_defer_cancel (struct xfs_trans *); -void xfs_defer_init (struct xfs_trans *tp,struct xfs_defer_ops *dop);void xfs_defer_move (struct xfs_trans *dtp,struct xfs_trans *stp);/* Description of a deferred type. */
fs/xfs/libxfs/xfs_dir2.c +0 -2
@@ -424,7 +424,6 @@ xfs_dir_removename(int v;/* type-checking value */ ASSERT (S_ISDIR (VFS_I (dp)->i_mode)); -ASSERT (tp->t_dfops);XFS_STATS_INC (dp->i_mount, xs_dir_remove); args =kmem_zalloc (sizeof (*args),KM_SLEEP |KM_NOFS );
@@ -483,7 +482,6 @@ xfs_dir_replace(int v;/* type-checking value */ ASSERT (S_ISDIR (VFS_I (dp)->i_mode)); -ASSERT (tp->t_dfops); rval =xfs_dir_ino_validate (tp->t_mountp, inum);if (rval)
fs/xfs/libxfs/xfs_dir2.h +0 -1
@@ -9,7 +9,6 @@#include "xfs_da_format.h" #include "xfs_da_btree.h" -struct xfs_defer_ops ;struct xfs_da_args ;struct xfs_inode ;struct xfs_mount ;
fs/xfs/xfs_inode.h +0 -1
@@ -15,7 +15,6 @@struct xfs_dinode ;struct xfs_inode ;struct xfs_buf ; -struct xfs_defer_ops ;struct xfs_bmbt_irec ;struct xfs_inode_log_item ;struct xfs_mount ;
fs/xfs/xfs_reflink.c +2 -3
@@ -502,7 +502,6 @@ xfs_reflink_cancel_cow_blocks(if (error)break ; }else if (del.br_state ==XFS_EXT_UNWRITTEN || cancel_real) { -ASSERT ((*tpp)->t_dfops);ASSERT ((*tpp)->t_firstblock ==NULLFSBLOCK );/* Free the CoW orphan record. */
@@ -678,7 +677,7 @@ xfs_reflink_end_cow( goto prev_extent;/* Unmap the old blocks in the data fork. */ -ASSERT (tp->t_dfops && tp->t_firstblock ==NULLFSBLOCK ); +ASSERT (tp->t_firstblock ==NULLFSBLOCK ); rlen = del.br_blockcount; error =__xfs_bunmapi (tp, ip, del.br_startoff, &rlen,0 ,1 );if (error)
@@ -1021,7 +1020,7 @@ xfs_reflink_remap_extent(/* Unmap the old blocks in the data fork. */ rlen = unmap_len;while (rlen) { -ASSERT (tp->t_dfops && tp->t_firstblock ==NULLFSBLOCK ); +ASSERT (tp->t_firstblock ==NULLFSBLOCK ); error =__xfs_bunmapi (tp, ip, destoff, &rlen,0 ,1 );if (error) goto out_cancel;
fs/xfs/xfs_trace.h +18 -22
@@ -2213,57 +2213,54 @@ DEFINE_BTREE_CUR_EVENT(xfs_btree_overlapped_query_range);/* deferred ops */ struct xfs_defer_pending ; -struct xfs_defer_ops ;DECLARE_EVENT_CLASS (xfs_defer_class, - TP_PROTO(struct xfs_mount *mp, struct xfs_defer_ops *dop, - unsigned long caller_ip), -TP_ARGS (mp, dop, caller_ip), + TP_PROTO(struct xfs_trans *tp, unsigned long caller_ip), +TP_ARGS (tp, caller_ip),TP_STRUCT__entry ( __field(dev_t, dev) - __field(void *, dop) + __field(struct xfs_trans *, tp) __field(char, committed) __field(unsigned long, caller_ip) ),TP_fast_assign ( - __entry->dev = mp ? mp->m_super->s_dev : 0; - __entry->dop = dop; + __entry->dev = tp->t_mountp->m_super->s_dev; + __entry->tp = tp; __entry->caller_ip = caller_ip; ), -TP_printk ("dev %d:%d ops %p caller %pS" , +TP_printk ("dev %d:%d tp %p caller %pS" ,MAJOR (__entry->dev),MINOR (__entry->dev), - __entry->dop, + __entry->tp, (char *)__entry->caller_ip) )#define DEFINE_DEFER_EVENT (name) \ DEFINE_EVENT(xfs_defer_class, name, \ - TP_PROTO(struct xfs_mount *mp, struct xfs_defer_ops *dop, \ - unsigned long caller_ip), \ - TP_ARGS(mp, dop, caller_ip)) + TP_PROTO(struct xfs_trans *tp, unsigned long caller_ip), \ + TP_ARGS(tp, caller_ip))DECLARE_EVENT_CLASS (xfs_defer_error_class, - TP_PROTO(struct xfs_mount *mp, struct xfs_defer_ops *dop, int error), -TP_ARGS (mp, dop, error), + TP_PROTO(struct xfs_trans *tp, int error), +TP_ARGS (tp, error),TP_STRUCT__entry ( __field(dev_t, dev) - __field(void *, dop) + __field(struct xfs_trans *, tp) __field(char, committed)__field (int, error) ),TP_fast_assign ( - __entry->dev = mp ? mp->m_super->s_dev : 0; - __entry->dop = dop; + __entry->dev = tp->t_mountp->m_super->s_dev; + __entry->tp = tp; __entry->error = error; ), -TP_printk ("dev %d:%d ops %p err %d" , +TP_printk ("dev %d:%d tp %p err %d" ,MAJOR (__entry->dev),MINOR (__entry->dev), - __entry->dop, + __entry->tp, __entry->error) )#define DEFINE_DEFER_ERROR_EVENT (name) \ DEFINE_EVENT(xfs_defer_error_class, name, \ - TP_PROTO(struct xfs_mount *mp, struct xfs_defer_ops *dop, int error), \ - TP_ARGS(mp, dop, error)) + TP_PROTO(struct xfs_trans *tp, int error), \ + TP_ARGS(tp, error)) DECLARE_EVENT_CLASS(xfs_defer_pending_class, TP_PROTO(struct xfs_mount *mp, struct xfs_defer_pending *dfp),
@@ -2382,7 +2379,6 @@ DEFINE_EVENT(xfs_map_extent_deferred_class, name, \ xfs_exntst_t state), \ TP_ARGS(mp, agno, op, agbno, ino, whichfork, offset, len, state)) -DEFINE_DEFER_EVENT(xfs_defer_init); DEFINE_DEFER_EVENT(xfs_defer_cancel);DEFINE_DEFER_EVENT (xfs_defer_trans_roll);DEFINE_DEFER_EVENT (xfs_defer_trans_abort);
fs/xfs/xfs_trans.c +5 -8
@@ -100,6 +100,7 @@ xfs_trans_dup( ntp->t_mountp = tp->t_mountp;INIT_LIST_HEAD (&ntp->t_items);INIT_LIST_HEAD (&ntp->t_busy); +INIT_LIST_HEAD (&ntp->t_dfops); ntp->t_firstblock =NULLFSBLOCK ;ASSERT (tp->t_flags &XFS_TRANS_PERM_LOG_RES );
@@ -120,12 +121,8 @@ xfs_trans_dup( tp->t_rtx_res = tp->t_rtx_res_used; ntp->t_pflags = tp->t_pflags; -/* copy the dfops pointer if it's external, otherwise move it */ -xfs_defer_init (ntp, &ntp->t_dfops_internal); -if (tp->t_dfops != &tp->t_dfops_internal) - ntp->t_dfops = tp->t_dfops; -else -xfs_defer_move (ntp, tp); +/* move deferred ops over to the new tp */ +xfs_defer_move (ntp, tp);xfs_trans_dup_dqinfo (tp, ntp);
@@ -280,8 +277,8 @@ xfs_trans_alloc( tp->t_mountp = mp;INIT_LIST_HEAD (&tp->t_items);INIT_LIST_HEAD (&tp->t_busy); +INIT_LIST_HEAD (&tp->t_dfops); tp->t_firstblock =NULLFSBLOCK ; -xfs_defer_init (tp, &tp->t_dfops_internal); error =xfs_trans_reserve (tp, resp, blocks, rtextents);if (error) {
@@ -929,7 +926,7 @@ __xfs_trans_commit(* Finish deferred items on final commit. Only permanent transactions * should ever have deferred ops. */ -WARN_ON_ONCE (!list_empty (&tp->t_dfops->dop_intake) && +WARN_ON_ONCE (!list_empty (&tp->t_dfops) && !(tp->t_flags &XFS_TRANS_PERM_LOG_RES ));if (!regrant && (tp->t_flags &XFS_TRANS_PERM_LOG_RES )) { error =xfs_defer_finish_noroll (&tp);
fs/xfs/xfs_trans.h +2 -6
@@ -90,13 +90,10 @@ void xfs_log_item_init(struct xfs_mount *mp, struct xfs_log_item *item,#define XFS_ITEM_FLUSHING 3/* -* Deferred operations tracking structure. +* Deferred operation item relogging limits. */ #define XFS_DEFER_OPS_NR_INODES 2/* join up to two inodes */ #define XFS_DEFER_OPS_NR_BUFS 2/* join up to two buffers */ -struct xfs_defer_ops { -struct list_head dop_intake;/* unlogged pending work */ -};/* * This is the structure maintained for every active transaction.
@@ -114,7 +111,6 @@ typedef struct xfs_trans {struct xlog_ticket *t_ticket;/* log mgr ticket */ struct xfs_mount *t_mountp;/* ptr to fs mount struct */ struct xfs_dquot_acct *t_dqinfo;/* acctg info for dquots */ -struct xfs_defer_ops *t_dfops;/* dfops reference */ int64_t t_icount_delta;/* superblock icount change */ int64_t t_ifree_delta;/* superblock ifree change */ int64_t t_fdblocks_delta;/* superblock fdblocks chg */
@@ -136,8 +132,8 @@ typedef struct xfs_trans {int64_t t_rextslog_delta;/* superblocks rextslog chg */ struct list_head t_items;/* log item descriptors */ struct list_head t_busy;/* list of busy extents */ +struct list_head t_dfops;/* deferred operations */ unsigned long t_pflags;/* saved process flags state */ -struct xfs_defer_ops t_dfops_internal; } xfs_trans_t;/*
Signature · 1 line
2.17.1
PATCH v3 15/15 xfs: fold dfops into the transaction
struct xfs_defer_ops has now been reduced to a single list_head. The external dfops mechanism is unused and thus everywhere a (permanent) transaction is accessible the associated dfops structure is as well.
Remove the xfs_defer_ops structure and fold the list_head into the transaction. Also remove the last remnant of external dfops in xfs_trans_dup().
---
v3:
- Resolve minor conflicts with v3 of patch 12/15.
fs/xfs/libxfs/xfs_bmap.c | 1 - fs/xfs/libxfs/xfs_btree.h | 1 - fs/xfs/libxfs/xfs_da_btree.h | 1 - fs/xfs/libxfs/xfs_defer.c | 67 ++++++++++-------------------------- fs/xfs/libxfs/xfs_defer.h | 2 -- fs/xfs/libxfs/xfs_dir2.c | 2 -- fs/xfs/libxfs/xfs_dir2.h | 1 - fs/xfs/xfs_inode.h | 1 - fs/xfs/xfs_reflink.c | 5 ++- fs/xfs/xfs_trace.h | 40 ++++++++++----------- fs/xfs/xfs_trans.c | 13 +++---- fs/xfs/xfs_trans.h | 8 ++--- 12 files changed, 46 insertions(+), 96 deletions(-)
fs/xfs/libxfs/xfs_bmap.c +0 -1
@@ -4285,7 +4285,6 @@ xfs_bmapi_write( bma.ip = ip; bma.total = total; bma.datatype =0 ; -ASSERT (!tp || tp->t_dfops);while (bno < end && n < *nmap) {bool need_alloc = false, wasdelay = false;
fs/xfs/libxfs/xfs_btree.h +0 -1
@@ -7,7 +7,6 @@#define __XFS_BTREE_H__struct xfs_buf ; -struct xfs_defer_ops ;struct xfs_inode ;struct xfs_mount ;struct xfs_trans ;
fs/xfs/libxfs/xfs_da_btree.h +0 -1
@@ -7,7 +7,6 @@#ifndef __XFS_DA_BTREE_H__#define __XFS_DA_BTREE_H__ -struct xfs_defer_ops ;struct xfs_inode ;struct xfs_trans ;struct zone ;
fs/xfs/libxfs/xfs_defer.c +19 -48
159 lines not on this page · raw message
fs/xfs/libxfs/xfs_defer.h +0 -2
@@ -7,7 +7,6 @@#define __XFS_DEFER_H__struct xfs_defer_op_type ; -struct xfs_defer_ops ;/* * Save a log intent item and a list of extents, so that we can replay
@@ -40,7 +39,6 @@ void xfs_defer_add(struct xfs_trans *tp, enum xfs_defer_ops_type type,int xfs_defer_finish_noroll (struct xfs_trans **tp);int xfs_defer_finish (struct xfs_trans **tp);void xfs_defer_cancel (struct xfs_trans *); -void xfs_defer_init (struct xfs_trans *tp,struct xfs_defer_ops *dop);void xfs_defer_move (struct xfs_trans *dtp,struct xfs_trans *stp);/* Description of a deferred type. */
fs/xfs/libxfs/xfs_dir2.c +0 -2
@@ -424,7 +424,6 @@ xfs_dir_removename(int v;/* type-checking value */ ASSERT (S_ISDIR (VFS_I (dp)->i_mode)); -ASSERT (tp->t_dfops);XFS_STATS_INC (dp->i_mount, xs_dir_remove); args =kmem_zalloc (sizeof (*args),KM_SLEEP |KM_NOFS );
@@ -483,7 +482,6 @@ xfs_dir_replace(int v;/* type-checking value */ ASSERT (S_ISDIR (VFS_I (dp)->i_mode)); -ASSERT (tp->t_dfops); rval =xfs_dir_ino_validate (tp->t_mountp, inum);if (rval)
fs/xfs/libxfs/xfs_dir2.h +0 -1
@@ -9,7 +9,6 @@#include "xfs_da_format.h" #include "xfs_da_btree.h" -struct xfs_defer_ops ;struct xfs_da_args ;struct xfs_inode ;struct xfs_mount ;
fs/xfs/xfs_inode.h +0 -1
@@ -15,7 +15,6 @@struct xfs_dinode ;struct xfs_inode ;struct xfs_buf ; -struct xfs_defer_ops ;struct xfs_bmbt_irec ;struct xfs_inode_log_item ;struct xfs_mount ;
fs/xfs/xfs_reflink.c +2 -3
@@ -502,7 +502,6 @@ xfs_reflink_cancel_cow_blocks(if (error)break ; }else if (del.br_state ==XFS_EXT_UNWRITTEN || cancel_real) { -ASSERT ((*tpp)->t_dfops);ASSERT ((*tpp)->t_firstblock ==NULLFSBLOCK );/* Free the CoW orphan record. */
@@ -678,7 +677,7 @@ xfs_reflink_end_cow( goto prev_extent;/* Unmap the old blocks in the data fork. */ -ASSERT (tp->t_dfops && tp->t_firstblock ==NULLFSBLOCK ); +ASSERT (tp->t_firstblock ==NULLFSBLOCK ); rlen = del.br_blockcount; error =__xfs_bunmapi (tp, ip, del.br_startoff, &rlen,0 ,1 );if (error)
@@ -1021,7 +1020,7 @@ xfs_reflink_remap_extent(/* Unmap the old blocks in the data fork. */ rlen = unmap_len;while (rlen) { -ASSERT (tp->t_dfops && tp->t_firstblock ==NULLFSBLOCK ); +ASSERT (tp->t_firstblock ==NULLFSBLOCK ); error =__xfs_bunmapi (tp, ip, destoff, &rlen,0 ,1 );if (error) goto out_cancel;
fs/xfs/xfs_trace.h +18 -22
@@ -2213,57 +2213,54 @@ DEFINE_BTREE_CUR_EVENT(xfs_btree_overlapped_query_range);/* deferred ops */ struct xfs_defer_pending ; -struct xfs_defer_ops ;DECLARE_EVENT_CLASS (xfs_defer_class, - TP_PROTO(struct xfs_mount *mp, struct xfs_defer_ops *dop, - unsigned long caller_ip), -TP_ARGS (mp, dop, caller_ip), + TP_PROTO(struct xfs_trans *tp, unsigned long caller_ip), +TP_ARGS (tp, caller_ip),TP_STRUCT__entry ( __field(dev_t, dev) - __field(void *, dop) + __field(struct xfs_trans *, tp) __field(char, committed) __field(unsigned long, caller_ip) ),TP_fast_assign ( - __entry->dev = mp ? mp->m_super->s_dev : 0; - __entry->dop = dop; + __entry->dev = tp->t_mountp->m_super->s_dev; + __entry->tp = tp; __entry->caller_ip = caller_ip; ), -TP_printk ("dev %d:%d ops %p caller %pS" , +TP_printk ("dev %d:%d tp %p caller %pS" ,MAJOR (__entry->dev),MINOR (__entry->dev), - __entry->dop, + __entry->tp, (char *)__entry->caller_ip) )#define DEFINE_DEFER_EVENT (name) \ DEFINE_EVENT(xfs_defer_class, name, \ - TP_PROTO(struct xfs_mount *mp, struct xfs_defer_ops *dop, \ - unsigned long caller_ip), \ - TP_ARGS(mp, dop, caller_ip)) + TP_PROTO(struct xfs_trans *tp, unsigned long caller_ip), \ + TP_ARGS(tp, caller_ip))DECLARE_EVENT_CLASS (xfs_defer_error_class, - TP_PROTO(struct xfs_mount *mp, struct xfs_defer_ops *dop, int error), -TP_ARGS (mp, dop, error), + TP_PROTO(struct xfs_trans *tp, int error), +TP_ARGS (tp, error),TP_STRUCT__entry ( __field(dev_t, dev) - __field(void *, dop) + __field(struct xfs_trans *, tp) __field(char, committed)__field (int, error) ),TP_fast_assign ( - __entry->dev = mp ? mp->m_super->s_dev : 0; - __entry->dop = dop; + __entry->dev = tp->t_mountp->m_super->s_dev; + __entry->tp = tp; __entry->error = error; ), -TP_printk ("dev %d:%d ops %p err %d" , +TP_printk ("dev %d:%d tp %p err %d" ,MAJOR (__entry->dev),MINOR (__entry->dev), - __entry->dop, + __entry->tp, __entry->error) )#define DEFINE_DEFER_ERROR_EVENT (name) \ DEFINE_EVENT(xfs_defer_error_class, name, \ - TP_PROTO(struct xfs_mount *mp, struct xfs_defer_ops *dop, int error), \ - TP_ARGS(mp, dop, error)) + TP_PROTO(struct xfs_trans *tp, int error), \ + TP_ARGS(tp, error)) DECLARE_EVENT_CLASS(xfs_defer_pending_class, TP_PROTO(struct xfs_mount *mp, struct xfs_defer_pending *dfp),
@@ -2382,7 +2379,6 @@ DEFINE_EVENT(xfs_map_extent_deferred_class, name, \ xfs_exntst_t state), \ TP_ARGS(mp, agno, op, agbno, ino, whichfork, offset, len, state)) -DEFINE_DEFER_EVENT(xfs_defer_init); DEFINE_DEFER_EVENT(xfs_defer_cancel);DEFINE_DEFER_EVENT (xfs_defer_trans_roll);DEFINE_DEFER_EVENT (xfs_defer_trans_abort);
fs/xfs/xfs_trans.c +5 -8
@@ -100,6 +100,7 @@ xfs_trans_dup( ntp->t_mountp = tp->t_mountp;INIT_LIST_HEAD (&ntp->t_items);INIT_LIST_HEAD (&ntp->t_busy); +INIT_LIST_HEAD (&ntp->t_dfops); ntp->t_firstblock =NULLFSBLOCK ;ASSERT (tp->t_flags &XFS_TRANS_PERM_LOG_RES );
@@ -120,12 +121,8 @@ xfs_trans_dup( tp->t_rtx_res = tp->t_rtx_res_used; ntp->t_pflags = tp->t_pflags; -/* copy the dfops pointer if it's external, otherwise move it */ -xfs_defer_init (ntp, &ntp->t_dfops_internal); -if (tp->t_dfops != &tp->t_dfops_internal) - ntp->t_dfops = tp->t_dfops; -else -xfs_defer_move (ntp, tp); +/* move deferred ops over to the new tp */ +xfs_defer_move (ntp, tp);xfs_trans_dup_dqinfo (tp, ntp);
@@ -280,8 +277,8 @@ xfs_trans_alloc( tp->t_mountp = mp;INIT_LIST_HEAD (&tp->t_items);INIT_LIST_HEAD (&tp->t_busy); +INIT_LIST_HEAD (&tp->t_dfops); tp->t_firstblock =NULLFSBLOCK ; -xfs_defer_init (tp, &tp->t_dfops_internal); error =xfs_trans_reserve (tp, resp, blocks, rtextents);if (error) {
@@ -929,7 +926,7 @@ __xfs_trans_commit(* Finish deferred items on final commit. Only permanent transactions * should ever have deferred ops. */ -WARN_ON_ONCE (!list_empty (&tp->t_dfops->dop_intake) && +WARN_ON_ONCE (!list_empty (&tp->t_dfops) && !(tp->t_flags &XFS_TRANS_PERM_LOG_RES ));if (!regrant && (tp->t_flags &XFS_TRANS_PERM_LOG_RES )) { error =xfs_defer_finish_noroll (&tp);
fs/xfs/xfs_trans.h +2 -6
@@ -90,13 +90,10 @@ void xfs_log_item_init(struct xfs_mount *mp, struct xfs_log_item *item,#define XFS_ITEM_FLUSHING 3/* -* Deferred operations tracking structure. +* Deferred operation item relogging limits. */ #define XFS_DEFER_OPS_NR_INODES 2/* join up to two inodes */ #define XFS_DEFER_OPS_NR_BUFS 2/* join up to two buffers */ -struct xfs_defer_ops { -struct list_head dop_intake;/* unlogged pending work */ -};/* * This is the structure maintained for every active transaction.
@@ -114,7 +111,6 @@ typedef struct xfs_trans {struct xlog_ticket *t_ticket;/* log mgr ticket */ struct xfs_mount *t_mountp;/* ptr to fs mount struct */ struct xfs_dquot_acct *t_dqinfo;/* acctg info for dquots */ -struct xfs_defer_ops *t_dfops;/* dfops reference */ int64_t t_icount_delta;/* superblock icount change */ int64_t t_ifree_delta;/* superblock ifree change */ int64_t t_fdblocks_delta;/* superblock fdblocks chg */
@@ -136,8 +132,8 @@ typedef struct xfs_trans {int64_t t_rextslog_delta;/* superblocks rextslog chg */ struct list_head t_items;/* log item descriptors */ struct list_head t_busy;/* list of busy extents */ +struct list_head t_dfops;/* deferred operations */ unsigned long t_pflags;/* saved process flags state */ -struct xfs_defer_ops t_dfops_internal; } xfs_trans_t;/*
Signature · 1 line
2.17.1