xfs: use a lockref for the xfs_dquot reference count

The xfs_dquot structure currently uses the anti-pattern of using the
in-object lock that protects the content to also serialize reference
count updates for the structure, leading to a cumbersome free path.
This is partially papered over by the fact that we never free the dquot
directly but always through the LRU.  Switch to use a lockref instead and
move the reference counter manipulations out of q_qlock.

To make this work, xfs_qm_flush_one and xfs_qm_flush_one are converted to
acquire a dquot reference while flushing to integrate with the lockref
"get if not dead" scheme.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>

diff --git a/fs/xfs/libxfs/xfs_quota_defs.h b/fs/xfs/libxfs/xfs_quota_defs.h
index 763d941a8420c5aefb5bf175edba737e80ee76cd..551d7ae46c5c21d7471ad3d4a5110b8a03fbaaa2 100644 (file)
--- a/fs/xfs/libxfs/xfs_quota_defs.h
+++ b/fs/xfs/libxfs/xfs_quota_defs.h
@@ -29,11 +29,9 @@ typedef uint8_t              xfs_dqtype_t;
  * flags for q_flags field in the dquot.
  */
 #define XFS_DQFLAG_DIRTY       (1u << 0)       /* dquot is dirty */
-#define XFS_DQFLAG_FREEING     (1u << 1)       /* dquot is being torn down */
 
 #define XFS_DQFLAG_STRINGS \
-       { XFS_DQFLAG_DIRTY,     "DIRTY" }, \
-       { XFS_DQFLAG_FREEING,   "FREEING" }
+       { XFS_DQFLAG_DIRTY,     "DIRTY" }
 
 /*
  * We have the possibility of all three quota types being active at once, and

diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index c2326cee7fae8a5caae5226bf12a45d945fe0035..34c325524ab9da85572d6f2de6a619029700814d 100644 (file)
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -816,20 +816,17 @@ restart:
                return NULL;
        }
 
-       mutex_lock(&dqp->q_qlock);
-       if (dqp->q_flags & XFS_DQFLAG_FREEING) {
-               mutex_unlock(&dqp->q_qlock);
+       if (!lockref_get_not_dead(&dqp->q_lockref)) {
                mutex_unlock(&qi->qi_tree_lock);
                trace_xfs_dqget_freeing(dqp);
                delay(1);
                goto restart;
        }
-
-       dqp->q_nrefs++;
        mutex_unlock(&qi->qi_tree_lock);
 
        trace_xfs_dqget_hit(dqp);
        XFS_STATS_INC(mp, xs_qm_dqcachehits);
+       mutex_lock(&dqp->q_qlock);
        return dqp;
 }
 
@@ -866,7 +863,7 @@ xfs_qm_dqget_cache_insert(
 
        /* Return a locked dquot to the caller, with a reference taken. */
        mutex_lock(&dqp->q_qlock);
-       dqp->q_nrefs = 1;
+       lockref_init(&dqp->q_lockref);
        qi->qi_dquots++;
 
 out_unlock:
