Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit fd5670f2 authored by Dave Chinner's avatar Dave Chinner Committed by Alex Elder
Browse files

xfs: make AIL target updates and compares 32bit safe. 



The recent conversion of the xfsaild functionality to a work queue
introduced a hard-to-hit log space grant hang. One of the problems
noticed was that updates of the push target are not 32 bit safe as
the target is a 64 bit value.

We cannot copy a 64 bit LSN without the possibility of corrupting
the result when racing with another updating thread. We have
function to do this update safely without needing to care about
32/64 bit issues - xfs_trans_ail_copy_lsn() - so use that when
updating the AIL push target.

Also move the reading of the target in the push work inside the AIL
lock, and use XFS_LSN_CMP() for the unlocked comparison during work
termination to close read holes as well.

Signed-off-by: default avatarDave Chinner <david@fromorbit.com>
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarAlex Elder <aelder@sgi.com>
parent cb64026b

fs/xfs/xfs_trans_ail.c

+4 −3
Original line number Original line Diff line number Diff line
@@ -354,7 +354,7 @@ xfs_ail_worker( 
	struct xfs_ail_cursor	*cur = &ailp->xa_cursors;

	struct xfs_ail_cursor	*cur = &ailp->xa_cursors;

	xfs_log_item_t		*lip;

	xfs_log_item_t		*lip;

	xfs_lsn_t		lsn;

	xfs_lsn_t		lsn;

	xfs_lsn_t		target = ailp->xa_target;

	xfs_lsn_t		target;

	long			tout = 10;

	long			tout = 10;

	int			flush_log = 0;

	int			flush_log = 0;

	int			stuck = 0;

	int			stuck = 0;
@@ -362,6 +362,7 @@ xfs_ail_worker( 
	int			push_xfsbufd = 0;

	int			push_xfsbufd = 0;





	spin_lock(&ailp->xa_lock);

	spin_lock(&ailp->xa_lock);

	target = ailp->xa_target;

	xfs_trans_ail_cursor_init(ailp, cur);

	xfs_trans_ail_cursor_init(ailp, cur);

	lip = xfs_trans_ail_cursor_first(ailp, cur, ailp->xa_last_pushed_lsn);

	lip = xfs_trans_ail_cursor_first(ailp, cur, ailp->xa_last_pushed_lsn);

	if (!lip || XFS_FORCED_SHUTDOWN(mp)) {

	if (!lip || XFS_FORCED_SHUTDOWN(mp)) {
@@ -491,7 +492,7 @@ xfs_ail_worker( 
		 * work to do. Wait a bit longer before starting that work.

		 * work to do. Wait a bit longer before starting that work.

		 */

		 */

		smp_rmb();

		smp_rmb();

		if (ailp->xa_target == target) {

		if (XFS_LSN_CMP(ailp->xa_target, target) == 0) {

			clear_bit(XFS_AIL_PUSHING_BIT, &ailp->xa_flags);

			clear_bit(XFS_AIL_PUSHING_BIT, &ailp->xa_flags);

			return;

			return;

		}

		}
@@ -553,7 +554,7 @@ xfs_ail_push( 
	 * the XFS_AIL_PUSHING_BIT.

	 * the XFS_AIL_PUSHING_BIT.

	 */

	 */

	smp_wmb();

	smp_wmb();

	ailp->xa_target = threshold_lsn;

	xfs_trans_ail_copy_lsn(ailp, &ailp->xa_target, &threshold_lsn);

	if (!test_and_set_bit(XFS_AIL_PUSHING_BIT, &ailp->xa_flags))

	if (!test_and_set_bit(XFS_AIL_PUSHING_BIT, &ailp->xa_flags))

		queue_delayed_work(xfs_syncd_wq, &ailp->xa_work, 0);

		queue_delayed_work(xfs_syncd_wq, &ailp->xa_work, 0);

}

}