Commit b73e0528 authored by Christoph Hellwig's avatar Christoph Hellwig Committed by Carlos Maiolino
Browse files

xfs: remove the leftover xfs_{set,clear}_li_failed infrastructure 



Marking a log item as failed kept a buffer reference around for
resubmission of inode and dquote items.

For inode items commit 298f7bec ("xfs: pin inode backing buffer to
the inode log item") started pinning the inode item buffers
unconditionally and removed the need for this.  Later commit acc8f862
("xfs: attach dquot buffer to dquot log item buffer") did the same for
dquot items but didn't fully clean up the xfs_clear_li_failed side
for them.  Stop adding the extra pin for dquot items and remove the
helpers.

This happens to fix a call to xfs_buf_free with the AIL lock held,
which would be incorrect for the unlikely case freeing the buffer
ends up calling vfree.

Reported-by: default avatarDan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarCarlos Maiolino <cmaiolino@redhat.com>
Signed-off-by: default avatarCarlos Maiolino <cem@kernel.org>
parent 8ffd015d

fs/xfs/xfs_dquot.c

+1 −2
Original line number Diff line number Diff line
@@ -1186,9 +1186,8 @@ xfs_qm_dqflush_done( 
	if (test_bit(XFS_LI_IN_AIL, &lip->li_flags) &&

	    (lip->li_lsn == qlip->qli_flush_lsn ||

	     test_bit(XFS_LI_FAILED, &lip->li_flags))) {



		spin_lock(&ailp->ail_lock);

		xfs_clear_li_failed(lip);

		clear_bit(XFS_LI_FAILED, &lip->li_flags);

		if (lip->li_lsn == qlip->qli_flush_lsn) {

			/* xfs_ail_update_finish() drops the AIL lock */

			tail_lsn = xfs_ail_delete_one(ailp, lip);

fs/xfs/xfs_inode_item.c

+0 −6
Original line number Diff line number Diff line
@@ -1089,13 +1089,7 @@ xfs_iflush_abort( 
	 * state. Whilst the inode is in the AIL, it should have a valid buffer

	 * pointer for push operations to access - it is only safe to remove the

	 * inode from the buffer once it has been removed from the AIL.

	 *

	 * We also clear the failed bit before removing the item from the AIL

	 * as xfs_trans_ail_delete()->xfs_clear_li_failed() will release buffer

	 * references the inode item owns and needs to hold until we've fully

	 * aborted the inode log item and detached it from the buffer.

	 */

	clear_bit(XFS_LI_FAILED, &iip->ili_item.li_flags);

	xfs_trans_ail_delete(&iip->ili_item, 0);



	/*

fs/xfs/xfs_trans_ail.c

+2 −3
Original line number Diff line number Diff line
@@ -909,10 +909,9 @@ xfs_trans_ail_delete( 
		return;

	}



	/* xfs_ail_update_finish() drops the AIL lock */

	xfs_clear_li_failed(lip);

	clear_bit(XFS_LI_FAILED, &lip->li_flags);

	tail_lsn = xfs_ail_delete_one(ailp, lip);

	xfs_ail_update_finish(ailp, tail_lsn);

	xfs_ail_update_finish(ailp, tail_lsn);	/* drops the AIL lock */

}



int

fs/xfs/xfs_trans_priv.h

+0 −28
Original line number Diff line number Diff line
@@ -167,32 +167,4 @@ xfs_trans_ail_copy_lsn( 
}

#endif



static inline void

xfs_clear_li_failed(

	struct xfs_log_item	*lip)

{

	struct xfs_buf	*bp = lip->li_buf;



	ASSERT(test_bit(XFS_LI_IN_AIL, &lip->li_flags));

	lockdep_assert_held(&lip->li_ailp->ail_lock);



	if (test_and_clear_bit(XFS_LI_FAILED, &lip->li_flags)) {

		lip->li_buf = NULL;

		xfs_buf_rele(bp);

	}

}



static inline void

xfs_set_li_failed(

	struct xfs_log_item	*lip,

	struct xfs_buf		*bp)

{

	lockdep_assert_held(&lip->li_ailp->ail_lock);



	if (!test_and_set_bit(XFS_LI_FAILED, &lip->li_flags)) {

		xfs_buf_hold(bp);

		lip->li_buf = bp;

	}

}



#endif	/* __XFS_TRANS_PRIV_H__ */