[PATCH 10/16] xfs: nest qm_dqfrlist_lock insise the dquot qlock

[Dave Chinner]

On Mon, Nov 28, 2011 at 03:27:32AM -0500, Christoph Hellwig wrote:
> Allow xfs_qm_dqput to work without trylock loops by nesting the freelist lock
> inside the dquot qlock.  In turn that requires trylocks in the reclaim path
> instead, but given it's a classic tradeoff between fast and slow path, and
> we follow the model of the inode and dentry caches.

Funny how that keeps coming up ;)

> Document our new lock order now that it has settled.

Excellent.

> Signed-off-by: Christoph Hellwig <hch at lst.de>

Subject line has a type (insise).

> 
> ---
>  fs/xfs/xfs_dquot.c |   98 +++++++++++++++++++++--------------------------------
>  fs/xfs/xfs_qm.c    |    4 +-
>  2 files changed, 42 insertions(+), 60 deletions(-)
> 
> Index: xfs/fs/xfs/xfs_dquot.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_dquot.c	2011-11-25 11:45:53.348630228 +0100
> +++ xfs/fs/xfs/xfs_dquot.c	2011-11-25 11:56:08.655296826 +0100
> @@ -39,20 +39,19 @@
>  #include "xfs_qm.h"
>  #include "xfs_trace.h"
>  
> -
>  /*
> -   LOCK ORDER
> -
> -   inode lock		    (ilock)
> -   dquot hash-chain lock    (hashlock)
> -   xqm dquot freelist lock  (freelistlock
> -   mount's dquot list lock  (mplistlock)
> -   user dquot lock - lock ordering among dquots is based on the uid or gid
> -   group dquot lock - similar to udquots. Between the two dquots, the udquot
> -		      has to be locked first.
> -   pin lock - the dquot lock must be held to take this lock.
> -   flush lock - ditto.
> -*/
> + * Lock order:
> + *
> + * ip->i_lock
> + *   qh->qh_lock
> + *     qi->qi_dqlist_lock
> + *       dquot->q_qlock
> + *         dquot->q_flush
> + *         xfs_Gqm->qm_dqfrlist_lock
> + *
> + * If two dquots need to be locked the order is user before group/project,
> + * otherwise by the lowest id first, see xfs_dqlock2.

For the dquot->q_* locks, it might be worth adding the name of the
locking functions here so it is obvious when reading the code as
q_qlock and q_flush aren't used anywhere outside the locking
wrappers...

Otherwise looks sane.

Reviewed-by: Dave Chinner <dchinner at redhat.com>

-- 
Dave Chinner
david at fromorbit.com