From aedbaf3ba5edf2f3c33496c3c542c8333943bca7 Mon Sep 17 00:00:00 2001 From: Alex Hornung Date: Sat, 17 Apr 2010 09:51:27 +0000 Subject: [PATCH] dsched_fq - Overhaul locking * 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 | 22 ++++++++++++++-------- sys/dsched/fq/dsched_fq_core.c | 17 ++++------------- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/sys/dsched/fq/dsched_fq.h b/sys/dsched/fq/dsched_fq.h index e8a27ea564..c825687280 100644 --- a/sys/dsched/fq/dsched_fq.h +++ b/sys/dsched/fq/dsched_fq.h @@ -48,23 +48,27 @@ #ifndef _SYS_SPINLOCK_H_ #include #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) @@ -75,8 +79,10 @@ #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; diff --git a/sys/dsched/fq/dsched_fq_core.c b/sys/dsched/fq/dsched_fq_core.c index b2d7434d71..23fe24f8c2 100644 --- a/sys/dsched/fq/dsched_fq_core.c +++ b/sys/dsched/fq/dsched_fq_core.c @@ -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; } -- 2.41.0