dsched_fq - Overhaul locking
authorAlex Hornung <ahornung@gmail.com>
Sat, 17 Apr 2010 09:51:27 +0000 (09:51 +0000)
committerAlex Hornung <ahornung@gmail.com>
Sat, 17 Apr 2010 10:56:14 +0000 (10:56 +0000)
* use lockmgr lock for FQP lock, as some strategy ops can sleep while
  acquiring another lock (CAM SIM lock, for example).

* reduce overall locking when it isn't really required, mainly during
  deallocation (losing last ref) of objects. The locking is only
  explicitly required to protect the internal TAILQs.

* NOTE: this is an _attempt_ to fix some unidentified deadlocks that have
  been reported occasionally. While it shouldn't happen, be aware that
  this might explode.

Reported-by: Antonio Huete, Jan Lentfer
sys/dsched/fq/dsched_fq.h
sys/dsched/fq/dsched_fq_core.c

index e8a27ea..c825687 100644 (file)
 #ifndef _SYS_SPINLOCK_H_
 #include <sys/spinlock.h>
 #endif
-/*
-#define        FQ_IOQ_INIT(x)          lockinit(&(x)->fq_lock, "fqioq", 0, LK_CANRECURSE)
-#define FQ_IOQ_LOCK(x)         lockmgr(&(x)->fq_lock, LK_EXCLUSIVE)
-#define        FQ_IOQ_UNLOCK(x)        lockmgr(&(x)->fq_lock, LK_RELEASE)
-*/
+
+#define        FQ_FQP_LOCKINIT(x)      lockinit(&(x)->lock, "fqpbioq", 0, LK_CANRECURSE)
+#define        FQ_FQP_LOCK(x)          fq_reference_priv((x)); \
+                               lockmgr(&(x)->lock, LK_EXCLUSIVE)
+#define        FQ_FQP_UNLOCK(x)        lockmgr(&(x)->lock, LK_RELEASE); \
+                               fq_dereference_priv((x));
 
 #define        FQ_FQMP_LOCKINIT(x)     spin_init(&(x)->lock)
+#if 0
 #define        FQ_FQP_LOCKINIT(x)      spin_init(&(x)->lock)
+#endif
 #define        FQ_DPRIV_LOCKINIT(x)    spin_init(&(x)->lock)
 #define        FQ_GLOBAL_FQMP_LOCKINIT(x)      spin_init(&fq_fqmp_lock)
 
 
 #define        FQ_FQMP_LOCK(x)         fq_reference_mpriv((x)); \
                                spin_lock_wr(&(x)->lock)
-
+#if 0
 #define        FQ_FQP_LOCK(x)          fq_reference_priv((x)); \
                                spin_lock_wr(&(x)->lock)
+#endif
 
 #define        FQ_DPRIV_LOCK(x)        fq_reference_dpriv((x)); \
                                spin_lock_wr(&(x)->lock)
 #define        FQ_FQMP_UNLOCK(x)       spin_unlock_wr(&(x)->lock); \
                                fq_dereference_mpriv((x))
 
+#if 0
 #define        FQ_FQP_UNLOCK(x)        spin_unlock_wr(&(x)->lock); \
                                fq_dereference_priv((x))
+#endif
 
 #define        FQ_DPRIV_UNLOCK(x)      spin_unlock_wr(&(x)->lock); \
                                fq_dereference_dpriv((x))
@@ -106,8 +112,8 @@ struct dsched_fq_priv {
        TAILQ_ENTRY(dsched_fq_priv)     dlink;
        TAILQ_HEAD(, bio)       queue;
 
-       struct  spinlock        lock;
-       struct  disk            *dp;
+       struct lock             lock;
+       struct disk             *dp;
        struct dsched_fq_dpriv  *dpriv;
        struct dsched_fq_mpriv  *fqmp;
        struct proc             *p;
index b2d7434..23fe24f 100644 (file)
@@ -164,9 +164,6 @@ fq_dereference_priv(struct dsched_fq_priv *fqp)
 #endif
                dpriv = fqp->dpriv;
                KKASSERT(dpriv != NULL);
-
-               spin_lock_wr(&fqp->lock);
-
                KKASSERT(fqp->qlength == 0);
 
                if (fqp->flags & FQP_LINKED_DPRIV) {
@@ -190,8 +187,6 @@ fq_dereference_priv(struct dsched_fq_priv *fqp)
                        spin_unlock_wr(&fqmp->lock);
                }
 
-               spin_unlock_wr(&fqp->lock);
-
                objcache_put(fq_priv_cache, fqp);
                atomic_subtract_int(&fq_stats.fqp_allocations, 1);
 #if 0
@@ -217,7 +212,6 @@ fq_dereference_mpriv(struct dsched_fq_mpriv *fqmp)
                print_backtrace(8);
 #endif
                FQ_GLOBAL_FQMP_LOCK();
-               spin_lock_wr(&fqmp->lock);
 
                TAILQ_FOREACH_MUTABLE(fqp, &fqmp->fq_priv_list, link, fqp2) {
                        TAILQ_REMOVE(&fqmp->fq_priv_list, fqp, link);
@@ -226,7 +220,6 @@ fq_dereference_mpriv(struct dsched_fq_mpriv *fqmp)
                }
                TAILQ_REMOVE(&dsched_fqmp_list, fqmp, link);
 
-               spin_unlock_wr(&fqmp->lock);
                FQ_GLOBAL_FQMP_UNLOCK();
 
                objcache_put(fq_mpriv_cache, fqmp);
@@ -249,10 +242,13 @@ fq_alloc_priv(struct disk *dp, struct dsched_fq_mpriv *fqmp)
        fq_reference_priv(fqp);
 
        FQ_FQP_LOCKINIT(fqp);
-       FQ_FQP_LOCK(fqp);
        fqp->dp = dp;
 
        fqp->dpriv = dsched_get_disk_priv(dp);
+       TAILQ_INIT(&fqp->queue);
+
+       TAILQ_INSERT_TAIL(&fqp->dpriv->fq_priv_list, fqp, dlink);
+       fqp->flags |= FQP_LINKED_DPRIV;
 
        if (fqmp) {
                fqp->fqmp = fqmp;
@@ -265,12 +261,7 @@ fq_alloc_priv(struct disk *dp, struct dsched_fq_mpriv *fqmp)
                fqp->flags |= FQP_LINKED_FQMP;
        }
 
-       TAILQ_INIT(&fqp->queue);
-       TAILQ_INSERT_TAIL(&fqp->dpriv->fq_priv_list, fqp, dlink);
-       fqp->flags |= FQP_LINKED_DPRIV;
-
        atomic_add_int(&fq_stats.fqp_allocations, 1);
-       FQ_FQP_UNLOCK(fqp);
        return fqp;
 }