2018-08-07 14:15:27

by Colin King

[permalink] [raw]
Subject: re: xfs: cancel dfops on xfs_defer_finish() error

Hi,

Recent commit 82ff27bc52a88cb5cc400bfa64e210d3ec8dfebd ("xfs: automatic
dfops buffer relogging") removed the assignment of variable error:

- error = xfs_defer_bjoin(tp->t_dfops, bp);
if (error) {
xfs_trans_bhold_release(tp, bp);
xfs_trans_brelse(tp, bp);


.. the removal of the assignment leads to dead code on the following if
statement as error is always zero at this point. Not sure if that was
intended, but it tripped a static analysis warning from CoverityScan.


364 xfs_trans_bhold(tp, bp);

const: At condition error, the value of error must be equal to 0.
dead_error_condition: The condition error cannot be true.

365 if (error) {
CID 1472288 (#1 of 1): Logically dead code (DEADCODE)
dead_error_begin: Execution cannot reach this statement:
xfs_trans_bhold_release(tp,....


366 xfs_trans_bhold_release(tp, bp);
367 xfs_trans_brelse(tp, bp);
368 goto error1;
369 }

Colin


2018-08-07 14:38:52

by Brian Foster

[permalink] [raw]
Subject: Re: xfs: cancel dfops on xfs_defer_finish() error

On Tue, Aug 07, 2018 at 03:14:07PM +0100, Colin Ian King wrote:
> Hi,
>
> Recent commit 82ff27bc52a88cb5cc400bfa64e210d3ec8dfebd ("xfs: automatic
> dfops buffer relogging") removed the assignment of variable error:
>
> - error = xfs_defer_bjoin(tp->t_dfops, bp);
> if (error) {
> xfs_trans_bhold_release(tp, bp);
> xfs_trans_brelse(tp, bp);
>

Hmm, I _think_ we can just drop these error checks now that this
pre-finish error state is non-existent, something like the appended diff
(with additional cleanups). E.g., if the buffer is held in the
transaction then the bjoin is implicit. If the finish fails, then the
state is essentially unchanged by the relogging patch.

That said, the error handling is a bit tricky here. Darrick, I think you
reworked this recently.. thoughts?

Brian

--- 8< ---

diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 70a76ac41f01..2106c4142ecd 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -311,7 +311,7 @@ xfs_dquot_disk_alloc(
XFS_DQUOT_CLUSTER_SIZE_FSB, XFS_BMAPI_METADATA,
XFS_QM_DQALLOC_SPACE_RES(mp), &map, &nmaps);
if (error)
- goto error0;
+ return error;
ASSERT(map.br_blockcount == XFS_DQUOT_CLUSTER_SIZE_FSB);
ASSERT(nmaps == 1);
ASSERT((map.br_startblock != DELAYSTARTBLOCK) &&
@@ -326,8 +326,8 @@ xfs_dquot_disk_alloc(
bp = xfs_trans_get_buf(tp, mp->m_ddev_targp, dqp->q_blkno,
mp->m_quotainfo->qi_dqchunklen, 0);
if (!bp) {
- error = -ENOMEM;
- goto error1;
+ xfs_defer_cancel(tp);
+ return -ENOMEM;
}
bp->b_ops = &xfs_dquot_buf_ops;

@@ -349,10 +349,8 @@ xfs_dquot_disk_alloc(
* the buffer locked across the _defer_finish call. We can now do
* this correctly with xfs_defer_bjoin.
*
- * Above, we allocated a disk block for the dquot information and
- * used get_buf to initialize the dquot. If the _defer_bjoin fails,
- * the buffer is still locked to *tpp, so we must _bhold_release and
- * then _trans_brelse the buffer. If the _defer_finish fails, the old
+ * Above, we allocated a disk block for the dquot information and used
+ * get_buf to initialize the dquot. If the _defer_finish fails, the old
* transaction is gone but the new buffer is not joined or held to any
* transaction, so we must _buf_relse it.
*
@@ -362,24 +360,14 @@ xfs_dquot_disk_alloc(
* manually or by committing the transaction.
*/
xfs_trans_bhold(tp, bp);
- if (error) {
- xfs_trans_bhold_release(tp, bp);
- xfs_trans_brelse(tp, bp);
- goto error1;
- }
error = xfs_defer_finish(tpp);
tp = *tpp;
if (error) {
xfs_buf_relse(bp);
- goto error0;
+ return error;
}
*bpp = bp;
return 0;
-
-error1:
- xfs_defer_cancel(tp);
-error0:
- return error;
}

/*

2018-08-07 15:33:04

by Darrick J. Wong

[permalink] [raw]
Subject: Re: xfs: cancel dfops on xfs_defer_finish() error

On Tue, Aug 07, 2018 at 11:18:36AM -0400, Brian Foster wrote:
> On Tue, Aug 07, 2018 at 08:10:29AM -0700, Darrick J. Wong wrote:
> > On Tue, Aug 07, 2018 at 10:37:21AM -0400, Brian Foster wrote:
> > > On Tue, Aug 07, 2018 at 03:14:07PM +0100, Colin Ian King wrote:
> > > > Hi,
> > > >
> > > > Recent commit 82ff27bc52a88cb5cc400bfa64e210d3ec8dfebd ("xfs: automatic
> > > > dfops buffer relogging") removed the assignment of variable error:
> > > >
> > > > - error = xfs_defer_bjoin(tp->t_dfops, bp);
> > > > if (error) {
> > > > xfs_trans_bhold_release(tp, bp);
> > > > xfs_trans_brelse(tp, bp);
> > > >
> > >
> > > Hmm, I _think_ we can just drop these error checks now that this
> > > pre-finish error state is non-existent, something like the appended diff
> > > (with additional cleanups). E.g., if the buffer is held in the
> > > transaction then the bjoin is implicit. If the finish fails, then the
> > > state is essentially unchanged by the relogging patch.
> > >
> > > That said, the error handling is a bit tricky here. Darrick, I think you
> > > reworked this recently.. thoughts?
> > >
> > > Brian
> > >
> > > --- 8< ---
> > >
> > > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> > > index 70a76ac41f01..2106c4142ecd 100644
> > > --- a/fs/xfs/xfs_dquot.c
> > > +++ b/fs/xfs/xfs_dquot.c
> > > @@ -311,7 +311,7 @@ xfs_dquot_disk_alloc(
> > > XFS_DQUOT_CLUSTER_SIZE_FSB, XFS_BMAPI_METADATA,
> > > XFS_QM_DQALLOC_SPACE_RES(mp), &map, &nmaps);
> > > if (error)
> > > - goto error0;
> > > + return error;
> > > ASSERT(map.br_blockcount == XFS_DQUOT_CLUSTER_SIZE_FSB);
> > > ASSERT(nmaps == 1);
> > > ASSERT((map.br_startblock != DELAYSTARTBLOCK) &&
> > > @@ -326,8 +326,8 @@ xfs_dquot_disk_alloc(
> > > bp = xfs_trans_get_buf(tp, mp->m_ddev_targp, dqp->q_blkno,
> > > mp->m_quotainfo->qi_dqchunklen, 0);
> > > if (!bp) {
> > > - error = -ENOMEM;
> > > - goto error1;
> > > + xfs_defer_cancel(tp);
> > > + return -ENOMEM;
> >
> > The only caller of xfs_dquot_disk_alloc checks the return value and
> > xfs_trans_cancels the transaction, which should take care of calling
> > xfs_defer_cancel, right?
> >
>
> Yeah. IIRC I originally left the defer_cancel() alone in this function
> just to be consistent, since it calls xfs_defer_finish() as well on the
> caller's transaction. Technically I don't think it matters either way,
> so I don't have much preference on if it stays or goes (assuming the
> other changes are correct)...

I can't think of a good reason for anything (except xfs_trans_cancel) to
call xfs_defer_cancel. It's never correct to kill all the deferred ops
attached to a transaction and then continue logging things to the
transaction, which means that the only thing you can really do after
cancelling the dfops is to cancel the transaction. I'd have to look
more carefully but I think all the remaining callers can all go away.

xfs_defer_finish is a different story -- it can be used (by online
repair for example) to process the deferred operations so they don't sit
around pinning memory and log, even if there's a lot more work to be
done before _trans_commit. But I don't think we need to _defer_cancel
if things go awry.

--D

> Brian
>
> > --D
> >
> > > }
> > > bp->b_ops = &xfs_dquot_buf_ops;
> > >
> > > @@ -349,10 +349,8 @@ xfs_dquot_disk_alloc(
> > > * the buffer locked across the _defer_finish call. We can now do
> > > * this correctly with xfs_defer_bjoin.
> > > *
> > > - * Above, we allocated a disk block for the dquot information and
> > > - * used get_buf to initialize the dquot. If the _defer_bjoin fails,
> > > - * the buffer is still locked to *tpp, so we must _bhold_release and
> > > - * then _trans_brelse the buffer. If the _defer_finish fails, the old
> > > + * Above, we allocated a disk block for the dquot information and used
> > > + * get_buf to initialize the dquot. If the _defer_finish fails, the old
> > > * transaction is gone but the new buffer is not joined or held to any
> > > * transaction, so we must _buf_relse it.
> > > *
> > > @@ -362,24 +360,14 @@ xfs_dquot_disk_alloc(
> > > * manually or by committing the transaction.
> > > */
> > > xfs_trans_bhold(tp, bp);
> > > - if (error) {
> > > - xfs_trans_bhold_release(tp, bp);
> > > - xfs_trans_brelse(tp, bp);
> > > - goto error1;
> > > - }
> > > error = xfs_defer_finish(tpp);
> > > tp = *tpp;
> > > if (error) {
> > > xfs_buf_relse(bp);
> > > - goto error0;
> > > + return error;
> > > }
> > > *bpp = bp;
> > > return 0;
> > > -
> > > -error1:
> > > - xfs_defer_cancel(tp);
> > > -error0:
> > > - return error;
> > > }
> > >
> > > /*
> > > --
> > > 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

2018-08-07 15:38:23

by Brian Foster

[permalink] [raw]
Subject: Re: xfs: cancel dfops on xfs_defer_finish() error

On Tue, Aug 07, 2018 at 08:26:44AM -0700, Darrick J. Wong wrote:
> On Tue, Aug 07, 2018 at 11:18:36AM -0400, Brian Foster wrote:
> > On Tue, Aug 07, 2018 at 08:10:29AM -0700, Darrick J. Wong wrote:
> > > On Tue, Aug 07, 2018 at 10:37:21AM -0400, Brian Foster wrote:
> > > > On Tue, Aug 07, 2018 at 03:14:07PM +0100, Colin Ian King wrote:
> > > > > Hi,
> > > > >
> > > > > Recent commit 82ff27bc52a88cb5cc400bfa64e210d3ec8dfebd ("xfs: automatic
> > > > > dfops buffer relogging") removed the assignment of variable error:
> > > > >
> > > > > - error = xfs_defer_bjoin(tp->t_dfops, bp);
> > > > > if (error) {
> > > > > xfs_trans_bhold_release(tp, bp);
> > > > > xfs_trans_brelse(tp, bp);
> > > > >
> > > >
> > > > Hmm, I _think_ we can just drop these error checks now that this
> > > > pre-finish error state is non-existent, something like the appended diff
> > > > (with additional cleanups). E.g., if the buffer is held in the
> > > > transaction then the bjoin is implicit. If the finish fails, then the
> > > > state is essentially unchanged by the relogging patch.
> > > >
> > > > That said, the error handling is a bit tricky here. Darrick, I think you
> > > > reworked this recently.. thoughts?
> > > >
> > > > Brian
> > > >
> > > > --- 8< ---
> > > >
> > > > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> > > > index 70a76ac41f01..2106c4142ecd 100644
> > > > --- a/fs/xfs/xfs_dquot.c
> > > > +++ b/fs/xfs/xfs_dquot.c
> > > > @@ -311,7 +311,7 @@ xfs_dquot_disk_alloc(
> > > > XFS_DQUOT_CLUSTER_SIZE_FSB, XFS_BMAPI_METADATA,
> > > > XFS_QM_DQALLOC_SPACE_RES(mp), &map, &nmaps);
> > > > if (error)
> > > > - goto error0;
> > > > + return error;
> > > > ASSERT(map.br_blockcount == XFS_DQUOT_CLUSTER_SIZE_FSB);
> > > > ASSERT(nmaps == 1);
> > > > ASSERT((map.br_startblock != DELAYSTARTBLOCK) &&
> > > > @@ -326,8 +326,8 @@ xfs_dquot_disk_alloc(
> > > > bp = xfs_trans_get_buf(tp, mp->m_ddev_targp, dqp->q_blkno,
> > > > mp->m_quotainfo->qi_dqchunklen, 0);
> > > > if (!bp) {
> > > > - error = -ENOMEM;
> > > > - goto error1;
> > > > + xfs_defer_cancel(tp);
> > > > + return -ENOMEM;
> > >
> > > The only caller of xfs_dquot_disk_alloc checks the return value and
> > > xfs_trans_cancels the transaction, which should take care of calling
> > > xfs_defer_cancel, right?
> > >
> >
> > Yeah. IIRC I originally left the defer_cancel() alone in this function
> > just to be consistent, since it calls xfs_defer_finish() as well on the
> > caller's transaction. Technically I don't think it matters either way,
> > so I don't have much preference on if it stays or goes (assuming the
> > other changes are correct)...
>
> I can't think of a good reason for anything (except xfs_trans_cancel) to
> call xfs_defer_cancel. It's never correct to kill all the deferred ops
> attached to a transaction and then continue logging things to the
> transaction, which means that the only thing you can really do after
> cancelling the dfops is to cancel the transaction. I'd have to look
> more carefully but I think all the remaining callers can all go away.
>

Indeed, sounds good to me. I'll post a variant of the patch with the
xfs_defer_cancel() call removed. I've also made a note to audit the
remaining xfs_defer_cancel() callers to see if we can kill those off.

Brian

> xfs_defer_finish is a different story -- it can be used (by online
> repair for example) to process the deferred operations so they don't sit
> around pinning memory and log, even if there's a lot more work to be
> done before _trans_commit. But I don't think we need to _defer_cancel
> if things go awry.
>
> --D
>
> > Brian
> >
> > > --D
> > >
> > > > }
> > > > bp->b_ops = &xfs_dquot_buf_ops;
> > > >
> > > > @@ -349,10 +349,8 @@ xfs_dquot_disk_alloc(
> > > > * the buffer locked across the _defer_finish call. We can now do
> > > > * this correctly with xfs_defer_bjoin.
> > > > *
> > > > - * Above, we allocated a disk block for the dquot information and
> > > > - * used get_buf to initialize the dquot. If the _defer_bjoin fails,
> > > > - * the buffer is still locked to *tpp, so we must _bhold_release and
> > > > - * then _trans_brelse the buffer. If the _defer_finish fails, the old
> > > > + * Above, we allocated a disk block for the dquot information and used
> > > > + * get_buf to initialize the dquot. If the _defer_finish fails, the old
> > > > * transaction is gone but the new buffer is not joined or held to any
> > > > * transaction, so we must _buf_relse it.
> > > > *
> > > > @@ -362,24 +360,14 @@ xfs_dquot_disk_alloc(
> > > > * manually or by committing the transaction.
> > > > */
> > > > xfs_trans_bhold(tp, bp);
> > > > - if (error) {
> > > > - xfs_trans_bhold_release(tp, bp);
> > > > - xfs_trans_brelse(tp, bp);
> > > > - goto error1;
> > > > - }
> > > > error = xfs_defer_finish(tpp);
> > > > tp = *tpp;
> > > > if (error) {
> > > > xfs_buf_relse(bp);
> > > > - goto error0;
> > > > + return error;
> > > > }
> > > > *bpp = bp;
> > > > return 0;
> > > > -
> > > > -error1:
> > > > - xfs_defer_cancel(tp);
> > > > -error0:
> > > > - return error;
> > > > }
> > > >
> > > > /*
> > > > --
> > > > 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

2018-08-07 16:36:52

by Darrick J. Wong

[permalink] [raw]
Subject: Re: xfs: cancel dfops on xfs_defer_finish() error

On Tue, Aug 07, 2018 at 10:37:21AM -0400, Brian Foster wrote:
> On Tue, Aug 07, 2018 at 03:14:07PM +0100, Colin Ian King wrote:
> > Hi,
> >
> > Recent commit 82ff27bc52a88cb5cc400bfa64e210d3ec8dfebd ("xfs: automatic
> > dfops buffer relogging") removed the assignment of variable error:
> >
> > - error = xfs_defer_bjoin(tp->t_dfops, bp);
> > if (error) {
> > xfs_trans_bhold_release(tp, bp);
> > xfs_trans_brelse(tp, bp);
> >
>
> Hmm, I _think_ we can just drop these error checks now that this
> pre-finish error state is non-existent, something like the appended diff
> (with additional cleanups). E.g., if the buffer is held in the
> transaction then the bjoin is implicit. If the finish fails, then the
> state is essentially unchanged by the relogging patch.
>
> That said, the error handling is a bit tricky here. Darrick, I think you
> reworked this recently.. thoughts?
>
> Brian
>
> --- 8< ---
>
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index 70a76ac41f01..2106c4142ecd 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -311,7 +311,7 @@ xfs_dquot_disk_alloc(
> XFS_DQUOT_CLUSTER_SIZE_FSB, XFS_BMAPI_METADATA,
> XFS_QM_DQALLOC_SPACE_RES(mp), &map, &nmaps);
> if (error)
> - goto error0;
> + return error;
> ASSERT(map.br_blockcount == XFS_DQUOT_CLUSTER_SIZE_FSB);
> ASSERT(nmaps == 1);
> ASSERT((map.br_startblock != DELAYSTARTBLOCK) &&
> @@ -326,8 +326,8 @@ xfs_dquot_disk_alloc(
> bp = xfs_trans_get_buf(tp, mp->m_ddev_targp, dqp->q_blkno,
> mp->m_quotainfo->qi_dqchunklen, 0);
> if (!bp) {
> - error = -ENOMEM;
> - goto error1;
> + xfs_defer_cancel(tp);
> + return -ENOMEM;

The only caller of xfs_dquot_disk_alloc checks the return value and
xfs_trans_cancels the transaction, which should take care of calling
xfs_defer_cancel, right?

--D

> }
> bp->b_ops = &xfs_dquot_buf_ops;
>
> @@ -349,10 +349,8 @@ xfs_dquot_disk_alloc(
> * the buffer locked across the _defer_finish call. We can now do
> * this correctly with xfs_defer_bjoin.
> *
> - * Above, we allocated a disk block for the dquot information and
> - * used get_buf to initialize the dquot. If the _defer_bjoin fails,
> - * the buffer is still locked to *tpp, so we must _bhold_release and
> - * then _trans_brelse the buffer. If the _defer_finish fails, the old
> + * Above, we allocated a disk block for the dquot information and used
> + * get_buf to initialize the dquot. If the _defer_finish fails, the old
> * transaction is gone but the new buffer is not joined or held to any
> * transaction, so we must _buf_relse it.
> *
> @@ -362,24 +360,14 @@ xfs_dquot_disk_alloc(
> * manually or by committing the transaction.
> */
> xfs_trans_bhold(tp, bp);
> - if (error) {
> - xfs_trans_bhold_release(tp, bp);
> - xfs_trans_brelse(tp, bp);
> - goto error1;
> - }
> error = xfs_defer_finish(tpp);
> tp = *tpp;
> if (error) {
> xfs_buf_relse(bp);
> - goto error0;
> + return error;
> }
> *bpp = bp;
> return 0;
> -
> -error1:
> - xfs_defer_cancel(tp);
> -error0:
> - return error;
> }
>
> /*
> --
> 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

2018-08-07 16:41:21

by Brian Foster

[permalink] [raw]
Subject: Re: xfs: cancel dfops on xfs_defer_finish() error

On Tue, Aug 07, 2018 at 08:10:29AM -0700, Darrick J. Wong wrote:
> On Tue, Aug 07, 2018 at 10:37:21AM -0400, Brian Foster wrote:
> > On Tue, Aug 07, 2018 at 03:14:07PM +0100, Colin Ian King wrote:
> > > Hi,
> > >
> > > Recent commit 82ff27bc52a88cb5cc400bfa64e210d3ec8dfebd ("xfs: automatic
> > > dfops buffer relogging") removed the assignment of variable error:
> > >
> > > - error = xfs_defer_bjoin(tp->t_dfops, bp);
> > > if (error) {
> > > xfs_trans_bhold_release(tp, bp);
> > > xfs_trans_brelse(tp, bp);
> > >
> >
> > Hmm, I _think_ we can just drop these error checks now that this
> > pre-finish error state is non-existent, something like the appended diff
> > (with additional cleanups). E.g., if the buffer is held in the
> > transaction then the bjoin is implicit. If the finish fails, then the
> > state is essentially unchanged by the relogging patch.
> >
> > That said, the error handling is a bit tricky here. Darrick, I think you
> > reworked this recently.. thoughts?
> >
> > Brian
> >
> > --- 8< ---
> >
> > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> > index 70a76ac41f01..2106c4142ecd 100644
> > --- a/fs/xfs/xfs_dquot.c
> > +++ b/fs/xfs/xfs_dquot.c
> > @@ -311,7 +311,7 @@ xfs_dquot_disk_alloc(
> > XFS_DQUOT_CLUSTER_SIZE_FSB, XFS_BMAPI_METADATA,
> > XFS_QM_DQALLOC_SPACE_RES(mp), &map, &nmaps);
> > if (error)
> > - goto error0;
> > + return error;
> > ASSERT(map.br_blockcount == XFS_DQUOT_CLUSTER_SIZE_FSB);
> > ASSERT(nmaps == 1);
> > ASSERT((map.br_startblock != DELAYSTARTBLOCK) &&
> > @@ -326,8 +326,8 @@ xfs_dquot_disk_alloc(
> > bp = xfs_trans_get_buf(tp, mp->m_ddev_targp, dqp->q_blkno,
> > mp->m_quotainfo->qi_dqchunklen, 0);
> > if (!bp) {
> > - error = -ENOMEM;
> > - goto error1;
> > + xfs_defer_cancel(tp);
> > + return -ENOMEM;
>
> The only caller of xfs_dquot_disk_alloc checks the return value and
> xfs_trans_cancels the transaction, which should take care of calling
> xfs_defer_cancel, right?
>

Yeah. IIRC I originally left the defer_cancel() alone in this function
just to be consistent, since it calls xfs_defer_finish() as well on the
caller's transaction. Technically I don't think it matters either way,
so I don't have much preference on if it stays or goes (assuming the
other changes are correct)...

Brian

> --D
>
> > }
> > bp->b_ops = &xfs_dquot_buf_ops;
> >
> > @@ -349,10 +349,8 @@ xfs_dquot_disk_alloc(
> > * the buffer locked across the _defer_finish call. We can now do
> > * this correctly with xfs_defer_bjoin.
> > *
> > - * Above, we allocated a disk block for the dquot information and
> > - * used get_buf to initialize the dquot. If the _defer_bjoin fails,
> > - * the buffer is still locked to *tpp, so we must _bhold_release and
> > - * then _trans_brelse the buffer. If the _defer_finish fails, the old
> > + * Above, we allocated a disk block for the dquot information and used
> > + * get_buf to initialize the dquot. If the _defer_finish fails, the old
> > * transaction is gone but the new buffer is not joined or held to any
> > * transaction, so we must _buf_relse it.
> > *
> > @@ -362,24 +360,14 @@ xfs_dquot_disk_alloc(
> > * manually or by committing the transaction.
> > */
> > xfs_trans_bhold(tp, bp);
> > - if (error) {
> > - xfs_trans_bhold_release(tp, bp);
> > - xfs_trans_brelse(tp, bp);
> > - goto error1;
> > - }
> > error = xfs_defer_finish(tpp);
> > tp = *tpp;
> > if (error) {
> > xfs_buf_relse(bp);
> > - goto error0;
> > + return error;
> > }
> > *bpp = bp;
> > return 0;
> > -
> > -error1:
> > - xfs_defer_cancel(tp);
> > -error0:
> > - return error;
> > }
> >
> > /*
> > --
> > 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