Commit ddf92053 authored by Christoph Hellwig's avatar Christoph Hellwig Committed by Darrick J. Wong
Browse files

xfs: split iop_unlock 



The iop_unlock method is called when comitting or cancelling a
transaction.  In the latter case, the transaction may or may not be
aborted.  While there is no known problem with the current code in
practice, this implementation is limited in that any log item
implementation that might want to differentiate between a commit and a
cancellation must rely on the aborted state.  The aborted bit is only
set when the cancelled transaction is dirty, however.  This means that
there is no way to distinguish between a commit and a clean transaction
cancellation.

For example, intent log items currently rely on this distinction.  The
log item is either transferred to the CIL on commit or released on
transaction cancel. There is currently no possibility for a clean intent
log item in a transaction, but if that state is ever introduced a cancel
of such a transaction will immediately result in memory leaks of the
associated log item(s).  This is an interface deficiency and landmine.

To clean this up, replace the iop_unlock method with an iop_release
method that is specific to transaction cancel.  The existing
iop_committing method occurs at the same time as iop_unlock in the
commit path and there is no need for two separate callbacks here.
Overload the iop_committing method with the current commit time
iop_unlock implementations to eliminate the need for the latter and
further simplify the interface.

Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarBrian Foster <bfoster@redhat.com>
Reviewed-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
parent 195cd83d

fs/xfs/xfs_bmap_item.c

