xfs: broad enablement of deferred agfl frees

A couple COW fork unwritten extent conversion helpers pass an uninitialized dfops pointer to xfs_bmapi_write(). This does not cause problems because conversion does not use a transaction or the dfops structure for the COW fork. Drop the uninitialized usage of dfops in these codepaths and pass NULL along to xfs_bmapi_write() instead.

Quoted 6 lines · Brian Foster

A couple COW fork unwritten extent conversion helpers pass an uninitialized dfops pointer to xfs_bmapi_write(). This does not cause problems because conversion does not use a transaction or the dfops structure for the COW fork. Drop the uninitialized usage of dfops in these codepaths and pass NULL along to xfs_bmapi_write() instead.

Looks good.

Is this something we should maybe queue up for 4.18?

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

Quoted 8 lines · Christoph Hellwig

Quoted 6 lines · Brian Foster

A couple COW fork unwritten extent conversion helpers pass an uninitialized dfops pointer to xfs_bmapi_write(). This does not cause problems because conversion does not use a transaction or the dfops structure for the COW fork. Drop the uninitialized usage of dfops in these codepaths and pass NULL along to xfs_bmapi_write() instead.

Quoted 4 lines

Looks good.

Is this something we should maybe queue up for 4.18?

That might make sense because of all the refactoring, but otherwise I don't have a strong opinion. Let's see what Darrick wants to do...

Brian

Quoted 5 lines

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

--
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

Quoted 14 lines · Brian Foster

Quoted 12 lines · Christoph Hellwig

Quoted 6 lines · Brian Foster

A couple COW fork unwritten extent conversion helpers pass an uninitialized dfops pointer to xfs_bmapi_write(). This does not cause problems because conversion does not use a transaction or the dfops structure for the COW fork. Drop the uninitialized usage of dfops in these codepaths and pass NULL along to xfs_bmapi_write() instead.

Looks good.

Is this something we should maybe queue up for 4.18?

Quoted 2 lines

That might make sense because of all the refactoring, but otherwise I don't have a strong opinion. Let's see what Darrick wants to do...

AFAICT this only eliminates the passing around of an unus{ed,able} dfops parameter, right? We're not fixing a regression or some other breakage, just eliminating cpu cycle waste, so I think this can soak (along with everything else) until 4.19.

--D

Quoted 11 lines

Brian

Quoted 5 lines

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

--
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

--
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

Quoted 15 lines · Darrick J. Wong

Quoted 14 lines · Brian Foster

Quoted 12 lines · Christoph Hellwig

Quoted 6 lines · Brian Foster

A couple COW fork unwritten extent conversion helpers pass an uninitialized dfops pointer to xfs_bmapi_write(). This does not cause problems because conversion does not use a transaction or the dfops structure for the COW fork. Drop the uninitialized usage of dfops in these codepaths and pass NULL along to xfs_bmapi_write() instead.

Looks good.

Is this something we should maybe queue up for 4.18?

Quoted 8 lines

Quoted 2 lines

That might make sense because of all the refactoring, but otherwise I don't have a strong opinion. Let's see what Darrick wants to do...

AFAICT this only eliminates the passing around of an unus{ed,able} dfops parameter, right? We're not fixing a regression or some other breakage, just eliminating cpu cycle waste, so I think this can soak (along with everything else) until 4.19.

Works for me. This does have the side effect of enabling the deferred AGFL block free behavior wherever dfops is used, but that is a not a critical change/fix. The problem that inspired the behavior in the first place was resolved by the more targeted changes in the first series. The goals of this (and the series or two to follow) are primarily follow up refactoring and to provide more consistent behavior fs-wide.

Brian

Quoted 17 lines

--D

Quoted 11 lines

Brian

Quoted 5 lines

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

--
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

--
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

Quoted 25 lines · Brian Foster

Quoted 15 lines · Darrick J. Wong

Quoted 14 lines · Brian Foster

Quoted 12 lines · Christoph Hellwig

Quoted 6 lines · Brian Foster

A couple COW fork unwritten extent conversion helpers pass an uninitialized dfops pointer to xfs_bmapi_write(). This does not cause problems because conversion does not use a transaction or the dfops structure for the COW fork. Drop the uninitialized usage of dfops in these codepaths and pass NULL along to xfs_bmapi_write() instead.

Looks good.

Is this something we should maybe queue up for 4.18?

Quoted 8 lines

Quoted 2 lines

That might make sense because of all the refactoring, but otherwise I don't have a strong opinion. Let's see what Darrick wants to do...

AFAICT this only eliminates the passing around of an unus{ed,able} dfops parameter, right? We're not fixing a regression or some other breakage, just eliminating cpu cycle waste, so I think this can soak (along with everything else) until 4.19.

Quoted 6 lines

Works for me. This does have the side effect of enabling the deferred AGFL block free behavior wherever dfops is used, but that is a not a critical change/fix. The problem that inspired the behavior in the first place was resolved by the more targeted changes in the first series. The goals of this (and the series or two to follow) are primarily follow up refactoring and to provide more consistent behavior fs-wide.

Follow-up question, then: are there situations where we defered agfl freeing is /not/ desirable? I couldn't think of any, but my head (and shop vac) are full of sawdust. :)

--D

Quoted 19 lines

Brian

Quoted 17 lines

--D

Quoted 11 lines

Brian

Quoted 5 lines

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

--
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

--
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

Quoted 33 lines · Darrick J. Wong

Quoted 25 lines · Brian Foster

Quoted 15 lines · Darrick J. Wong

Quoted 14 lines · Brian Foster

Quoted 12 lines · Christoph Hellwig

Quoted 6 lines · Brian Foster

A couple COW fork unwritten extent conversion helpers pass an uninitialized dfops pointer to xfs_bmapi_write(). This does not cause problems because conversion does not use a transaction or the dfops structure for the COW fork. Drop the uninitialized usage of dfops in these codepaths and pass NULL along to xfs_bmapi_write() instead.

Looks good.

Is this something we should maybe queue up for 4.18?

Quoted 8 lines

Quoted 2 lines

That might make sense because of all the refactoring, but otherwise I don't have a strong opinion. Let's see what Darrick wants to do...

AFAICT this only eliminates the passing around of an unus{ed,able} dfops parameter, right? We're not fixing a regression or some other breakage, just eliminating cpu cycle waste, so I think this can soak (along with everything else) until 4.19.

Quoted 6 lines

Works for me. This does have the side effect of enabling the deferred AGFL block free behavior wherever dfops is used, but that is a not a critical change/fix. The problem that inspired the behavior in the first place was resolved by the more targeted changes in the first series. The goals of this (and the series or two to follow) are primarily follow up refactoring and to provide more consistent behavior fs-wide.

Quoted 4 lines

Follow-up question, then: are there situations where we defered agfl freeing is /not/ desirable? I couldn't think of any, but my head (and shop vac) are full of sawdust. :)