@@ -1124,18 +1121,22 @@ void
 xfs_qm_dqput(
        struct xfs_dquot        *dqp)
 {
-       ASSERT(dqp->q_nrefs > 0);
        ASSERT(XFS_DQ_IS_LOCKED(dqp));
 
        trace_xfs_dqput(dqp);
 
-       if (--dqp->q_nrefs == 0) {
+       if (lockref_put_or_lock(&dqp->q_lockref))
+               goto out_unlock;
+
+       if (!--dqp->q_lockref.count) {
                struct xfs_quotainfo    *qi = dqp->q_mount->m_quotainfo;
                trace_xfs_dqput_free(dqp);
 
                if (list_lru_add_obj(&qi->qi_lru, &dqp->q_lru))
                        XFS_STATS_INC(dqp->q_mount, xs_qm_dquot_unused);
        }
+       spin_unlock(&dqp->q_lockref.lock);
+out_unlock:
        mutex_unlock(&dqp->q_qlock);
 }
 

diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index 10c39b8cdd039a4f8077e88c13c702d425600e0c..c56fbc39d0890bcf96beb87507e1e488e5f4db50 100644 (file)
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -71,7 +71,7 @@ struct xfs_dquot {
        xfs_dqtype_t            q_type;
        uint16_t                q_flags;
        xfs_dqid_t              q_id;
-       uint                    q_nrefs;
+       struct lockref          q_lockref;
        int                     q_bufoffset;
        xfs_daddr_t             q_blkno;
        xfs_fileoff_t           q_fileoffset;
@@ -231,9 +231,7 @@ void xfs_dquot_detach_buf(struct xfs_dquot *dqp);
 
 static inline struct xfs_dquot *xfs_qm_dqhold(struct xfs_dquot *dqp)
 {
-       mutex_lock(&dqp->q_qlock);
-       dqp->q_nrefs++;
-       mutex_unlock(&dqp->q_qlock);
+       lockref_get(&dqp->q_lockref);
        return dqp;
 }
 

diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 3e88bea9a4657deccdb6886798a935dde8f14297..f98f9fdac0b5d007f1e0b64c36fd167df06a578e 100644 (file)
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -126,14 +126,16 @@ xfs_qm_dqpurge(
        void                    *data)
 {
        struct xfs_quotainfo    *qi = dqp->q_mount->m_quotainfo;
-       int                     error = -EAGAIN;
 
-       mutex_lock(&dqp->q_qlock);
-       if ((dqp->q_flags & XFS_DQFLAG_FREEING) || dqp->q_nrefs != 0)
-               goto out_unlock;
-
-       dqp->q_flags |= XFS_DQFLAG_FREEING;
+       spin_lock(&dqp->q_lockref.lock);
+       if (dqp->q_lockref.count > 0 || __lockref_is_dead(&dqp->q_lockref)) {
+               spin_unlock(&dqp->q_lockref.lock);
+               return -EAGAIN;
+       }
+       lockref_mark_dead(&dqp->q_lockref);
+       spin_unlock(&dqp->q_lockref.lock);
 
+       mutex_lock(&dqp->q_qlock);
        xfs_qm_dqunpin_wait(dqp);
        xfs_dqflock(dqp);
 
@@ -144,6 +146,7 @@ xfs_qm_dqpurge(
         */
        if (XFS_DQ_IS_DIRTY(dqp)) {
                struct xfs_buf  *bp = NULL;
+               int             error;
 
                /*
                 * We don't care about getting disk errors here. We need
@@ -151,9 +154,9 @@ xfs_qm_dqpurge(
                 */
                error = xfs_dquot_use_attached_buf(dqp, &bp);
                if (error == -EAGAIN) {
-                       xfs_dqfunlock(dqp);
-                       dqp->q_flags &= ~XFS_DQFLAG_FREEING;
-                       goto out_unlock;
+                       /* resurrect the refcount from the dead. */
+                       dqp->q_lockref.count = 0;
+                       goto out_funlock;
                }
                if (!bp)
                        goto out_funlock;
@@ -192,10 +195,6 @@ out_funlock:
 
        xfs_qm_dqdestroy(dqp);
        return 0;
-
-out_unlock:
-       mutex_unlock(&dqp->q_qlock);
-       return error;
 }
 
 /*
@@ -468,7 +467,7 @@ xfs_qm_dquot_isolate(
        struct xfs_qm_isolate   *isol = arg;
        enum lru_status         ret = LRU_SKIP;
 
-       if (!mutex_trylock(&dqp->q_qlock))
+       if (!spin_trylock(&dqp->q_lockref.lock))
                goto out_miss_busy;
 
        /*
@@ -476,7 +475,7 @@ xfs_qm_dquot_isolate(
         * from the LRU, leave it for the freeing task to complete the freeing
         * process rather than risk it being free from under us here.
         */
-       if (dqp->q_flags & XFS_DQFLAG_FREEING)
+       if (__lockref_is_dead(&dqp->q_lockref))
                goto out_miss_unlock;
 
        /*
@@ -485,16 +484,15 @@ xfs_qm_dquot_isolate(
         * again.
         */
        ret = LRU_ROTATE;
-       if (XFS_DQ_IS_DIRTY(dqp) || atomic_read(&dqp->q_pincount) > 0) {
+       if (XFS_DQ_IS_DIRTY(dqp) || atomic_read(&dqp->q_pincount) > 0)
                goto out_miss_unlock;
-       }
 
        /*
         * This dquot has acquired a reference in the meantime remove it from
         * the freelist and try again.
         */
-       if (dqp->q_nrefs) {
-               mutex_unlock(&dqp->q_qlock);
+       if (dqp->q_lockref.count) {
+               spin_unlock(&dqp->q_lockref.lock);
                XFS_STATS_INC(dqp->q_mount, xs_qm_dqwants);
 
                trace_xfs_dqreclaim_want(dqp);
@@ -518,10 +516,9 @@ xfs_qm_dquot_isolate(
        /*
         * Prevent lookups now that we are past the point of no return.
         */
-       dqp->q_flags |= XFS_DQFLAG_FREEING;
-       mutex_unlock(&dqp->q_qlock);
+       lockref_mark_dead(&dqp->q_lockref);
+       spin_unlock(&dqp->q_lockref.lock);
 
-       ASSERT(dqp->q_nrefs == 0);
        list_lru_isolate_move(lru, &dqp->q_lru, &isol->dispose);
        XFS_STATS_DEC(dqp->q_mount, xs_qm_dquot_unused);
        trace_xfs_dqreclaim_done(dqp);
@@ -529,7 +526,7 @@ xfs_qm_dquot_isolate(
        return LRU_REMOVED;
 
 out_miss_unlock:
-       mutex_unlock(&dqp->q_qlock);
+       spin_unlock(&dqp->q_lockref.lock);
 out_miss_busy:
        trace_xfs_dqreclaim_busy(dqp);
        XFS_STATS_INC(dqp->q_mount, xs_qm_dqreclaim_misses);
@@ -1467,9 +1464,10 @@ xfs_qm_flush_one(
        struct xfs_buf          *bp = NULL;
        int                     error = 0;
 
+       if (!lockref_get_not_dead(&dqp->q_lockref))
+               return 0;
+
        mutex_lock(&dqp->q_qlock);
-       if (dqp->q_flags & XFS_DQFLAG_FREEING)
-               goto out_unlock;
        if (!XFS_DQ_IS_DIRTY(dqp))
                goto out_unlock;
 
@@ -1489,7 +1487,7 @@ xfs_qm_flush_one(
                xfs_buf_delwri_queue(bp, buffer_list);
        xfs_buf_relse(bp);
 out_unlock:
-       mutex_unlock(&dqp->q_qlock);
+       xfs_qm_dqput(dqp);
        return error;
 }
 

diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 79b8641880ab9da7f711376ee216444d49ba9a0a..46d21eb11ccbf42f76bdaf320bf437f0606c415b 100644 (file)
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -1350,7 +1350,7 @@ DECLARE_EVENT_CLASS(xfs_dquot_class,
                __entry->id = dqp->q_id;
                __entry->type = dqp->q_type;
                __entry->flags = dqp->q_flags;
-               __entry->nrefs = dqp->q_nrefs;
+               __entry->nrefs = data_race(dqp->q_lockref.count);
 
                __entry->res_bcount = dqp->q_blk.reserved;
                __entry->res_rtbcount = dqp->q_rtb.reserved;