+7 −10
Original line number Diff line number Diff line
@@ -120,10 +120,9 @@ xfs_bui_item_unpin( 
 * constructed and thus we free the BUI here directly.

 */

STATIC void

xfs_bui_item_unlock(

xfs_bui_item_release(

	struct xfs_log_item	*lip)

{

	if (test_bit(XFS_LI_ABORTED, &lip->li_flags))

	xfs_bui_release(BUI_ITEM(lip));

}
@@ -134,7 +133,7 @@ static const struct xfs_item_ops xfs_bui_item_ops = { 
	.iop_size	= xfs_bui_item_size,

	.iop_format	= xfs_bui_item_format,

	.iop_unpin	= xfs_bui_item_unpin,

	.iop_unlock	= xfs_bui_item_unlock,

	.iop_release	= xfs_bui_item_release,

};



/*
@@ -201,16 +200,14 @@ xfs_bud_item_format( 
 * BUD.

 */

STATIC void

xfs_bud_item_unlock(

xfs_bud_item_release(

	struct xfs_log_item	*lip)

{

	struct xfs_bud_log_item	*budp = BUD_ITEM(lip);



	if (test_bit(XFS_LI_ABORTED, &lip->li_flags)) {

	xfs_bui_release(budp->bud_buip);

	kmem_zone_free(xfs_bud_zone, budp);

}

}



/*

 * When the bud item is committed to disk, all we need to do is delete our
@@ -243,7 +240,7 @@ xfs_bud_item_committed( 
static const struct xfs_item_ops xfs_bud_item_ops = {

	.iop_size	= xfs_bud_item_size,

	.iop_format	= xfs_bud_item_format,

	.iop_unlock	= xfs_bud_item_unlock,

	.iop_release	= xfs_bud_item_release,

	.iop_committed	= xfs_bud_item_committed,

};

fs/xfs/xfs_buf_item.c

+12 −3
Original line number Diff line number Diff line
@@ -595,7 +595,7 @@ xfs_buf_item_put( 
 * free the item.

 */

STATIC void

xfs_buf_item_unlock(

xfs_buf_item_release(

	struct xfs_log_item	*lip)

{

	struct xfs_buf_log_item	*bip = BUF_ITEM(lip);
@@ -610,7 +610,7 @@ xfs_buf_item_unlock( 
						   &lip->li_flags);

#endif



	trace_xfs_buf_item_unlock(bip);

	trace_xfs_buf_item_release(bip);



	/*

	 * The bli dirty state should match whether the blf has logged segments
@@ -640,6 +640,14 @@ xfs_buf_item_unlock( 
	xfs_buf_relse(bp);

}



STATIC void

xfs_buf_item_committing(

	struct xfs_log_item	*lip,

	xfs_lsn_t		commit_lsn)

{

	return xfs_buf_item_release(lip);

}



/*

 * This is called to find out where the oldest active copy of the

 * buf log item in the on disk log resides now that the last log
@@ -680,7 +688,8 @@ static const struct xfs_item_ops xfs_buf_item_ops = { 
	.iop_format	= xfs_buf_item_format,

	.iop_pin	= xfs_buf_item_pin,

	.iop_unpin	= xfs_buf_item_unpin,

	.iop_unlock	= xfs_buf_item_unlock,

	.iop_release	= xfs_buf_item_release,

	.iop_committing	= xfs_buf_item_committing,

	.iop_committed	= xfs_buf_item_committed,

	.iop_push	= xfs_buf_item_push,

};

fs/xfs/xfs_dquot_item.c

+11 −8
Original line number Diff line number Diff line
@@ -198,14 +198,8 @@ out_unlock: 
	return rval;

}



/*

 * Unlock the dquot associated with the log item.

 * Clear the fields of the dquot and dquot log item that

 * are specific to the current transaction.  If the

 * hold flags is set, do not unlock the dquot.

 */

STATIC void

xfs_qm_dquot_logitem_unlock(

xfs_qm_dquot_logitem_release(

	struct xfs_log_item	*lip)

{

	struct xfs_dquot	*dqp = DQUOT_ITEM(lip)->qli_dquot;
@@ -221,6 +215,14 @@ xfs_qm_dquot_logitem_unlock( 
	xfs_dqunlock(dqp);

}



STATIC void

xfs_qm_dquot_logitem_committing(

	struct xfs_log_item	*lip,

	xfs_lsn_t		commit_lsn)

{

	return xfs_qm_dquot_logitem_release(lip);

}



/*

 * This is the ops vector for dquots

 */
@@ -229,7 +231,8 @@ static const struct xfs_item_ops xfs_dquot_item_ops = { 
	.iop_format	= xfs_qm_dquot_logitem_format,

	.iop_pin	= xfs_qm_dquot_logitem_pin,

	.iop_unpin	= xfs_qm_dquot_logitem_unpin,

	.iop_unlock	= xfs_qm_dquot_logitem_unlock,

	.iop_release	= xfs_qm_dquot_logitem_release,

	.iop_committing	= xfs_qm_dquot_logitem_committing,

	.iop_push	= xfs_qm_dquot_logitem_push,

	.iop_error	= xfs_dquot_item_error

};

fs/xfs/xfs_extfree_item.c

+7 −10
Original line number Diff line number Diff line
@@ -130,10 +130,9 @@ xfs_efi_item_unpin( 
 * constructed and thus we free the EFI here directly.

 */

STATIC void

xfs_efi_item_unlock(

xfs_efi_item_release(

	struct xfs_log_item	*lip)

{

	if (test_bit(XFS_LI_ABORTED, &lip->li_flags))

	xfs_efi_release(EFI_ITEM(lip));

}
@@ -144,7 +143,7 @@ static const struct xfs_item_ops xfs_efi_item_ops = { 
	.iop_size	= xfs_efi_item_size,

	.iop_format	= xfs_efi_item_format,

	.iop_unpin	= xfs_efi_item_unpin,

	.iop_unlock	= xfs_efi_item_unlock,

	.iop_release	= xfs_efi_item_release,

};
@@ -300,16 +299,14 @@ xfs_efd_item_format( 
 * the transaction is cancelled, drop our reference to the EFI and free the EFD.

 */

STATIC void

xfs_efd_item_unlock(

xfs_efd_item_release(

	struct xfs_log_item	*lip)

{

	struct xfs_efd_log_item	*efdp = EFD_ITEM(lip);



	if (test_bit(XFS_LI_ABORTED, &lip->li_flags)) {

	xfs_efi_release(efdp->efd_efip);

	xfs_efd_item_free(efdp);

}

}



/*

 * When the efd item is committed to disk, all we need to do is delete our
@@ -342,7 +339,7 @@ xfs_efd_item_committed( 
static const struct xfs_item_ops xfs_efd_item_ops = {

	.iop_size	= xfs_efd_item_size,

	.iop_format	= xfs_efd_item_format,

	.iop_unlock	= xfs_efd_item_unlock,

	.iop_release	= xfs_efd_item_release,

	.iop_committed	= xfs_efd_item_committed,

};

fs/xfs/xfs_icreate_item.c

+3 −7
Original line number Diff line number Diff line
@@ -57,14 +57,10 @@ xfs_icreate_item_format( 
}



STATIC void

xfs_icreate_item_unlock(

xfs_icreate_item_release(

	struct xfs_log_item	*lip)

{

	struct xfs_icreate_item	*icp = ICR_ITEM(lip);



	if (test_bit(XFS_LI_ABORTED, &lip->li_flags))

		kmem_zone_free(xfs_icreate_zone, icp);

	return;

	kmem_zone_free(xfs_icreate_zone, ICR_ITEM(lip));

}



/*
@@ -89,7 +85,7 @@ xfs_icreate_item_committed( 
static const struct xfs_item_ops xfs_icreate_item_ops = {

	.iop_size	= xfs_icreate_item_size,

	.iop_format	= xfs_icreate_item_format,

	.iop_unlock	= xfs_icreate_item_unlock,

	.iop_release	= xfs_icreate_item_release,

	.iop_committed	= xfs_icreate_item_committed,

};

CRA Git | Maintained and supported by SUSTech CRA and CCSE