Commit 08fb3905 authored by Dave Chinner's avatar Dave Chinner Committed by Ben Myers
Browse files

xfs: avoid nesting transactions in xfs_qm_scall_setqlim() 



Lockdep reports:

=============================================
[ INFO: possible recursive locking detected ]
3.9.0+ #3 Not tainted
---------------------------------------------
setquota/28368 is trying to acquire lock:
 (sb_internal){++++.?}, at: [<c11e8846>] xfs_trans_alloc+0x26/0x50

but task is already holding lock:
 (sb_internal){++++.?}, at: [<c11e8846>] xfs_trans_alloc+0x26/0x50

from xfs_qm_scall_setqlim()->xfs_dqread() when a dquot needs to be
allocated.

xfs_qm_scall_setqlim() is starting a transaction and then not
passing it into xfs_qm_dqet() and so it starts it's own transaction
when allocating the dquot.  Splat!

Fix this by not allocating the dquot in xfs_qm_scall_setqlim()
inside the setqlim transaction. This requires getting the dquot
first (and allocating it if necessary) then dropping and relocking
the dquot before joining it to the setqlim transaction.

Reported-by: default avatarMichael L. Semon <mlsemon35@gmail.com>
Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
Reviewed-by: default avatarBen Myers <bpm@sgi.com>
Signed-off-by: default avatarBen Myers <bpm@sgi.com>
(cherry picked from commit f648167f)
parent 7ae07780

fs/xfs/xfs_qm_syscalls.c

+23 −17
Original line number Diff line number Diff line
@@ -489,31 +489,36 @@ xfs_qm_scall_setqlim( 
	if ((newlim->d_fieldmask & XFS_DQ_MASK) == 0)

		return 0;



	tp = xfs_trans_alloc(mp, XFS_TRANS_QM_SETQLIM);

	error = xfs_trans_reserve(tp, 0, XFS_QM_SETQLIM_LOG_RES(mp),

				  0, 0, XFS_DEFAULT_LOG_COUNT);

	if (error) {

		xfs_trans_cancel(tp, 0);

		return (error);

	}



	/*

	 * We don't want to race with a quotaoff so take the quotaoff lock.

	 * (We don't hold an inode lock, so there's nothing else to stop

	 * a quotaoff from happening). (XXXThis doesn't currently happen

	 * because we take the vfslock before calling xfs_qm_sysent).

	 * We don't hold an inode lock, so there's nothing else to stop

	 * a quotaoff from happening.

	 */

	mutex_lock(&q->qi_quotaofflock);



	/*

	 * Get the dquot (locked), and join it to the transaction.

	 * Allocate the dquot if this doesn't exist.

	 * Get the dquot (locked) before we start, as we need to do a

	 * transaction to allocate it if it doesn't exist. Once we have the

	 * dquot, unlock it so we can start the next transaction safely. We hold

	 * a reference to the dquot, so it's safe to do this unlock/lock without

	 * it being reclaimed in the mean time.

	 */

	if ((error = xfs_qm_dqget(mp, NULL, id, type, XFS_QMOPT_DQALLOC, &dqp))) {

		xfs_trans_cancel(tp, XFS_TRANS_ABORT);

	error = xfs_qm_dqget(mp, NULL, id, type, XFS_QMOPT_DQALLOC, &dqp);

	if (error) {

		ASSERT(error != ENOENT);

		goto out_unlock;

	}

	xfs_dqunlock(dqp);



	tp = xfs_trans_alloc(mp, XFS_TRANS_QM_SETQLIM);

	error = xfs_trans_reserve(tp, 0, XFS_QM_SETQLIM_LOG_RES(mp),

				  0, 0, XFS_DEFAULT_LOG_COUNT);

	if (error) {

		xfs_trans_cancel(tp, 0);

		goto out_rele;

	}



	xfs_dqlock(dqp);

	xfs_trans_dqjoin(tp, dqp);

	ddq = &dqp->q_core;
@@ -621,8 +626,9 @@ xfs_qm_scall_setqlim( 
	xfs_trans_log_dquot(tp, dqp);



	error = xfs_trans_commit(tp, 0);

	xfs_qm_dqrele(dqp);



out_rele:

	xfs_qm_dqrele(dqp);

out_unlock:

	mutex_unlock(&q->qi_quotaofflock);

	return error;

CRA Git | Maintained and supported by SUSTech CRA and CCSE