I don't think so. My presumption since the original series is that this is essentially an analogus situation to freeing file blocks. We defer it unconditionally to provide some semblence of consistent/predictable transaction reservation consumption. I haven't audited to the point of seeing if we need to add dfops in certain paths purely for deferred AGFL frees (i.e., adding dfops to paths that don't already have a dfops), but the bit I mentioned in my last mail would effectively cover that last case by embedding ->t_dfops in the transaction. With that, any transaction is capable of deferring operations to be finished automatically at commit time (and the boilerplate trans_alloc() -> defer_init() -> defer_finish() -> trans_commit() sequence can be factored away). One obvious tradeoff is that the transaction becomes bigger, but I'm not sure how much that matters given that we have a dfops in most cases anyways and we're using it more progressively.

BTW, I don't think any of this precludes us from not deferring AGFL frees in a particular case if that becomes necessary for some reason in the future. We'd just need to decide out how to communicate that to the allocator (alloc_arg flag, tx flag, etc.).

Brian

Quoted 21 lines

--D

Quoted 19 lines

Brian

Quoted 17 lines

--D

Quoted 11 lines

Brian

Quoted 5 lines

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

--
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

--
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

--
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

The ->t_agfl_dfops field is currently used to defer agfl block frees from associated transaction contexts. While all known problematic contexts have already been updated to use ->t_agfl_dfops, the broader goal is defer agfl frees from all callers that already use a deferred operations structure. Further, the transaction field facilitates a good amount of code clean up where the transaction and dfops have historically been passed down through the stack separately.

Rename the field to something more generic to prepare to use it as such throughout XFS. This patch does not change behavior.

Signed-off-by Brian Foster <[email protected]>

The ->t_agfl_dfops field is currently used to defer agfl block frees from associated transaction contexts. While all known problematic contexts have already been updated to use ->t_agfl_dfops, the broader goal is defer agfl frees from all callers that already use a deferred operations structure. Further, the transaction field facilitates a good amount of code clean up where the transaction and dfops have historically been passed down through the stack separately.

Rename the field to something more generic to prepare to use it as such throughout XFS. This patch does not change behavior.

Signed-off-by Brian Foster <[email protected]>

---
 fs/xfs/libxfs/xfs_alloc.c |  4 ++--
 fs/xfs/libxfs/xfs_defer.c |  8 ++++----
 fs/xfs/xfs_inode.c        | 10 +++++-----
 fs/xfs/xfs_symlink.c      |  2 +-
 fs/xfs/xfs_trans.c        |  6 +++---
 fs/xfs/xfs_trans.h        |  2 +-
 6 files changed, 16 insertions(+), 16 deletions(-)

fs/xfs/libxfs/xfs_alloc.c +2 -2

index eef466260d43..5b1607d76fe9 100644
@@ -2323,8 +2323,8 @@ xfs_alloc_fix_freelist(
 			goto out_agbp_relse;
 
 		/* defer agfl frees if dfops is provided */
-		if (tp->t_agfl_dfops) {
-			xfs_defer_agfl_block(mp, tp->t_agfl_dfops, args->agno,
+		if (tp->t_dfops) {
+			xfs_defer_agfl_block(mp, tp->t_dfops, args->agno,
 					     bno, &targs.oinfo);
 		} else {
 			error = xfs_free_agfl_block(tp, args->agno, bno, agbp,

fs/xfs/libxfs/xfs_defer.c +4 -4

index c3e5bffda4f5..560a7d178c1e 100644
@@ -350,9 +350,9 @@ xfs_defer_finish(
 	 * Note that this code can go away once all dfops users attach to the
 	 * associated tp.
 	 */
-	ASSERT(!(*tp)->t_agfl_dfops || ((*tp)->t_agfl_dfops == dop));
-	orig_dop = (*tp)->t_agfl_dfops;
-	(*tp)->t_agfl_dfops = dop;
+	ASSERT(!(*tp)->t_dfops || ((*tp)->t_dfops == dop));
+	orig_dop = (*tp)->t_dfops;
+	(*tp)->t_dfops = dop;
 
 	/* Until we run out of pending work to finish... */
 	while (xfs_defer_has_unfinished_work(dop)) {

@@ -425,7 +425,7 @@ xfs_defer_finish(
 	}
 
 out:
-	(*tp)->t_agfl_dfops = orig_dop;
+	(*tp)->t_dfops = orig_dop;
 	if (error)
 		trace_xfs_defer_finish_error((*tp)->t_mountp, dop, error);
 	else

fs/xfs/xfs_inode.c +5 -5

index 5df4de666cc1..9c5fbc94cf14 100644
@@ -1196,7 +1196,7 @@ xfs_create(
 	unlock_dp_on_error = true;
 
 	xfs_defer_init(&dfops, &first_block);
-	tp->t_agfl_dfops = &dfops;
+	tp->t_dfops = &dfops;
 
 	/*
 	 * Reserve disk quota and the inode.

@@ -1452,7 +1452,7 @@ xfs_link(
 	}
 
 	xfs_defer_init(&dfops, &first_block);
-	tp->t_agfl_dfops = &dfops;
+	tp->t_dfops = &dfops;
 
 	/*
 	 * Handle initial link state of O_TMPFILE inode

@@ -1813,7 +1813,7 @@ xfs_inactive_ifree(
 	xfs_trans_ijoin(tp, ip, 0);
 
 	xfs_defer_init(&dfops, &first_block);
-	tp->t_agfl_dfops = &dfops;
+	tp->t_dfops = &dfops;
 	error = xfs_ifree(tp, ip, &dfops);
 	if (error) {
 		/*

@@ -2659,7 +2659,7 @@ xfs_remove(
 		goto out_trans_cancel;
 
 	xfs_defer_init(&dfops, &first_block);
-	tp->t_agfl_dfops = &dfops;
+	tp->t_dfops = &dfops;
 	error = xfs_dir_removename(tp, dp, name, ip->i_ino,
 					&first_block, &dfops, resblks);
 	if (error) {

@@ -3027,7 +3027,7 @@ xfs_rename(
 	}
 
 	xfs_defer_init(&dfops, &first_block);
-	tp->t_agfl_dfops = &dfops;
+	tp->t_dfops = &dfops;
 
 	/* RENAME_EXCHANGE is unique from here on. */
 	if (flags & RENAME_EXCHANGE)

fs/xfs/xfs_symlink.c +1 -1

index 3783afcb68d2..44335bdebea2 100644
@@ -247,7 +247,7 @@ xfs_symlink(
 	 * bmapi or the directory create code.
 	 */
 	xfs_defer_init(&dfops, &first_block);
-	tp->t_agfl_dfops = &dfops;
+	tp->t_dfops = &dfops;
 
 	/*
 	 * Allocate an inode for the symlink.

fs/xfs/xfs_trans.c +3 -3

index 524f543c5b82..630993387517 100644
@@ -118,7 +118,7 @@ xfs_trans_dup(
 	ntp->t_rtx_res = tp->t_rtx_res - tp->t_rtx_res_used;
 	tp->t_rtx_res = tp->t_rtx_res_used;
 	ntp->t_pflags = tp->t_pflags;
-	ntp->t_agfl_dfops = tp->t_agfl_dfops;
+	ntp->t_dfops = tp->t_dfops;
 
 	xfs_trans_dup_dqinfo(tp, ntp);
 

@@ -914,8 +914,8 @@ __xfs_trans_commit(
 	int			error = 0;
 	int			sync = tp->t_flags & XFS_TRANS_SYNC;
 
-	ASSERT(!tp->t_agfl_dfops ||
-	       !xfs_defer_has_unfinished_work(tp->t_agfl_dfops) || regrant);
+	ASSERT(!tp->t_dfops ||
+	       !xfs_defer_has_unfinished_work(tp->t_dfops) || regrant);
 
 	trace_xfs_trans_commit(tp, _RET_IP_);
 

fs/xfs/xfs_trans.h +1 -1

index 6526314f0b8f..d8a695c57103 100644
@@ -105,7 +105,7 @@ 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_agfl_dfops;	/* optional agfl fixup dfops */
+	struct xfs_defer_ops	*t_dfops;	/* dfops reference */
 	unsigned int		t_flags;	/* misc flags */
 	int64_t			t_icount_delta;	/* superblock icount change */
 	int64_t			t_ifree_delta;	/* superblock ifree change */

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

The inode free callchain starting in xfs_inactive_ifree() already associates its dfops with the transaction. It still passes the dfops on the stack down through xfs_difree_inobt(), however.

Clean up the call stack and reference dfops directly from the transaction. This patch does not change behavior.

Signed-off-by Brian Foster <[email protected]>

The inode free callchain starting in xfs_inactive_ifree() already associates its dfops with the transaction. It still passes the dfops on the stack down through xfs_difree_inobt(), however.

Clean up the call stack and reference dfops directly from the transaction. This patch does not change behavior.

Signed-off-by Brian Foster <[email protected]>

---
 fs/xfs/libxfs/xfs_ialloc.c | 6 ++----
 fs/xfs/libxfs/xfs_ialloc.h | 1 -
 fs/xfs/xfs_inode.c         | 9 ++++-----
 fs/xfs/xfs_inode.h         | 3 +--
 4 files changed, 7 insertions(+), 12 deletions(-)

fs/xfs/libxfs/xfs_ialloc.c +2 -4

index 0d968e8143aa..c38d14106b53 100644
@@ -1915,7 +1915,6 @@ xfs_difree_inobt(
 	struct xfs_trans		*tp,
 	struct xfs_buf			*agbp,
 	xfs_agino_t			agino,
-	struct xfs_defer_ops		*dfops,
 	struct xfs_icluster		*xic,
 	struct xfs_inobt_rec_incore	*orec)
 {

@@ -2003,7 +2002,7 @@ xfs_difree_inobt(
 			goto error0;
 		}
 
-		xfs_difree_inode_chunk(mp, agno, &rec, dfops);
+		xfs_difree_inode_chunk(mp, agno, &rec, tp->t_dfops);
 	} else {
 		xic->deleted = false;
 

@@ -2148,7 +2147,6 @@ int
 xfs_difree(
 	struct xfs_trans	*tp,		/* transaction pointer */
 	xfs_ino_t		inode,		/* inode to be freed */
-	struct xfs_defer_ops	*dfops,		/* extents to free */
 	struct xfs_icluster	*xic)	/* cluster info if deleted */
 {
 	/* REFERENCED */

@@ -2200,7 +2198,7 @@ xfs_difree(
 	/*
 	 * Fix up the inode allocation btree.
 	 */
-	error = xfs_difree_inobt(mp, tp, agbp, agino, dfops, xic, &rec);
+	error = xfs_difree_inobt(mp, tp, agbp, agino, xic, &rec);
 	if (error)
 		goto error0;
 

fs/xfs/libxfs/xfs_ialloc.h +0 -1

index 90b09c5f163b..e936b7cc9389 100644
@@ -82,7 +82,6 @@ int					/* error */
 xfs_difree(
 	struct xfs_trans *tp,		/* transaction pointer */
 	xfs_ino_t	inode,		/* inode to be freed */
-	struct xfs_defer_ops *dfops,	/* extents to free */
 	struct xfs_icluster *ifree);	/* cluster info if deleted */
 
 /*

fs/xfs/xfs_inode.c +4 -5

index 9c5fbc94cf14..f5649a023d46 100644
@@ -1814,7 +1814,7 @@ xfs_inactive_ifree(
 
 	xfs_defer_init(&dfops, &first_block);
 	tp->t_dfops = &dfops;
-	error = xfs_ifree(tp, ip, &dfops);
+	error = xfs_ifree(tp, ip);
 	if (error) {
 		/*
 		 * If we fail to free the inode, shut down.  The cancel

@@ -2445,9 +2445,8 @@ xfs_ifree_local_data(
  */
 int
 xfs_ifree(
-	xfs_trans_t	*tp,
-	xfs_inode_t	*ip,
-	struct xfs_defer_ops	*dfops)
+	struct xfs_trans	*tp,
+	struct xfs_inode	*ip)
 {
 	int			error;
 	struct xfs_icluster	xic = { 0 };

@@ -2466,7 +2465,7 @@ xfs_ifree(
 	if (error)
 		return error;
 
-	error = xfs_difree(tp, ip->i_ino, dfops, &xic);
+	error = xfs_difree(tp, ip->i_ino, &xic);
 	if (error)
 		return error;
 

fs/xfs/xfs_inode.h +1 -2

index 2ed63a49e890..b1f0e8394f3b 100644
@@ -415,8 +415,7 @@ uint		xfs_ilock_data_map_shared(struct xfs_inode *);
 uint		xfs_ilock_attr_map_shared(struct xfs_inode *);
 
 uint		xfs_ip2xflags(struct xfs_inode *);
-int		xfs_ifree(struct xfs_trans *, xfs_inode_t *,
-			   struct xfs_defer_ops *);
+int		xfs_ifree(struct xfs_trans *, struct xfs_inode *);
 int		xfs_itruncate_extents_flags(struct xfs_trans **,
 				struct xfs_inode *, int, xfs_fsize_t, int);
 void		xfs_iext_realloc(xfs_inode_t *, int, int);

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

fs/xfs/libxfs/xfs_dir2.c xfs_dir_createname( · line 275

Quoted 5 lines

fs/xfs/libxfs/xfs_dir2.c +1 -1

-	args->dfops = dfops;
 	args->total = total;
 	args->whichfork = XFS_DATA_FORK;
 	args->trans = tp;
+	ASSERT(tp->t_dfops || !first);

Nitpick: I like to have calling convention asserts always at the beginning of the function to clearly see the assumed state on the first look.

Otherwise looks fine:

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

All callers of the directory create, rename and remove interfaces already associate the dfops with the transaction. Drop the dfops parameters in these calls in preparation for further cleanups in the layers below. This patch does not change behavior.

Signed-off-by Brian Foster <[email protected]>

Reviewed-by Christoph Hellwig <[email protected]>

---

v2:
- Move asserts to function start.

 fs/xfs/libxfs/xfs_dir2.c | 17 +++++++-------
 fs/xfs/libxfs/xfs_dir2.h |  9 +++-----
 fs/xfs/xfs_inode.c       | 50 ++++++++++++++++++----------------------
 fs/xfs/xfs_symlink.c     |  2 +-
 4 files changed, 36 insertions(+), 42 deletions(-)

fs/xfs/libxfs/xfs_dir2.c +9 -8

index 59169aff30fe..c98250f0de50 100644
@@ -244,7 +244,6 @@ xfs_dir_createname(
 	struct xfs_name		*name,
 	xfs_ino_t		inum,		/* new entry inode number */
 	xfs_fsblock_t		*first,		/* bmap's firstblock */
-	struct xfs_defer_ops	*dfops,		/* bmap's freeblock list */
 	xfs_extlen_t		total)		/* bmap's total block count */
 {
 	struct xfs_da_args	*args;

@@ -252,6 +251,8 @@ xfs_dir_createname(
 	int			v;		/* type-checking value */
 
 	ASSERT(S_ISDIR(VFS_I(dp)->i_mode));
+	ASSERT(tp->t_dfops || !first);
+
 	if (inum) {
 		rval = xfs_dir_ino_validate(tp->t_mountp, inum);
 		if (rval)

@@ -270,11 +271,11 @@ xfs_dir_createname(
 	args->hashval = dp->i_mount->m_dirnameops->hashname(name);
 	args->inumber = inum;
 	args->dp = dp;
-	args->firstblock = first;
-	args->dfops = dfops;
 	args->total = total;
 	args->whichfork = XFS_DATA_FORK;
 	args->trans = tp;
+	args->dfops = tp->t_dfops;
+	args->firstblock = first;
 	args->op_flags = XFS_DA_OP_ADDNAME | XFS_DA_OP_OKNOENT;
 	if (!inum)
 		args->op_flags |= XFS_DA_OP_JUSTCHECK;

@@ -421,7 +422,6 @@ xfs_dir_removename(
 	struct xfs_name	*name,
 	xfs_ino_t	ino,
 	xfs_fsblock_t	*first,		/* bmap's firstblock */
-	struct xfs_defer_ops	*dfops,		/* bmap's freeblock list */
 	xfs_extlen_t	total)		/* bmap's total block count */
 {
 	struct xfs_da_args *args;

@@ -429,6 +429,7 @@ 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);

@@ -443,10 +444,10 @@ xfs_dir_removename(
 	args->inumber = ino;
 	args->dp = dp;
 	args->firstblock = first;
-	args->dfops = dfops;
 	args->total = total;
 	args->whichfork = XFS_DATA_FORK;
 	args->trans = tp;
+	args->dfops = tp->t_dfops;
 
 	if (dp->i_d.di_format == XFS_DINODE_FMT_LOCAL) {
 		rval = xfs_dir2_sf_removename(args);

@@ -483,7 +484,6 @@ xfs_dir_replace(
 	struct xfs_name	*name,		/* name of entry to replace */
 	xfs_ino_t	inum,		/* new inode number */
 	xfs_fsblock_t	*first,		/* bmap's firstblock */
-	struct xfs_defer_ops	*dfops,		/* bmap's freeblock list */
 	xfs_extlen_t	total)		/* bmap's total block count */
 {
 	struct xfs_da_args *args;

@@ -491,6 +491,7 @@ 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)

@@ -508,10 +509,10 @@ xfs_dir_replace(
 	args->inumber = inum;
 	args->dp = dp;
 	args->firstblock = first;
-	args->dfops = dfops;
 	args->total = total;
 	args->whichfork = XFS_DATA_FORK;
 	args->trans = tp;
+	args->dfops = tp->t_dfops;
 
 	if (dp->i_d.di_format == XFS_DINODE_FMT_LOCAL) {
 		rval = xfs_dir2_sf_replace(args);

@@ -547,7 +548,7 @@ xfs_dir_canenter(
 	xfs_inode_t	*dp,
 	struct xfs_name	*name)		/* name of entry to add */
 {
-	return xfs_dir_createname(tp, dp, name, 0, NULL, NULL, 0);
+	return xfs_dir_createname(tp, dp, name, 0, NULL, 0);
 }
 
 /*

fs/xfs/libxfs/xfs_dir2.h +3 -6

index ed385316c7dc..f203aebc07ed 100644
@@ -118,19 +118,16 @@ extern int xfs_dir_init(struct xfs_trans *tp, struct xfs_inode *dp,
 				struct xfs_inode *pdp);
 extern int xfs_dir_createname(struct xfs_trans *tp, struct xfs_inode *dp,
 				struct xfs_name *name, xfs_ino_t inum,
-				xfs_fsblock_t *first,
-				struct xfs_defer_ops *dfops, xfs_extlen_t tot);
+				xfs_fsblock_t *first, xfs_extlen_t tot);
 extern int xfs_dir_lookup(struct xfs_trans *tp, struct xfs_inode *dp,
 				struct xfs_name *name, xfs_ino_t *inum,
 				struct xfs_name *ci_name);
 extern int xfs_dir_removename(struct xfs_trans *tp, struct xfs_inode *dp,
 				struct xfs_name *name, xfs_ino_t ino,
-				xfs_fsblock_t *first,
-				struct xfs_defer_ops *dfops, xfs_extlen_t tot);
+				xfs_fsblock_t *first, xfs_extlen_t tot);
 extern int xfs_dir_replace(struct xfs_trans *tp, struct xfs_inode *dp,
 				struct xfs_name *name, xfs_ino_t inum,
-				xfs_fsblock_t *first,
-				struct xfs_defer_ops *dfops, xfs_extlen_t tot);
+				xfs_fsblock_t *first, xfs_extlen_t tot);
 extern int xfs_dir_canenter(struct xfs_trans *tp, struct xfs_inode *dp,
 				struct xfs_name *name);
 

fs/xfs/xfs_inode.c +23 -27

index f5649a023d46..e1bc686b70b4 100644
@@ -1225,8 +1225,8 @@ xfs_create(
 	xfs_trans_ijoin(tp, dp, XFS_ILOCK_EXCL);
 	unlock_dp_on_error = false;
 
-	error = xfs_dir_createname(tp, dp, name, ip->i_ino,
-					&first_block, &dfops, resblks ?
+	error = xfs_dir_createname(tp, dp, name, ip->i_ino, &first_block,
+				   resblks ?
 					resblks - XFS_IALLOC_SPACE_RES(mp) : 0);
 	if (error) {
 		ASSERT(error != -ENOSPC);

@@ -1464,7 +1464,7 @@ xfs_link(
 	}
 
 	error = xfs_dir_createname(tp, tdp, target_name, sip->i_ino,
-					&first_block, &dfops, resblks);
+				   &first_block, resblks);
 	if (error)
 		goto error_return;
 	xfs_trans_ichgtime(tp, tdp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);

@@ -2659,8 +2659,8 @@ xfs_remove(
 
 	xfs_defer_init(&dfops, &first_block);
 	tp->t_dfops = &dfops;
-	error = xfs_dir_removename(tp, dp, name, ip->i_ino,
-					&first_block, &dfops, resblks);
+	error = xfs_dir_removename(tp, dp, name, ip->i_ino, &first_block,
+				   resblks);
 	if (error) {
 		ASSERT(error != -ENOENT);
 		goto out_bmap_cancel;

@@ -2748,9 +2748,9 @@ xfs_sort_for_rename(
 
 static int
 xfs_finish_rename(
-	struct xfs_trans	*tp,
-	struct xfs_defer_ops	*dfops)
+	struct xfs_trans	*tp)
 {
+	struct xfs_defer_ops	*dfops = tp->t_dfops;
 	int			error;
 
 	/*

@@ -2784,7 +2784,6 @@ xfs_cross_rename(
 	struct xfs_inode	*dp2,
 	struct xfs_name		*name2,
 	struct xfs_inode	*ip2,
-	struct xfs_defer_ops	*dfops,
 	xfs_fsblock_t		*first_block,
 	int			spaceres)
 {

@@ -2794,16 +2793,14 @@ xfs_cross_rename(
 	int		dp2_flags = 0;
 
 	/* Swap inode number for dirent in first parent */
-	error = xfs_dir_replace(tp, dp1, name1,
-				ip2->i_ino,
-				first_block, dfops, spaceres);
+	error = xfs_dir_replace(tp, dp1, name1, ip2->i_ino, first_block,
+				spaceres);
 	if (error)
 		goto out_trans_abort;
 
 	/* Swap inode number for dirent in second parent */
-	error = xfs_dir_replace(tp, dp2, name2,
-				ip1->i_ino,
-				first_block, dfops, spaceres);
+	error = xfs_dir_replace(tp, dp2, name2, ip1->i_ino, first_block,
+				spaceres);
 	if (error)
 		goto out_trans_abort;
 

@@ -2818,7 +2815,7 @@ xfs_cross_rename(
 		if (S_ISDIR(VFS_I(ip2)->i_mode)) {
 			error = xfs_dir_replace(tp, ip2, &xfs_name_dotdot,
 						dp1->i_ino, first_block,
-						dfops, spaceres);
+						spaceres);
 			if (error)
 				goto out_trans_abort;
 

@@ -2845,7 +2842,7 @@ xfs_cross_rename(
 		if (S_ISDIR(VFS_I(ip1)->i_mode)) {
 			error = xfs_dir_replace(tp, ip1, &xfs_name_dotdot,
 						dp2->i_ino, first_block,
-						dfops, spaceres);
+						spaceres);
 			if (error)
 				goto out_trans_abort;
 

@@ -2884,10 +2881,10 @@ xfs_cross_rename(
 	}
 	xfs_trans_ichgtime(tp, dp1, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
 	xfs_trans_log_inode(tp, dp1, XFS_ILOG_CORE);
-	return xfs_finish_rename(tp, dfops);
+	return xfs_finish_rename(tp);
 
 out_trans_abort:
-	xfs_defer_cancel(dfops);
+	xfs_defer_cancel(tp->t_dfops);
 	xfs_trans_cancel(tp);
 	return error;
 }

@@ -3032,7 +3029,7 @@ xfs_rename(
 	if (flags & RENAME_EXCHANGE)
 		return xfs_cross_rename(tp, src_dp, src_name, src_ip,
 					target_dp, target_name, target_ip,
-					&dfops, &first_block, spaceres);
+					&first_block, spaceres);
 
 	/*
 	 * Set up the target.

@@ -3054,7 +3051,7 @@ xfs_rename(
 		 */
 		error = xfs_dir_createname(tp, target_dp, target_name,
 						src_ip->i_ino, &first_block,
-						&dfops, spaceres);
+						spaceres);
 		if (error)
 			goto out_bmap_cancel;
 

@@ -3093,8 +3090,7 @@ xfs_rename(
 		 * name at the destination directory, remove it first.
 		 */
 		error = xfs_dir_replace(tp, target_dp, target_name,
-					src_ip->i_ino,
-					&first_block, &dfops, spaceres);
+					src_ip->i_ino, &first_block, spaceres);
 		if (error)
 			goto out_bmap_cancel;
 

@@ -3128,8 +3124,8 @@ xfs_rename(
 		 * directory.
 		 */
 		error = xfs_dir_replace(tp, src_ip, &xfs_name_dotdot,
-					target_dp->i_ino,
-					&first_block, &dfops, spaceres);
+					target_dp->i_ino, &first_block,
+					spaceres);
 		ASSERT(error != -EEXIST);
 		if (error)
 			goto out_bmap_cancel;

@@ -3168,10 +3164,10 @@ xfs_rename(
 	 */
 	if (wip) {
 		error = xfs_dir_replace(tp, src_dp, src_name, wip->i_ino,
-					&first_block, &dfops, spaceres);
+					&first_block, spaceres);
 	} else
 		error = xfs_dir_removename(tp, src_dp, src_name, src_ip->i_ino,
-					   &first_block, &dfops, spaceres);
+					   &first_block, spaceres);
 	if (error)
 		goto out_bmap_cancel;
 

@@ -3206,7 +3202,7 @@ xfs_rename(
 	if (new_parent)
 		xfs_trans_log_inode(tp, target_dp, XFS_ILOG_CORE);
 
-	error = xfs_finish_rename(tp, &dfops);
+	error = xfs_finish_rename(tp);
 	if (wip)
 		IRELE(wip);
 	return error;

fs/xfs/xfs_symlink.c +1 -1

index 44335bdebea2..e347a3db018f 100644
@@ -339,7 +339,7 @@ xfs_symlink(
 	 * Create the directory entry for the symlink.
 	 */
 	error = xfs_dir_createname(tp, dp, link_name, ip->i_ino,
-					&first_block, &dfops, resblks);
+				   &first_block, resblks);
 	if (error)
 		goto out_bmap_cancel;
 	xfs_trans_ichgtime(tp, dp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);

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

Log recovery passes down a central dfops structure to recovery handlers for bui and cui log items. Each of these handlers allocates and commits a transaction and defers any remaining operations to be completed by the main recovery sequence.

Since dfops outlives the transaction in this context, set and clear ->t_dfops appropriately such that the *_finish_item() paths and below (i.e., xfs_bmapi*()) can expect to find the dfops in the transaction without it being committed with the dfops attached. This is required because transaction commit expects that an associated dfops is finished and in this context the dfops may be populated at commit time.

Quoted 5 lines · Brian Foster

Log recovery passes down a central dfops structure to recovery handlers for bui and cui log items. Each of these handlers allocates and commits a transaction and defers any remaining operations to be completed by the main recovery sequence.

Quoted 7 lines

Since dfops outlives the transaction in this context, set and clear ->t_dfops appropriately such that the *_finish_item() paths and below (i.e., xfs_bmapi*()) can expect to find the dfops in the transaction without it being committed with the dfops attached. This is required because transaction commit expects that an associated dfops is finished and in this context the dfops may be populated at commit time.

Do we want comments explaining this in the code?

Otherwise looks fine:

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

Log recovery passes down a central dfops structure to recovery handlers for bui and cui log items. Each of these handlers allocates and commits a transaction and defers any remaining operations to be completed by the main recovery sequence.

Since dfops outlives the transaction in this context, set and clear ->t_dfops appropriately such that the *_finish_item() paths and below (i.e., xfs_bmapi*()) can expect to find the dfops in the transaction without it being committed with the dfops attached. This is required because transaction commit expects that an associated dfops is finished and in this context the dfops may be populated at commit time.

Signed-off-by Brian Foster <[email protected]>

Reviewed-by Christoph Hellwig <[email protected]>

---

v2:
- Add comments to explain unique ->t_dfops usage in bui/cui log item recovery.

 fs/xfs/xfs_bmap_item.c     | 8 ++++++++
 fs/xfs/xfs_refcount_item.c | 8 ++++++++

 2 files changed, 16 insertions(+)

fs/xfs/xfs_bmap_item.c +8 -0

index 956ebd583e27..478bfc798861 100644
@@ -441,6 +441,7 @@ xfs_bui_recover(
 			XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK), 0, 0, &tp);
 	if (error)
 		return error;
+	tp->t_dfops = dfops;
 	budp = xfs_trans_get_bud(tp, buip);
 
 	/* Grab the inode. */

@@ -487,6 +488,12 @@ xfs_bui_recover(
 	}
 
 	set_bit(XFS_BUI_RECOVERED, &buip->bui_flags);
+	/*
+	 * Recovery finishes all deferred ops once intent processing is
+	 * complete. Reset the trans reference because commit expects a finished
+	 * dfops or none at all.
+	 */
+	tp->t_dfops = NULL;
 	error = xfs_trans_commit(tp);
 	xfs_iunlock(ip, XFS_ILOCK_EXCL);
 	IRELE(ip);

@@ -494,6 +501,7 @@ xfs_bui_recover(
 	return error;
 
 err_inode:
+	tp->t_dfops = NULL;
 	xfs_trans_cancel(tp);
 	if (ip) {
 		xfs_iunlock(ip, XFS_ILOCK_EXCL);

fs/xfs/xfs_refcount_item.c +8 -0

index 472a73e9d331..2064c689bc72 100644
@@ -452,6 +452,7 @@ xfs_cui_recover(
 			mp->m_refc_maxlevels * 2, 0, XFS_TRANS_RESERVE, &tp);
 	if (error)
 		return error;
+	tp->t_dfops = dfops;
 	cudp = xfs_trans_get_cud(tp, cuip);
 
 	for (i = 0; i < cuip->cui_format.cui_nextents; i++) {

@@ -514,11 +515,18 @@ xfs_cui_recover(
 
 	xfs_refcount_finish_one_cleanup(tp, rcur, error);
 	set_bit(XFS_CUI_RECOVERED, &cuip->cui_flags);
+	/*
+	 * Recovery finishes all deferred ops once intent processing is
+	 * complete. Reset the trans reference because commit expects a finished
+	 * dfops or none at all.
+	 */
+	tp->t_dfops = NULL;
 	error = xfs_trans_commit(tp);
 	return error;
 
 abort_error:
 	xfs_refcount_finish_one_cleanup(tp, rcur, error);
+	tp->t_dfops = NULL;
 	xfs_trans_cancel(tp);
 	return error;
 }

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

Quoted 21 lines · Brian Foster

Log recovery passes down a central dfops structure to recovery handlers for bui and cui log items. Each of these handlers allocates and commits a transaction and defers any remaining operations to be completed by the main recovery sequence.

Since dfops outlives the transaction in this context, set and clear ->t_dfops appropriately such that the *_finish_item() paths and below (i.e., xfs_bmapi*()) can expect to find the dfops in the transaction without it being committed with the dfops attached. This is required because transaction commit expects that an associated dfops is finished and in this context the dfops may be populated at commit time.

Signed-off-by Brian Foster <[email protected]>

Reviewed-by Christoph Hellwig <[email protected]>

---

v2:
- Add comments to explain unique ->t_dfops usage in bui/cui log item recovery.

Quoted 2 lines

 fs/xfs/xfs_bmap_item.c     | 8 ++++++++
 fs/xfs/xfs_refcount_item.c | 8 ++++++++

General note: realtime rmap (being rooted in an inode) log item replay can generate further dfops, so I'll have to wire that up when I pass the log recovery dfops collector into xfs_rui_recover, and I'll have to remember to attach it to the transaction when I do that.

Otherwise I think this is fine,

Reviewed-by Darrick J. Wong <[email protected]>

--D

Quoted 73 lines

 2 files changed, 16 insertions(+)

fs/xfs/xfs_bmap_item.c +8 -0

index 956ebd583e27..478bfc798861 100644
@@ -441,6 +441,7 @@ xfs_bui_recover(
 			XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK), 0, 0, &tp);
 	if (error)
 		return error;
+	tp->t_dfops = dfops;
 	budp = xfs_trans_get_bud(tp, buip);
 
 	/* Grab the inode. */

@@ -487,6 +488,12 @@ xfs_bui_recover(
 	}
 
 	set_bit(XFS_BUI_RECOVERED, &buip->bui_flags);
+	/*
+	 * Recovery finishes all deferred ops once intent processing is
+	 * complete. Reset the trans reference because commit expects a finished
+	 * dfops or none at all.
+	 */
+	tp->t_dfops = NULL;
 	error = xfs_trans_commit(tp);
 	xfs_iunlock(ip, XFS_ILOCK_EXCL);
 	IRELE(ip);

@@ -494,6 +501,7 @@ xfs_bui_recover(
 	return error;
 
 err_inode:
+	tp->t_dfops = NULL;
 	xfs_trans_cancel(tp);
 	if (ip) {
 		xfs_iunlock(ip, XFS_ILOCK_EXCL);

fs/xfs/xfs_refcount_item.c +8 -0

index 472a73e9d331..2064c689bc72 100644
@@ -452,6 +452,7 @@ xfs_cui_recover(
 			mp->m_refc_maxlevels * 2, 0, XFS_TRANS_RESERVE, &tp);
 	if (error)
 		return error;
+	tp->t_dfops = dfops;
 	cudp = xfs_trans_get_cud(tp, cuip);
 
 	for (i = 0; i < cuip->cui_format.cui_nextents; i++) {

@@ -514,11 +515,18 @@ xfs_cui_recover(
 
 	xfs_refcount_finish_one_cleanup(tp, rcur, error);
 	set_bit(XFS_CUI_RECOVERED, &cuip->cui_flags);
+	/*
+	 * Recovery finishes all deferred ops once intent processing is
+	 * complete. Reset the trans reference because commit expects a finished
+	 * dfops or none at all.
+	 */
+	tp->t_dfops = NULL;
 	error = xfs_trans_commit(tp);
 	return error;
 
 abort_error:
 	xfs_refcount_finish_one_cleanup(tp, rcur, error);
+	tp->t_dfops = NULL;
 	xfs_trans_cancel(tp);
 	return error;
 }

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

Quoted 22 lines · Darrick J. Wong

Quoted 21 lines · Brian Foster

Log recovery passes down a central dfops structure to recovery handlers for bui and cui log items. Each of these handlers allocates and commits a transaction and defers any remaining operations to be completed by the main recovery sequence.

Since dfops outlives the transaction in this context, set and clear ->t_dfops appropriately such that the *_finish_item() paths and below (i.e., xfs_bmapi*()) can expect to find the dfops in the transaction without it being committed with the dfops attached. This is required because transaction commit expects that an associated dfops is finished and in this context the dfops may be populated at commit time.

Signed-off-by Brian Foster <[email protected]>

Reviewed-by Christoph Hellwig <[email protected]>

---

v2:
- Add comments to explain unique ->t_dfops usage in bui/cui log item recovery.

Quoted 8 lines

Quoted 2 lines

 fs/xfs/xfs_bmap_item.c     | 8 ++++++++
 fs/xfs/xfs_refcount_item.c | 8 ++++++++

General note: realtime rmap (being rooted in an inode) log item replay can generate further dfops, so I'll have to wire that up when I pass the log recovery dfops collector into xfs_rui_recover, and I'll have to remember to attach it to the transaction when I do that.

Ok. I'm hoping that a last step to embed ->t_dfops directly in the transaction may clean up this ugly set/unset pattern a bit. What I'm thinking is that a transaction roll would end up having to transfer (splice) the pending dfops items from the current ->t_dfops to the new one rather than simply copy the pointer. Some of the dfops code has to become a bit more transaction centric to make sure we don't lose the dfops pointer, but otherwise if we factor that into a little helper I think that means these recovery functions would just have to do something like:

	xfs_defer_move(&tp->t_dfops, dfops);

... before committing tp to collect all the deferred items in the dfops from the caller. Otherwise, the transaction commit would actually run xfs_defer_finish() itself if the associated dfops has pending work. I haven't really dug into that yet, though, so there may be more issues to consider (thoughts appreciated, if that triggers any for you).

Brian

Quoted 74 lines

Otherwise I think this is fine,

Reviewed-by Darrick J. Wong <[email protected]>

--D

Quoted 69 lines

 2 files changed, 16 insertions(+)

fs/xfs/xfs_bmap_item.c +8 -0

index 956ebd583e27..478bfc798861 100644
@@ -441,6 +441,7 @@ xfs_bui_recover(
 			XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK), 0, 0, &tp);
 	if (error)
 		return error;
+	tp->t_dfops = dfops;
 	budp = xfs_trans_get_bud(tp, buip);
 
 	/* Grab the inode. */

@@ -487,6 +488,12 @@ xfs_bui_recover(
 	}
 
 	set_bit(XFS_BUI_RECOVERED, &buip->bui_flags);
+	/*
+	 * Recovery finishes all deferred ops once intent processing is
+	 * complete. Reset the trans reference because commit expects a finished
+	 * dfops or none at all.
+	 */
+	tp->t_dfops = NULL;
 	error = xfs_trans_commit(tp);
 	xfs_iunlock(ip, XFS_ILOCK_EXCL);
 	IRELE(ip);

@@ -494,6 +501,7 @@ xfs_bui_recover(
 	return error;
 
 err_inode:
+	tp->t_dfops = NULL;
 	xfs_trans_cancel(tp);
 	if (ip) {
 		xfs_iunlock(ip, XFS_ILOCK_EXCL);

fs/xfs/xfs_refcount_item.c +8 -0

index 472a73e9d331..2064c689bc72 100644
@@ -452,6 +452,7 @@ xfs_cui_recover(
 			mp->m_refc_maxlevels * 2, 0, XFS_TRANS_RESERVE, &tp);
 	if (error)
 		return error;
+	tp->t_dfops = dfops;
 	cudp = xfs_trans_get_cud(tp, cuip);
 
 	for (i = 0; i < cuip->cui_format.cui_nextents; i++) {

@@ -514,11 +515,18 @@ xfs_cui_recover(
 
 	xfs_refcount_finish_one_cleanup(tp, rcur, error);
 	set_bit(XFS_CUI_RECOVERED, &cuip->cui_flags);
+	/*
+	 * Recovery finishes all deferred ops once intent processing is
+	 * complete. Reset the trans reference because commit expects a finished
+	 * dfops or none at all.
+	 */
+	tp->t_dfops = NULL;
 	error = xfs_trans_commit(tp);
 	return error;
 
 abort_error:
 	xfs_refcount_finish_one_cleanup(tp, rcur, error);
+	tp->t_dfops = NULL;
 	xfs_trans_cancel(tp);
 	return error;
 }

Signature · 1 line

2.17.1

Quoted 4 lines

Quoted 4 lines

--
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

--
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

Attach the local dfops to the transaction allocated for xattr add and remove operations. Add an earlier initialization in xfs_attr_remove() to ensure the structure is valid if it remains unused at transaction commit time.

Attach the local dfops to the transaction allocated for xattr add and remove operations. Add an earlier initialization in xfs_attr_remove() to ensure the structure is valid if it remains unused at transaction commit time.

Signed-off-by Brian Foster <[email protected]>

---
 fs/xfs/libxfs/xfs_attr.c | 11 +++++++----
 fs/xfs/libxfs/xfs_bmap.c |  3 ++-
 2 files changed, 9 insertions(+), 5 deletions(-)

fs/xfs/libxfs/xfs_attr.c +7 -4

index 99590f61d624..cc23c269f2bf 100644
@@ -254,6 +254,8 @@ xfs_attr_set(
 			rsvd ? XFS_TRANS_RESERVE : 0, &args.trans);
 	if (error)
 		return error;
+	xfs_defer_init(&dfops, &firstblock);
+	args.trans->t_dfops = &dfops;
 
 	xfs_ilock(dp, XFS_ILOCK_EXCL);
 	error = xfs_trans_reserve_quota_nblks(args.trans, dp, args.total, 0,

@@ -315,7 +317,6 @@ xfs_attr_set(
 		 * It won't fit in the shortform, transform to a leaf block.
 		 * GROT: another possible req'mt for a double-split btree op.
 		 */
-		xfs_defer_init(args.dfops, args.firstblock);
 		error = xfs_attr_shortform_to_leaf(&args, &leaf_bp);
 		if (error)
 			goto out_defer_cancel;

@@ -325,9 +326,9 @@ xfs_attr_set(
 		 * buffer and run into problems with the write verifier.
 		 */
 		xfs_trans_bhold(args.trans, leaf_bp);
-		xfs_defer_bjoin(args.dfops, leaf_bp);
-		xfs_defer_ijoin(args.dfops, dp);
-		error = xfs_defer_finish(&args.trans, args.dfops);
+		xfs_defer_bjoin(&dfops, leaf_bp);
+		xfs_defer_ijoin(&dfops, dp);
+		error = xfs_defer_finish(&args.trans, &dfops);
 		if (error)
 			goto out_defer_cancel;
 

@@ -429,6 +430,8 @@ xfs_attr_remove(
 			&args.trans);
 	if (error)
 		return error;
+	xfs_defer_init(&dfops, &firstblock);
+	args.trans->t_dfops = &dfops;
 
 	xfs_ilock(dp, XFS_ILOCK_EXCL);
 	/*

fs/xfs/libxfs/xfs_bmap.c +2 -1

index 7205268b30bc..16bbc8114d62 100644
@@ -1057,6 +1057,8 @@ xfs_bmap_add_attrfork(
 			rsvd ? XFS_TRANS_RESERVE : 0, &tp);
 	if (error)
 		return error;
+	xfs_defer_init(&dfops, &firstblock);
+	tp->t_dfops = &dfops;
 
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
 	error = xfs_trans_reserve_quota_nblks(tp, ip, blks, 0, rsvd ?

@@ -1104,7 +1106,6 @@ xfs_bmap_add_attrfork(
 	ip->i_afp = kmem_zone_zalloc(xfs_ifork_zone, KM_SLEEP);
 	ip->i_afp->if_flags = XFS_IFEXTENTS;
 	logflags = 0;
-	xfs_defer_init(&dfops, &firstblock);
 	switch (ip->i_d.di_format) {
 	case XFS_DINODE_FMT_LOCAL:
 		error = xfs_bmap_add_attrfork_local(tp, ip, &firstblock, &dfops,

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

Now that the attribute fork add tx carries dfops along with the transaction, it is unnecessary to pass it down the stack. Remove the dfops parameter and access ->t_dfops directly where necessary. This patch does not change behavior.

Signed-off-by Brian Foster <[email protected]>

Now that the attribute fork add tx carries dfops along with the transaction, it is unnecessary to pass it down the stack. Remove the dfops parameter and access ->t_dfops directly where necessary. This patch does not change behavior.

Signed-off-by Brian Foster <[email protected]>

---
 fs/xfs/libxfs/xfs_bmap.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

fs/xfs/libxfs/xfs_bmap.c +8 -11

index 16bbc8114d62..c4b81fee2dee 100644
@@ -918,7 +918,6 @@ xfs_bmap_add_attrfork_btree(
 	xfs_trans_t		*tp,		/* transaction pointer */
 	xfs_inode_t		*ip,		/* incore inode pointer */
 	xfs_fsblock_t		*firstblock,	/* first block allocated */
-	struct xfs_defer_ops	*dfops,		/* blocks to free at commit */
 	int			*flags)		/* inode logging flags */
 {
 	xfs_btree_cur_t		*cur;		/* btree cursor */

@@ -931,7 +930,7 @@ xfs_bmap_add_attrfork_btree(
 		*flags |= XFS_ILOG_DBROOT;
 	else {
 		cur = xfs_bmbt_init_cursor(mp, tp, ip, XFS_DATA_FORK);
-		cur->bc_private.b.dfops = dfops;
+		cur->bc_private.b.dfops = tp->t_dfops;
 		cur->bc_private.b.firstblock = *firstblock;
 		error = xfs_bmbt_lookup_first(cur, &stat);
 		if (error)

@@ -962,7 +961,6 @@ xfs_bmap_add_attrfork_extents(
 	xfs_trans_t		*tp,		/* transaction pointer */
 	xfs_inode_t		*ip,		/* incore inode pointer */
 	xfs_fsblock_t		*firstblock,	/* first block allocated */
-	struct xfs_defer_ops	*dfops,		/* blocks to free at commit */
 	int			*flags)		/* inode logging flags */
 {
 	xfs_btree_cur_t		*cur;		/* bmap btree cursor */

@@ -971,7 +969,7 @@ xfs_bmap_add_attrfork_extents(
 	if (ip->i_d.di_nextents * sizeof(xfs_bmbt_rec_t) <= XFS_IFORK_DSIZE(ip))
 		return 0;
 	cur = NULL;
-	error = xfs_bmap_extents_to_btree(tp, ip, firstblock, dfops, &cur, 0,
+	error = xfs_bmap_extents_to_btree(tp, ip, firstblock, tp->t_dfops, &cur, 0,
 		flags, XFS_DATA_FORK);
 	if (cur) {
 		cur->bc_private.b.allocated = 0;

@@ -997,7 +995,6 @@ xfs_bmap_add_attrfork_local(
 	xfs_trans_t		*tp,		/* transaction pointer */
 	xfs_inode_t		*ip,		/* incore inode pointer */
 	xfs_fsblock_t		*firstblock,	/* first block allocated */
-	struct xfs_defer_ops	*dfops,		/* blocks to free at commit */
 	int			*flags)		/* inode logging flags */
 {
 	xfs_da_args_t		dargs;		/* args for dir/attr code */

@@ -1010,7 +1007,7 @@ xfs_bmap_add_attrfork_local(
 		dargs.geo = ip->i_mount->m_dir_geo;
 		dargs.dp = ip;
 		dargs.firstblock = firstblock;
-		dargs.dfops = dfops;
+		dargs.dfops = tp->t_dfops;
 		dargs.total = dargs.geo->fsbcount;
 		dargs.whichfork = XFS_DATA_FORK;
 		dargs.trans = tp;

@@ -1108,16 +1105,16 @@ xfs_bmap_add_attrfork(
 	logflags = 0;
 	switch (ip->i_d.di_format) {
 	case XFS_DINODE_FMT_LOCAL:
-		error = xfs_bmap_add_attrfork_local(tp, ip, &firstblock, &dfops,
-			&logflags);
+		error = xfs_bmap_add_attrfork_local(tp, ip, &firstblock,
+						    &logflags);
 		break;
 	case XFS_DINODE_FMT_EXTENTS:
 		error = xfs_bmap_add_attrfork_extents(tp, ip, &firstblock,
-			&dfops, &logflags);
+						      &logflags);
 		break;
 	case XFS_DINODE_FMT_BTREE:
-		error = xfs_bmap_add_attrfork_btree(tp, ip, &firstblock, &dfops,
-			&logflags);
+		error = xfs_bmap_add_attrfork_btree(tp, ip, &firstblock,
+						    &logflags);
 		break;
 	default:
 		error = 0;

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

Attach the local dfops to ->t_dfops of the extent split transaction. Since this is the only caller of xfs_bmap_split_extent_at(), remove the dfops parameter as well.

Signed-off-by Brian Foster <[email protected]>

Attach the local dfops to ->t_dfops of the extent split transaction. Since this is the only caller of xfs_bmap_split_extent_at(), remove the dfops parameter as well.

Signed-off-by Brian Foster <[email protected]>

---
 fs/xfs/libxfs/xfs_bmap.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

fs/xfs/libxfs/xfs_bmap.c +6 -7

index c4b81fee2dee..dbd07b0adfa3 100644
@@ -5921,8 +5921,7 @@ xfs_bmap_split_extent_at(
 	struct xfs_trans	*tp,
 	struct xfs_inode	*ip,
 	xfs_fileoff_t		split_fsb,
-	xfs_fsblock_t		*firstfsb,
-	struct xfs_defer_ops	*dfops)
+	xfs_fsblock_t		*firstfsb)
 {
 	int				whichfork = XFS_DATA_FORK;
 	struct xfs_btree_cur		*cur = NULL;

@@ -5972,7 +5971,7 @@ xfs_bmap_split_extent_at(
 	if (ifp->if_flags & XFS_IFBROOT) {
 		cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
 		cur->bc_private.b.firstblock = *firstfsb;
-		cur->bc_private.b.dfops = dfops;
+		cur->bc_private.b.dfops = tp->t_dfops;
 		cur->bc_private.b.flags = 0;
 		error = xfs_bmbt_lookup_eq(cur, &got, &i);
 		if (error)

@@ -6016,7 +6015,7 @@ xfs_bmap_split_extent_at(
 		int tmp_logflags; /* partial log flag return val */
 
 		ASSERT(cur == NULL);
-		error = xfs_bmap_extents_to_btree(tp, ip, firstfsb, dfops,
+		error = xfs_bmap_extents_to_btree(tp, ip, firstfsb, tp->t_dfops,
 				&cur, 0, &tmp_logflags, whichfork);
 		logflags |= tmp_logflags;
 	}

@@ -6048,14 +6047,14 @@ xfs_bmap_split_extent(
 			XFS_DIOSTRAT_SPACE_RES(mp, 0), 0, 0, &tp);
 	if (error)
 		return error;
+	xfs_defer_init(&dfops, &firstfsb);
+	tp->t_dfops = &dfops;
 
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
 	xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
 
-	xfs_defer_init(&dfops, &firstfsb);
-
 	error = xfs_bmap_split_extent_at(tp, ip, split_fsb,
-			&firstfsb, &dfops);
+					 &firstfsb);
 	if (error)
 		goto out;
 

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

Now that xfs_da_args->dfops is always assigned from a ->t_dfops pointer (or one that is immediately attached), replace all downstream accesses of the former with the latter and remove the field from struct xfs_da_args.

Signed-off-by Brian Foster <[email protected]>

Now that xfs_da_args->dfops is always assigned from a ->t_dfops pointer (or one that is immediately attached), replace all downstream accesses of the former with the latter and remove the field from struct xfs_da_args.

Signed-off-by Brian Foster <[email protected]>

---
 fs/xfs/libxfs/xfs_attr.c        | 103 ++++++++++++++++----------------
 fs/xfs/libxfs/xfs_attr_leaf.c   |  24 ++++----
 fs/xfs/libxfs/xfs_attr_remote.c |  23 +++----
 fs/xfs/libxfs/xfs_bmap.c        |   1 -
 fs/xfs/libxfs/xfs_da_btree.c    |  19 +++---
 fs/xfs/libxfs/xfs_da_btree.h    |   1 -
 fs/xfs/libxfs/xfs_dir2.c        |  65 ++++++++++----------
 7 files changed, 117 insertions(+), 119 deletions(-)

fs/xfs/libxfs/xfs_attr.c +53 -50

index cc23c269f2bf..a14ab9b2669e 100644
@@ -220,7 +220,6 @@ xfs_attr_set(
 	args.value = value;
 	args.valuelen = valuelen;
 	args.firstblock = &firstblock;
-	args.dfops = &dfops;
 	args.op_flags = XFS_DA_OP_ADDNAME | XFS_DA_OP_OKNOENT;
 	args.total = xfs_attr_calc_size(&args, &local);
 

@@ -407,7 +406,6 @@ xfs_attr_remove(
 		return error;
 
 	args.firstblock = &firstblock;
-	args.dfops = &dfops;
 
 	/*
 	 * we have no control over the attribute names that userspace passes us

@@ -539,11 +537,12 @@ xfs_attr_shortform_addname(xfs_da_args_t *args)
  * if bmap_one_block() says there is only one block (ie: no remote blks).
  */
 STATIC int
-xfs_attr_leaf_addname(xfs_da_args_t *args)
+xfs_attr_leaf_addname(
+	struct xfs_da_args	*args)
 {
-	xfs_inode_t *dp;
-	struct xfs_buf *bp;
-	int retval, error, forkoff;
+	struct xfs_inode	*dp;
+	struct xfs_buf		*bp;
+	int			retval, error, forkoff;
 
 	trace_xfs_attr_leaf_addname(args);
 

@@ -601,12 +600,12 @@ xfs_attr_leaf_addname(xfs_da_args_t *args)
 		 * Commit that transaction so that the node_addname() call
 		 * can manage its own transactions.
 		 */
-		xfs_defer_init(args->dfops, args->firstblock);
+		xfs_defer_init(args->trans->t_dfops, args->firstblock);
 		error = xfs_attr3_leaf_to_node(args);
 		if (error)
 			goto out_defer_cancel;
-		xfs_defer_ijoin(args->dfops, dp);
-		error = xfs_defer_finish(&args->trans, args->dfops);
+		xfs_defer_ijoin(args->trans->t_dfops, dp);
+		error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
 		if (error)
 			goto out_defer_cancel;
 

@@ -690,13 +689,13 @@ xfs_attr_leaf_addname(xfs_da_args_t *args)
 		 * If the result is small enough, shrink it all into the inode.
 		 */
 		if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) {
-			xfs_defer_init(args->dfops, args->firstblock);
+			xfs_defer_init(args->trans->t_dfops, args->firstblock);
 			error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
 			/* bp is gone due to xfs_da_shrink_inode */
 			if (error)
 				goto out_defer_cancel;
-			xfs_defer_ijoin(args->dfops, dp);
-			error = xfs_defer_finish(&args->trans, args->dfops);
+			xfs_defer_ijoin(args->trans->t_dfops, dp);
+			error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
 			if (error)
 				goto out_defer_cancel;
 		}

@@ -714,7 +713,7 @@ xfs_attr_leaf_addname(xfs_da_args_t *args)
 	}
 	return error;
 out_defer_cancel:
-	xfs_defer_cancel(args->dfops);
+	xfs_defer_cancel(args->trans->t_dfops);
 	return error;
 }
 

@@ -725,11 +724,12 @@ xfs_attr_leaf_addname(xfs_da_args_t *args)
  * if bmap_one_block() says there is only one block (ie: no remote blks).
  */
 STATIC int
-xfs_attr_leaf_removename(xfs_da_args_t *args)
+xfs_attr_leaf_removename(
+	struct xfs_da_args	*args)
 {
-	xfs_inode_t *dp;
-	struct xfs_buf *bp;
-	int error, forkoff;
+	struct xfs_inode	*dp;
+	struct xfs_buf		*bp;
+	int			error, forkoff;
 
 	trace_xfs_attr_leaf_removename(args);
 

@@ -754,19 +754,19 @@ xfs_attr_leaf_removename(xfs_da_args_t *args)
 	 * If the result is small enough, shrink it all into the inode.
 	 */
 	if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) {
-		xfs_defer_init(args->dfops, args->firstblock);
+		xfs_defer_init(args->trans->t_dfops, args->firstblock);
 		error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
 		/* bp is gone due to xfs_da_shrink_inode */
 		if (error)
 			goto out_defer_cancel;
-		xfs_defer_ijoin(args->dfops, dp);
-		error = xfs_defer_finish(&args->trans, args->dfops);
+		xfs_defer_ijoin(args->trans->t_dfops, dp);
+		error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
 		if (error)
 			goto out_defer_cancel;
 	}
 	return 0;
 out_defer_cancel:
-	xfs_defer_cancel(args->dfops);
+	xfs_defer_cancel(args->trans->t_dfops);
 	return error;
 }
 

@@ -817,13 +817,14 @@ xfs_attr_leaf_get(xfs_da_args_t *args)
  * add a whole extra layer of confusion on top of that.
  */
 STATIC int
-xfs_attr_node_addname(xfs_da_args_t *args)
+xfs_attr_node_addname(
+	struct xfs_da_args	*args)
 {
-	xfs_da_state_t *state;
-	xfs_da_state_blk_t *blk;
-	xfs_inode_t *dp;
-	xfs_mount_t *mp;
-	int retval, error;
+	struct xfs_da_state	*state;
+	struct xfs_da_state_blk	*blk;
+	struct xfs_inode	*dp;
+	struct xfs_mount	*mp;
+	int			retval, error;
 
 	trace_xfs_attr_node_addname(args);
 

@@ -882,12 +883,13 @@ xfs_attr_node_addname(xfs_da_args_t *args)
 			 */
 			xfs_da_state_free(state);
 			state = NULL;
-			xfs_defer_init(args->dfops, args->firstblock);
+			xfs_defer_init(args->trans->t_dfops, args->firstblock);
 			error = xfs_attr3_leaf_to_node(args);
 			if (error)
 				goto out_defer_cancel;
-			xfs_defer_ijoin(args->dfops, dp);
-			error = xfs_defer_finish(&args->trans, args->dfops);
+			xfs_defer_ijoin(args->trans->t_dfops, dp);
+			error = xfs_defer_finish(&args->trans,
+						 args->trans->t_dfops);
 			if (error)
 				goto out_defer_cancel;
 

@@ -908,12 +910,12 @@ xfs_attr_node_addname(xfs_da_args_t *args)
 		 * in the index/blkno/rmtblkno/rmtblkcnt fields and
 		 * in the index2/blkno2/rmtblkno2/rmtblkcnt2 fields.
 		 */
-		xfs_defer_init(args->dfops, args->firstblock);
+		xfs_defer_init(args->trans->t_dfops, args->firstblock);
 		error = xfs_da3_split(state);
 		if (error)
 			goto out_defer_cancel;
-		xfs_defer_ijoin(args->dfops, dp);
-		error = xfs_defer_finish(&args->trans, args->dfops);
+		xfs_defer_ijoin(args->trans->t_dfops, dp);
+		error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
 		if (error)
 			goto out_defer_cancel;
 	} else {

@@ -1006,12 +1008,12 @@ xfs_attr_node_addname(xfs_da_args_t *args)
 		 * Check to see if the tree needs to be collapsed.
 		 */
 		if (retval && (state->path.active > 1)) {
-			xfs_defer_init(args->dfops, args->firstblock);
+			xfs_defer_init(args->trans->t_dfops, args->firstblock);
 			error = xfs_da3_join(state);
 			if (error)
 				goto out_defer_cancel;
-			xfs_defer_ijoin(args->dfops, dp);
-			error = xfs_defer_finish(&args->trans, args->dfops);
+			xfs_defer_ijoin(args->trans->t_dfops, dp);
+			error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
 			if (error)
 				goto out_defer_cancel;
 		}

@@ -1040,7 +1042,7 @@ xfs_attr_node_addname(xfs_da_args_t *args)
 		return error;
 	return retval;
 out_defer_cancel:
-	xfs_defer_cancel(args->dfops);
+	xfs_defer_cancel(args->trans->t_dfops);
 	goto out;
 }
 

@@ -1052,13 +1054,14 @@ xfs_attr_node_addname(xfs_da_args_t *args)
  * the root node (a special case of an intermediate node).
  */
 STATIC int
-xfs_attr_node_removename(xfs_da_args_t *args)
+xfs_attr_node_removename(
+	struct xfs_da_args	*args)
 {
-	xfs_da_state_t *state;
-	xfs_da_state_blk_t *blk;
-	xfs_inode_t *dp;
-	struct xfs_buf *bp;
-	int retval, error, forkoff;
+	struct xfs_da_state	*state;
+	struct xfs_da_state_blk	*blk;
+	struct xfs_inode	*dp;
+	struct xfs_buf		*bp;
+	int			retval, error, forkoff;
 
 	trace_xfs_attr_node_removename(args);
 

@@ -1130,12 +1133,12 @@ xfs_attr_node_removename(xfs_da_args_t *args)
 	 * Check to see if the tree needs to be collapsed.
 	 */
 	if (retval && (state->path.active > 1)) {
-		xfs_defer_init(args->dfops, args->firstblock);
+		xfs_defer_init(args->trans->t_dfops, args->firstblock);
 		error = xfs_da3_join(state);
 		if (error)
 			goto out_defer_cancel;
-		xfs_defer_ijoin(args->dfops, dp);
-		error = xfs_defer_finish(&args->trans, args->dfops);
+		xfs_defer_ijoin(args->trans->t_dfops, dp);
+		error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
 		if (error)
 			goto out_defer_cancel;
 		/*

@@ -1162,13 +1165,13 @@ xfs_attr_node_removename(xfs_da_args_t *args)
 			goto out;
 
 		if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) {
-			xfs_defer_init(args->dfops, args->firstblock);
+			xfs_defer_init(args->trans->t_dfops, args->firstblock);
 			error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
 			/* bp is gone due to xfs_da_shrink_inode */
 			if (error)
 				goto out_defer_cancel;
-			xfs_defer_ijoin(args->dfops, dp);
-			error = xfs_defer_finish(&args->trans, args->dfops);
+			xfs_defer_ijoin(args->trans->t_dfops, dp);
+			error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
 			if (error)
 				goto out_defer_cancel;
 		} else

@@ -1180,7 +1183,7 @@ xfs_attr_node_removename(xfs_da_args_t *args)
 	xfs_da_state_free(state);
 	return error;
 out_defer_cancel:
-	xfs_defer_cancel(args->dfops);
+	xfs_defer_cancel(args->trans->t_dfops);
 	goto out;
 }
 

fs/xfs/libxfs/xfs_attr_leaf.c +11 -13

index 76e90046731c..c131469db0f1 100644
@@ -747,18 +747,18 @@ xfs_attr_shortform_getvalue(xfs_da_args_t *args)
  */
 int
 xfs_attr_shortform_to_leaf(
-	struct xfs_da_args	*args,
-	struct xfs_buf		**leaf_bp)
+	struct xfs_da_args		*args,
+	struct xfs_buf			**leaf_bp)
 {
-	xfs_inode_t *dp;
-	xfs_attr_shortform_t *sf;
-	xfs_attr_sf_entry_t *sfe;
-	xfs_da_args_t nargs;
-	char *tmpbuffer;
-	int error, i, size;
-	xfs_dablk_t blkno;
-	struct xfs_buf *bp;
-	xfs_ifork_t *ifp;
+	struct xfs_inode		*dp;
+	struct xfs_attr_shortform	*sf;
+	struct xfs_attr_sf_entry	*sfe;
+	struct xfs_da_args		nargs;
+	char				*tmpbuffer;
+	int				error, i, size;
+	xfs_dablk_t			blkno;
+	struct xfs_buf			*bp;
+	struct xfs_ifork		*ifp;
 
 	trace_xfs_attr_sf_to_leaf(args);
 

@@ -803,7 +803,6 @@ xfs_attr_shortform_to_leaf(
 	nargs.dp = dp;
 	nargs.geo = args->geo;
 	nargs.firstblock = args->firstblock;
-	nargs.dfops = args->dfops;
 	nargs.total = args->total;
 	nargs.whichfork = XFS_ATTR_FORK;
 	nargs.trans = args->trans;

@@ -1007,7 +1006,6 @@ xfs_attr3_leaf_to_shortform(
 	nargs.geo = args->geo;
 	nargs.dp = dp;
 	nargs.firstblock = args->firstblock;
-	nargs.dfops = args->dfops;
 	nargs.total = args->total;
 	nargs.whichfork = XFS_ATTR_FORK;
 	nargs.trans = args->trans;

fs/xfs/libxfs/xfs_attr_remote.c +12 -11

index bf2e0371149b..577c4c372265 100644
@@ -480,15 +480,16 @@ xfs_attr_rmtval_set(
 		 * extent and then crash then the block may not contain the
 		 * correct metadata after log recovery occurs.
 		 */
-		xfs_defer_init(args->dfops, args->firstblock);
+		xfs_defer_init(args->trans->t_dfops, args->firstblock);
 		nmap = 1;
 		error = xfs_bmapi_write(args->trans, dp, (xfs_fileoff_t)lblkno,
 				  blkcnt, XFS_BMAPI_ATTRFORK, args->firstblock,
-				  args->total, &map, &nmap, args->dfops);
+				  args->total, &map, &nmap,
+				  args->trans->t_dfops);
 		if (error)
 			goto out_defer_cancel;
-		xfs_defer_ijoin(args->dfops, dp);
-		error = xfs_defer_finish(&args->trans, args->dfops);
+		xfs_defer_ijoin(args->trans->t_dfops, dp);
+		error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
 		if (error)
 			goto out_defer_cancel;
 

@@ -522,7 +523,7 @@ xfs_attr_rmtval_set(
 
 		ASSERT(blkcnt > 0);
 
-		xfs_defer_init(args->dfops, args->firstblock);
+		xfs_defer_init(args->trans->t_dfops, args->firstblock);
 		nmap = 1;
 		error = xfs_bmapi_read(dp, (xfs_fileoff_t)lblkno,
 				       blkcnt, &map, &nmap,

@@ -557,7 +558,7 @@ xfs_attr_rmtval_set(
 	ASSERT(valuelen == 0);
 	return 0;
 out_defer_cancel:
-	xfs_defer_cancel(args->dfops);
+	xfs_defer_cancel(args->trans->t_dfops);
 	args->trans = NULL;
 	return error;
 }

@@ -626,14 +627,14 @@ xfs_attr_rmtval_remove(
 	blkcnt = args->rmtblkcnt;
 	done = 0;
 	while (!done) {
-		xfs_defer_init(args->dfops, args->firstblock);
+		xfs_defer_init(args->trans->t_dfops, args->firstblock);
 		error = xfs_bunmapi(args->trans, args->dp, lblkno, blkcnt,
 				    XFS_BMAPI_ATTRFORK, 1, args->firstblock,
-				    args->dfops, &done);
+				    args->trans->t_dfops, &done);
 		if (error)
 			goto out_defer_cancel;
-		xfs_defer_ijoin(args->dfops, args->dp);
-		error = xfs_defer_finish(&args->trans, args->dfops);
+		xfs_defer_ijoin(args->trans->t_dfops, args->dp);
+		error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
 		if (error)
 			goto out_defer_cancel;
 

@@ -646,7 +647,7 @@ xfs_attr_rmtval_remove(
 	}
 	return 0;
 out_defer_cancel:
-	xfs_defer_cancel(args->dfops);
+	xfs_defer_cancel(args->trans->t_dfops);
 	args->trans = NULL;
 	return error;
 }

fs/xfs/libxfs/xfs_bmap.c +0 -1

index dbd07b0adfa3..93dbcf678e54 100644
@@ -1007,7 +1007,6 @@ xfs_bmap_add_attrfork_local(
 		dargs.geo = ip->i_mount->m_dir_geo;
 		dargs.dp = ip;
 		dargs.firstblock = firstblock;
-		dargs.dfops = tp->t_dfops;
 		dargs.total = dargs.geo->fsbcount;
 		dargs.whichfork = XFS_DATA_FORK;
 		dargs.trans = tp;

fs/xfs/libxfs/xfs_da_btree.c +10 -9

index 8a301402bbc4..d2048e4e93ec 100644
@@ -2063,7 +2063,7 @@ xfs_da_grow_inode_int(
 	error = xfs_bmapi_write(tp, dp, *bno, count,
 			xfs_bmapi_aflag(w)|XFS_BMAPI_METADATA|XFS_BMAPI_CONTIG,
 			args->firstblock, args->total, &map, &nmap,
-			args->dfops);
+			args->trans->t_dfops);
 	if (error)
 		return error;
 

@@ -2086,7 +2086,8 @@ xfs_da_grow_inode_int(
 			error = xfs_bmapi_write(tp, dp, b, c,
 					xfs_bmapi_aflag(w)|XFS_BMAPI_METADATA,
 					args->firstblock, args->total,
-					&mapp[mapi], &nmap, args->dfops);
+					&mapp[mapi], &nmap,
+					args->trans->t_dfops);
 			if (error)
 				goto out_free_map;
 			if (nmap < 1)

@@ -2375,13 +2376,13 @@ xfs_da3_swap_lastblock(
  */
 int
 xfs_da_shrink_inode(
-	xfs_da_args_t	*args,
-	xfs_dablk_t	dead_blkno,
-	struct xfs_buf	*dead_buf)
+	struct xfs_da_args	*args,
+	xfs_dablk_t		dead_blkno,
+	struct xfs_buf		*dead_buf)
 {
-	xfs_inode_t *dp;
-	int done, error, w, count;
-	xfs_trans_t *tp;
+	struct xfs_inode	*dp;
+	int			done, error, w, count;
+	struct xfs_trans	*tp;
 
 	trace_xfs_da_shrink_inode(args);
 

@@ -2396,7 +2397,7 @@ xfs_da_shrink_inode(
 		 */
 		error = xfs_bunmapi(tp, dp, dead_blkno, count,
 				    xfs_bmapi_aflag(w), 0, args->firstblock,
-				    args->dfops, &done);
+				    args->trans->t_dfops, &done);
 		if (error == -ENOSPC) {
 			if (w != XFS_DATA_FORK)
 				break;

fs/xfs/libxfs/xfs_da_btree.h +0 -1

index 28260073ae71..6b8a04f3f162 100644
@@ -58,7 +58,6 @@ typedef struct xfs_da_args {
 	xfs_ino_t	inumber;	/* input/output inode number */
 	struct xfs_inode *dp;		/* directory inode to manipulate */
 	xfs_fsblock_t	*firstblock;	/* ptr to firstblock for bmap calls */
-	struct xfs_defer_ops *dfops;	/* ptr to freelist for bmap_finish */
 	struct xfs_trans *trans;	/* current trans (changes over time) */
 	xfs_extlen_t	total;		/* total blocks needed, for 1st bmap */
 	int		whichfork;	/* data or attribute fork */

fs/xfs/libxfs/xfs_dir2.c +31 -34

index ceedc3d31114..ffa288c37da0 100644
@@ -239,8 +239,8 @@ xfs_dir_init(
  */
 int
 xfs_dir_createname(
-	xfs_trans_t		*tp,
-	xfs_inode_t		*dp,
+	struct xfs_trans	*tp,
+	struct xfs_inode	*dp,
 	struct xfs_name		*name,
 	xfs_ino_t		inum,		/* new entry inode number */
 	xfs_fsblock_t		*first,		/* bmap's firstblock */

@@ -273,7 +273,6 @@ xfs_dir_createname(
 	args->whichfork = XFS_DATA_FORK;
 	args->trans = tp;
 	ASSERT(tp->t_dfops || !first);
-	args->dfops = tp->t_dfops;
 	args->firstblock = first;
 	args->op_flags = XFS_DA_OP_ADDNAME | XFS_DA_OP_OKNOENT;
 	if (!inum)

@@ -416,16 +415,16 @@ xfs_dir_lookup(
  */
 int
 xfs_dir_removename(
-	xfs_trans_t	*tp,
-	xfs_inode_t	*dp,
-	struct xfs_name	*name,
-	xfs_ino_t	ino,
-	xfs_fsblock_t	*first,		/* bmap's firstblock */
-	xfs_extlen_t	total)		/* bmap's total block count */
+	struct xfs_trans	*tp,
+	struct xfs_inode	*dp,
+	struct xfs_name		*name,
+	xfs_ino_t		ino,
+	xfs_fsblock_t		*first,		/* bmap's firstblock */
+	xfs_extlen_t		total)		/* bmap's total block count */
 {
-	struct xfs_da_args *args;
-	int		rval;
-	int		v;		/* type-checking value */
+	struct xfs_da_args	*args;
+	int			rval;
+	int			v;		/* type-checking value */
 
 	ASSERT(S_ISDIR(VFS_I(dp)->i_mode));
 	XFS_STATS_INC(dp->i_mount, xs_dir_remove);

@@ -446,7 +445,6 @@ xfs_dir_removename(
 	args->whichfork = XFS_DATA_FORK;
 	args->trans = tp;
 	ASSERT(tp->t_dfops);
-	args->dfops = tp->t_dfops;
 
 	if (dp->i_d.di_format == XFS_DINODE_FMT_LOCAL) {
 		rval = xfs_dir2_sf_removename(args);

@@ -478,16 +476,16 @@ xfs_dir_removename(
  */
 int
 xfs_dir_replace(
-	xfs_trans_t	*tp,
-	xfs_inode_t	*dp,
-	struct xfs_name	*name,		/* name of entry to replace */
-	xfs_ino_t	inum,		/* new inode number */
-	xfs_fsblock_t	*first,		/* bmap's firstblock */
-	xfs_extlen_t	total)		/* bmap's total block count */
+	struct xfs_trans	*tp,
+	struct xfs_inode	*dp,
+	struct xfs_name		*name,		/* name of entry to replace */
+	xfs_ino_t		inum,		/* new inode number */
+	xfs_fsblock_t		*first,		/* bmap's firstblock */
+	xfs_extlen_t		total)		/* bmap's total block count */
 {
-	struct xfs_da_args *args;
-	int		rval;
-	int		v;		/* type-checking value */
+	struct xfs_da_args	*args;
+	int			rval;
+	int			v;		/* type-checking value */
 
 	ASSERT(S_ISDIR(VFS_I(dp)->i_mode));
 

@@ -511,7 +509,6 @@ xfs_dir_replace(
 	args->whichfork = XFS_DATA_FORK;
 	args->trans = tp;
 	ASSERT(tp->t_dfops);
-	args->dfops = tp->t_dfops;
 
 	if (dp->i_d.di_format == XFS_DINODE_FMT_LOCAL) {
 		rval = xfs_dir2_sf_replace(args);

@@ -645,17 +642,17 @@ xfs_dir2_isleaf(
  */
 int
 xfs_dir2_shrink_inode(
-	xfs_da_args_t	*args,
-	xfs_dir2_db_t	db,
-	struct xfs_buf	*bp)
+	struct xfs_da_args	*args,
+	xfs_dir2_db_t		db,
+	struct xfs_buf		*bp)
 {
-	xfs_fileoff_t	bno;		/* directory file offset */
-	xfs_dablk_t	da;		/* directory file offset */
-	int		done;		/* bunmap is finished */
-	xfs_inode_t	*dp;
-	int		error;
-	xfs_mount_t	*mp;
-	xfs_trans_t	*tp;
+	xfs_fileoff_t		bno;		/* directory file offset */
+	xfs_dablk_t		da;		/* directory file offset */
+	int			done;		/* bunmap is finished */
+	struct xfs_inode	*dp;
+	int			error;
+	struct xfs_mount	*mp;
+	struct xfs_trans	*tp;
 
 	trace_xfs_dir2_shrink_inode(args, db);
 

@@ -666,7 +663,7 @@ xfs_dir2_shrink_inode(
 
 	/* Unmap the fsblock(s). */
 	error = xfs_bunmapi(tp, dp, da, args->geo->fsbcount, 0, 0,
-			    args->firstblock, args->dfops, &done);
+			    args->firstblock, args->trans->t_dfops, &done);
 	if (error) {
 		/*
 		 * ENOSPC actually can happen if we're in a removename with no

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

xfs_dquot_disk_alloc() receives a transaction from the caller and passes a local dfops along to xfs_bmapi_write(). If we attach this dfops to the transaction, we have to make sure to clear it before returning to avoid invalid access of stack memory.

Since xfs_qm_dqread_alloc() is the only caller, pull dfops into the caller and attach it to the transaction to eliminate this pattern entirely.

Signed-off-by Brian Foster <[email protected]>

xfs_dquot_disk_alloc() receives a transaction from the caller and passes a local dfops along to xfs_bmapi_write(). If we attach this dfops to the transaction, we have to make sure to clear it before returning to avoid invalid access of stack memory.

Since xfs_qm_dqread_alloc() is the only caller, pull dfops into the caller and attach it to the transaction to eliminate this pattern entirely.

Signed-off-by Brian Foster <[email protected]>

---
 fs/xfs/xfs_dquot.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

fs/xfs/xfs_dquot.c +2 -2

index 0973a0423bed..aa62f8b17376 100644
@@ -286,8 +286,8 @@ xfs_dquot_disk_alloc(
 	struct xfs_buf		**bpp)
 {
 	struct xfs_bmbt_irec	map;
-	struct xfs_defer_ops	dfops;
-	struct xfs_mount	*mp = (*tpp)->t_mountp;
+	struct xfs_trans	*tp = *tpp;
+	struct xfs_mount	*mp = tp->t_mountp;
 	struct xfs_buf		*bp;
 	struct xfs_inode	*quotip = xfs_quota_inode(mp, dqp->dq_flags);
 	xfs_fsblock_t		firstblock;

fs/xfs/xfs_dquot.c +14 -11

+
 	xfs_ilock(quotip, XFS_ILOCK_EXCL);
 	if (!xfs_this_quota_on(dqp->q_mount, dqp->dq_flags)) {
 		/*

@@ -308,11 +309,11 @@ xfs_dquot_disk_alloc(
 	}
 
 	/* Create the block mapping. */
-	xfs_trans_ijoin(*tpp, quotip, XFS_ILOCK_EXCL);
-	error = xfs_bmapi_write(*tpp, quotip, dqp->q_fileoffset,
+	xfs_trans_ijoin(tp, quotip, XFS_ILOCK_EXCL);
+	error = xfs_bmapi_write(tp, quotip, dqp->q_fileoffset,
 			XFS_DQUOT_CLUSTER_SIZE_FSB, XFS_BMAPI_METADATA,
 			&firstblock, XFS_QM_DQALLOC_SPACE_RES(mp),
-			&map, &nmaps, &dfops);
+			&map, &nmaps, tp->t_dfops);
 	if (error)
 		goto error0;
 	ASSERT(map.br_blockcount == XFS_DQUOT_CLUSTER_SIZE_FSB);

@@ -326,7 +327,7 @@ xfs_dquot_disk_alloc(
 	dqp->q_blkno = XFS_FSB_TO_DADDR(mp, map.br_startblock);
 
 	/* now we can just get the buffer (there's nothing to read yet) */
-	bp = xfs_trans_get_buf(*tpp, mp->m_ddev_targp, dqp->q_blkno,
+	bp = xfs_trans_get_buf(tp, mp->m_ddev_targp, dqp->q_blkno,
 			mp->m_quotainfo->qi_dqchunklen, 0);
 	if (!bp) {
 		error = -ENOMEM;

@@ -338,7 +339,7 @@ xfs_dquot_disk_alloc(
 	 * Make a chunk of dquots out of this buffer and log
 	 * the entire thing.
 	 */
-	xfs_qm_init_dquot_blk(*tpp, mp, be32_to_cpu(dqp->q_core.d_id),
+	xfs_qm_init_dquot_blk(tp, mp, be32_to_cpu(dqp->q_core.d_id),
 			      dqp->dq_flags & XFS_DQ_ALLTYPES, bp);
 	xfs_buf_set_ref(bp, XFS_DQUOT_REF);
 

@@ -364,14 +365,15 @@ xfs_dquot_disk_alloc(
 	 * is responsible for unlocking any buffer passed back, either
 	 * manually or by committing the transaction.
 	 */
-	xfs_trans_bhold(*tpp, bp);
-	error = xfs_defer_bjoin(&dfops, bp);
+	xfs_trans_bhold(tp, bp);
+	error = xfs_defer_bjoin(tp->t_dfops, bp);
 	if (error) {
-		xfs_trans_bhold_release(*tpp, bp);
-		xfs_trans_brelse(*tpp, bp);
+		xfs_trans_bhold_release(tp, bp);
+		xfs_trans_brelse(tp, bp);
 		goto error1;
 	}
-	error = xfs_defer_finish(tpp, &dfops);
+	error = xfs_defer_finish(tpp, tp->t_dfops);
+	tp = *tpp;
 	if (error) {
 		xfs_buf_relse(bp);
 		goto error1;

@@ -380,7 +382,7 @@ xfs_dquot_disk_alloc(
 	return 0;
 
 error1:
-	xfs_defer_cancel(&dfops);
+	xfs_defer_cancel(tp->t_dfops);
 error0:
 	return error;
 }

@@ -538,13 +540,17 @@ xfs_qm_dqread_alloc(
 	struct xfs_buf		**bpp)
 {
 	struct xfs_trans	*tp;
+	struct xfs_defer_ops	dfops;

fs/xfs/xfs_dquot.c +0 -0

 	error = xfs_dquot_disk_alloc(&tp, dqp, &bp);
 	if (error)

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

Quoted 30 lines · Brian Foster

xfs_dquot_disk_alloc() receives a transaction from the caller and passes a local dfops along to xfs_bmapi_write(). If we attach this dfops to the transaction, we have to make sure to clear it before returning to avoid invalid access of stack memory.

Since xfs_qm_dqread_alloc() is the only caller, pull dfops into the caller and attach it to the transaction to eliminate this pattern entirely.

Signed-off-by Brian Foster <[email protected]>

---
 fs/xfs/xfs_dquot.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

fs/xfs/xfs_dquot.c +2 -2

index 0973a0423bed..aa62f8b17376 100644
@@ -286,8 +286,8 @@ xfs_dquot_disk_alloc(
 	struct xfs_buf		**bpp)
 {
 	struct xfs_bmbt_irec	map;
-	struct xfs_defer_ops	dfops;
-	struct xfs_mount	*mp = (*tpp)->t_mountp;
+	struct xfs_trans	*tp = *tpp;
+	struct xfs_mount	*mp = tp->t_mountp;
 	struct xfs_buf		*bp;
 	struct xfs_inode	*quotip = xfs_quota_inode(mp, dqp->dq_flags);
 	xfs_fsblock_t		firstblock;

fs/xfs/xfs_dquot.c xfs_dquot_disk_alloc( · line 299

Quoted 5 lines

fs/xfs/xfs_dquot.c +1 -1

@@ -296,7 +296,8 @@ xfs_dquot_disk_alloc(
 
 	trace_xfs_dqalloc(dqp);
 
-	xfs_defer_init(&dfops, &firstblock);
+	xfs_defer_init(tp->t_dfops, &firstblock);

Initializing a pointed-to dfops is a little eyebrow-raising because...

Quoted 76 lines

fs/xfs/xfs_dquot.c +15 -11

+
 	xfs_ilock(quotip, XFS_ILOCK_EXCL);
 	if (!xfs_this_quota_on(dqp->q_mount, dqp->dq_flags)) {
 		/*

@@ -308,11 +309,11 @@ xfs_dquot_disk_alloc(
 	}
 
 	/* Create the block mapping. */
-	xfs_trans_ijoin(*tpp, quotip, XFS_ILOCK_EXCL);
-	error = xfs_bmapi_write(*tpp, quotip, dqp->q_fileoffset,
+	xfs_trans_ijoin(tp, quotip, XFS_ILOCK_EXCL);
+	error = xfs_bmapi_write(tp, quotip, dqp->q_fileoffset,
 			XFS_DQUOT_CLUSTER_SIZE_FSB, XFS_BMAPI_METADATA,
 			&firstblock, XFS_QM_DQALLOC_SPACE_RES(mp),
-			&map, &nmaps, &dfops);
+			&map, &nmaps, tp->t_dfops);
 	if (error)
 		goto error0;
 	ASSERT(map.br_blockcount == XFS_DQUOT_CLUSTER_SIZE_FSB);

@@ -326,7 +327,7 @@ xfs_dquot_disk_alloc(
 	dqp->q_blkno = XFS_FSB_TO_DADDR(mp, map.br_startblock);
 
 	/* now we can just get the buffer (there's nothing to read yet) */
-	bp = xfs_trans_get_buf(*tpp, mp->m_ddev_targp, dqp->q_blkno,
+	bp = xfs_trans_get_buf(tp, mp->m_ddev_targp, dqp->q_blkno,
 			mp->m_quotainfo->qi_dqchunklen, 0);
 	if (!bp) {
 		error = -ENOMEM;

@@ -338,7 +339,7 @@ xfs_dquot_disk_alloc(
 	 * Make a chunk of dquots out of this buffer and log
 	 * the entire thing.
 	 */
-	xfs_qm_init_dquot_blk(*tpp, mp, be32_to_cpu(dqp->q_core.d_id),
+	xfs_qm_init_dquot_blk(tp, mp, be32_to_cpu(dqp->q_core.d_id),
 			      dqp->dq_flags & XFS_DQ_ALLTYPES, bp);
 	xfs_buf_set_ref(bp, XFS_DQUOT_REF);
 

@@ -364,14 +365,15 @@ xfs_dquot_disk_alloc(
 	 * is responsible for unlocking any buffer passed back, either
 	 * manually or by committing the transaction.
 	 */
-	xfs_trans_bhold(*tpp, bp);
-	error = xfs_defer_bjoin(&dfops, bp);
+	xfs_trans_bhold(tp, bp);
+	error = xfs_defer_bjoin(tp->t_dfops, bp);
 	if (error) {
-		xfs_trans_bhold_release(*tpp, bp);
-		xfs_trans_brelse(*tpp, bp);
+		xfs_trans_bhold_release(tp, bp);
+		xfs_trans_brelse(tp, bp);
 		goto error1;
 	}
-	error = xfs_defer_finish(tpp, &dfops);
+	error = xfs_defer_finish(tpp, tp->t_dfops);
+	tp = *tpp;
 	if (error) {
 		xfs_buf_relse(bp);
 		goto error1;

@@ -380,7 +382,7 @@ xfs_dquot_disk_alloc(
 	return 0;
 
 error1:
-	xfs_defer_cancel(&dfops);
+	xfs_defer_cancel(tp->t_dfops);
 error0:
 	return error;
 }

@@ -538,13 +540,17 @@ xfs_qm_dqread_alloc(
 	struct xfs_buf		**bpp)
 {
 	struct xfs_trans	*tp;
+	struct xfs_defer_ops	dfops;
 	struct xfs_buf		*bp;
+	xfs_fsblock_t		firstblock;
 	int			error;