[RFC PATCH v3 3/6] blk-cgroup: protect blkgs with blkcg_mutex

From: Yu Kuai <[email protected]>

queue_lock is still needed by block core users, but blkcg no longer
needs it for blkg topology now that throttle runtime state has a private
lock.

Move queue-local blkg topology synchronization to q->blkcg_mutex. Hold
it while creating and destroying blkgs, while preparing and undoing
configuration, and while activating or deactivating policies.

Keep the common bio_blkg() lookup on an RCU fast path so I/O for an
existing blkg does not acquire blkcg_mutex. Only take the mutex when the
blkg hierarchy needs to be created.

Update the BFQ, iocost, iolatency and throttle paths which walk
q->blkg_list or access per-blkg policy state to use the same lock.

blkcg->lock still protects blkcg-local list updates. Some lookups under
blkcg_mutex can race with blkcg updates done for other queues, so keep
those lookups in RCU read-side critical sections. In particular, protect
the parent lookup in blkg_create() and the parent walk in
blkg_lookup_create().

Signed-off-by: Yu Kuai <[email protected]>
---
 block/bfq-cgroup.c    |  10 ++-
 block/blk-cgroup.c    | 153 +++++++++++++++++-------------------------
 block/blk-cgroup.h    |  11 ++-
 block/blk-iocost.c    |   8 ++-
 block/blk-iolatency.c |   7 +-
 block/blk-throttle.c  |  10 +--
 6 files changed, 87 insertions(+), 112 deletions(-)

diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
index 4a3975f9ff74..d64cea475d7b 100644
--- a/block/bfq-cgroup.c
+++ b/block/bfq-cgroup.c
@@ -426,11 +426,11 @@ static void bfqg_stats_xfer_dead(struct bfq_group *bfqg)
 	if (!bfqg) /* root_group */
 		return;
 
 	parent = bfqg_parent(bfqg);
 
-	lockdep_assert_held(&bfqg_to_blkg(bfqg)->q->queue_lock);
+	lockdep_assert_held(&bfqg_to_blkg(bfqg)->q->blkcg_mutex);
 
 	if (unlikely(!parent))
 		return;
 
 	bfqg_stats_add_aux(&parent->stats, &bfqg->stats);
@@ -876,11 +876,11 @@ static void bfq_reparent_active_queues(struct bfq_data *bfqd,
 /**
  * bfq_pd_offline - deactivate the entity associated with @pd,
  *		    and reparent its children entities.
  * @pd: descriptor of the policy going offline.
  *
- * blkio already grabs the queue_lock for us, so no need to use
+ * blkio already grabs the blkcg_mutex for us, so no need to use
  * RCU-based magic
  */
 static void bfq_pd_offline(struct blkg_policy_data *pd)
 {
 	struct bfq_service_tree *st;
@@ -949,22 +949,20 @@ void bfq_end_wr_async(struct bfq_data *bfqd)
 {
 	struct request_queue *q = bfqd->queue;
 	struct blkcg_gq *blkg;
 
 	mutex_lock(&q->blkcg_mutex);
-	spin_lock_irq(&q->queue_lock);
-	spin_lock(&bfqd->lock);
+	spin_lock_irq(&bfqd->lock);
 
 	list_for_each_entry(blkg, &q->blkg_list, q_node) {
 		struct bfq_group *bfqg = blkg_to_bfqg(blkg);
 
 		bfq_end_wr_async_queues(bfqd, bfqg);
 	}
 	bfq_end_wr_async_queues(bfqd, bfqd->root_group);
 
-	spin_unlock(&bfqd->lock);
-	spin_unlock_irq(&q->queue_lock);
+	spin_unlock_irq(&bfqd->lock);
 	mutex_unlock(&q->blkcg_mutex);
 }
 
 static int bfq_io_show_weight_legacy(struct seq_file *sf, void *v)
 {
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 5b51be2fefc1..0f34a80a726d 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -62,12 +62,10 @@ static LIST_HEAD(all_blkcgs);		/* protected by blkcg_pol_mutex */
 
 bool blkcg_debug_stats = false;
 
 static DEFINE_RAW_SPINLOCK(blkg_stat_lock);
 
-#define BLKG_DESTROY_BATCH_SIZE  64
-
 const struct rhashtable_params blkg_hash_params = {
 	.key_len		= sizeof_field(struct blkcg_gq, blkcg_id),
 	.key_offset		= offsetof(struct blkcg_gq, blkcg_id),
 	.head_offset		= offsetof(struct blkcg_gq, q_hash_node),
 	.automatic_shrinking	= true,
@@ -139,15 +137,13 @@ static void blkg_free_workfn(struct work_struct *work)
 	for (i = 0; i < BLKCG_MAX_POLS; i++)
 		if (blkg->pd[i])
 			blkcg_policy[i]->pd_free_fn(blkg->pd[i]);
 	if (blkg->parent)
 		blkg_put(blkg->parent);
-	spin_lock_irq(&q->queue_lock);
 	list_del_init(&blkg->q_node);
 	if (list_empty(&q->blkg_list))
 		wake_up_var(&q->blkg_list);
-	spin_unlock_irq(&q->queue_lock);
 	mutex_unlock(&q->blkcg_mutex);
 
 	/*
 	 * Release blkcg css ref only after blkg is removed from q->blkg_list,
 	 * so concurrent iterators won't see a blkg with a freed blkcg.
@@ -397,11 +393,11 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg, struct gendisk *disk,
 				    struct blkcg_gq *new_blkg)
 {
 	struct blkcg_gq *blkg;
 	int i, ret;
 
-	lockdep_assert_held(&disk->queue->queue_lock);
+	lockdep_assert_held(&disk->queue->blkcg_mutex);
 
 	/* request_queue is dying, do not create/recreate a blkg */
 	if (blk_queue_dying(disk->queue)) {
 		ret = -ENODEV;
 		goto err_free_blkg;
@@ -417,16 +413,19 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg, struct gendisk *disk,
 	}
 	blkg = new_blkg;
 
 	/* link parent */
 	if (blkcg_parent(blkcg)) {
+		rcu_read_lock();
 		blkg->parent = blkg_lookup(blkcg_parent(blkcg), disk->queue);
 		if (WARN_ON_ONCE(!blkg->parent)) {
+			rcu_read_unlock();
 			ret = -ENODEV;
 			goto err_free_blkg;
 		}
 		blkg_get(blkg->parent);
+		rcu_read_unlock();
 	}
 
 	/* invoke per-policy init */
 	for (i = 0; i < BLKCG_MAX_POLS; i++) {
 		struct blkcg_policy *pol = blkcg_policy[i];
@@ -434,11 +433,11 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg, struct gendisk *disk,
 		if (blkg->pd[i] && pol->pd_init_fn)
 			pol->pd_init_fn(blkg->pd[i]);
 	}
 
 	/* insert */
-	spin_lock(&blkcg->lock);
+	spin_lock_irq(&blkcg->lock);
 	ret = rhashtable_insert_fast(&disk->queue->blkg_hash,
 				     &blkg->q_hash_node, blkg_hash_params);
 	if (likely(!ret)) {
 		hlist_add_head_rcu(&blkg->blkcg_node, &blkcg->blkg_list);
 		list_add(&blkg->q_node, &disk->queue->blkg_list);
@@ -452,11 +451,11 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg, struct gendisk *disk,
 				blkg->pd[i]->online = true;
 			}
 		}
 		blkg->online = true;
 	}
-	spin_unlock(&blkcg->lock);
+	spin_unlock_irq(&blkcg->lock);
 
 	if (!ret)
 		return blkg;
 
 	/* @blkg failed fully initialized, use the usual release path */
@@ -485,13 +484,12 @@ static struct blkcg_gq *blkg_lookup_tryget(struct blkcg_gq *blkg)
  * @blkcg: blkcg of interest
  * @disk: gendisk of interest
  *
  * Lookup blkg for the @blkcg - @disk pair.  If it doesn't exist, try to
  * create one.  blkg creation is performed recursively from blkcg_root such
- * that all non-root blkg's have access to the parent blkg.
- *
- * Must be called with @disk->queue->queue_lock held.
+ * that all non-root blkg's have access to the parent blkg.  This function
+ * must be called with @disk->queue->blkcg_mutex held.
  *
  * Returns the closest blkg with an extra reference acquired.  If
  * blkg_create() fails while walking down from root, the returned blkg may
  * belong to an ancestor of @blkcg.  This function never returns %NULL.
  */
@@ -499,11 +497,11 @@ static struct blkcg_gq *blkg_lookup_create(struct blkcg *blkcg,
 		struct gendisk *disk)
 {
 	struct request_queue *q = disk->queue;
 	struct blkcg_gq *blkg;
 
-	lockdep_assert_held(&q->queue_lock);
+	lockdep_assert_held(&q->blkcg_mutex);
 
 	rcu_read_lock();
 	blkg = blkg_lookup(blkcg, q);
 	if (blkg) {
 		blkg = blkg_lookup_tryget(blkg);
@@ -520,20 +518,22 @@ static struct blkcg_gq *blkg_lookup_create(struct blkcg *blkcg,
 	while (true) {
 		struct blkcg *pos = blkcg;
 		struct blkcg *parent = blkcg_parent(blkcg);
 		struct blkcg_gq *ret_blkg = q->root_blkg;
 
+		rcu_read_lock();
 		while (parent) {
 			blkg = blkg_lookup(parent, q);
 			if (blkg) {
 				/* remember closest blkg */
 				ret_blkg = blkg;
 				break;
 			}
 			pos = parent;
 			parent = blkcg_parent(parent);
 		}
+		rcu_read_unlock();
 
 		blkg = blkg_create(pos, disk, NULL);
 		if (IS_ERR(blkg)) {
 			blkg = ret_blkg;
 			break;
@@ -548,11 +548,11 @@ static struct blkcg_gq *blkg_lookup_create(struct blkcg *blkcg,
 static void blkg_destroy(struct blkcg_gq *blkg)
 {
 	struct blkcg *blkcg = blkg->blkcg;
 	int i;
 
-	lockdep_assert_held(&blkg->q->queue_lock);
+	lockdep_assert_held(&blkg->q->blkcg_mutex);
 	lockdep_assert_held(&blkcg->lock);
 
 	/*
 	 * blkg stays on the queue list until blkg_free_workfn(), see details in
 	 * blkg_free_workfn(), hence this function can be called from
@@ -585,37 +585,22 @@ static void blkg_destroy(struct blkcg_gq *blkg)
 
 static void blkg_destroy_all(struct gendisk *disk)
 {
 	struct request_queue *q = disk->queue;
 	struct blkcg_gq *blkg;
-	int count = BLKG_DESTROY_BATCH_SIZE;
 	int i;
 
-restart:
 	mutex_lock(&q->blkcg_mutex);
-	spin_lock_irq(&q->queue_lock);
 	list_for_each_entry(blkg, &q->blkg_list, q_node) {
 		struct blkcg *blkcg = blkg->blkcg;
 
 		if (hlist_unhashed(&blkg->blkcg_node))
 			continue;
 
-		spin_lock(&blkcg->lock);
+		spin_lock_irq(&blkcg->lock);
 		blkg_destroy(blkg);
-		spin_unlock(&blkcg->lock);
-
-		/*
-		 * in order to avoid holding the spin lock for too long, release
-		 * it when a batch of blkgs are destroyed.
-		 */
-		if (!(--count)) {
-			count = BLKG_DESTROY_BATCH_SIZE;
-			spin_unlock_irq(&q->queue_lock);
-			mutex_unlock(&q->blkcg_mutex);
-			cond_resched();
-			goto restart;
-		}
+		spin_unlock_irq(&blkcg->lock);
 	}
 
 	/*
 	 * Mark policy deactivated since policy offline has been done, and
 	 * the free is scheduled, so future blkcg_deactivate_policy() can
@@ -627,11 +612,10 @@ static void blkg_destroy_all(struct gendisk *disk)
 		if (pol)
 			__clear_bit(pol->plid, q->blkcg_pols);
 	}
 
 	q->root_blkg = NULL;
-	spin_unlock_irq(&q->queue_lock);
 	mutex_unlock(&q->blkcg_mutex);
 }
 
 static void blkg_iostat_set(struct blkg_iostat *dst, struct blkg_iostat *src)
 {
@@ -843,12 +827,12 @@ EXPORT_SYMBOL_GPL(blkg_conf_open_bdev);
  * accordingly. On success, @ctx->body points to the part of @ctx->input
  * following MAJ:MIN, @ctx->bdev points to the target block device and
  * @ctx->blkg to the blkg being configured.
  *
  * blkg_conf_open_bdev() must be called on @ctx beforehand. On success, this
- * function returns with queue lock held and must be followed by
- * blkg_conf_close_bdev().
+ * function returns with blkcg_mutex held and must be followed by
+ * blkg_conf_unprep().
  */
 int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
 		   struct blkg_conf_ctx *ctx)
 {
 	struct gendisk *disk;
@@ -862,18 +846,19 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
 	disk = ctx->bdev->bd_disk;
 	q = disk->queue;
 
 	/* Prevent concurrent with blkcg_deactivate_policy() */
 	mutex_lock(&q->blkcg_mutex);
-	spin_lock_irq(&q->queue_lock);
 
 	if (!blkcg_policy_enabled(q, pol)) {
 		ret = -EOPNOTSUPP;
 		goto fail_unlock;
 	}
 
+	rcu_read_lock();
 	blkg = blkg_lookup(blkcg, q);
+	rcu_read_unlock();
 	if (blkg)
 		goto success;
 
 	/*
 	 * Create blkgs walking down from blkcg_root to @blkcg, so that all
@@ -883,33 +868,32 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
 		struct blkcg *pos = blkcg;
 		struct blkcg *parent;
 		struct blkcg_gq *new_blkg;
 
 		parent = blkcg_parent(blkcg);
+		rcu_read_lock();
 		while (parent && !blkg_lookup(parent, q)) {
 			pos = parent;
 			parent = blkcg_parent(parent);
 		}
-
-		/* Drop locks to do new blkg allocation with GFP_KERNEL. */
-		spin_unlock_irq(&q->queue_lock);
+		rcu_read_unlock();
 
 		new_blkg = blkg_alloc(pos, disk, GFP_NOIO);
 		if (unlikely(!new_blkg)) {
 			ret = -ENOMEM;
-			goto fail_exit;
+			goto fail_unlock;
 		}
 
-		spin_lock_irq(&q->queue_lock);
-
 		if (!blkcg_policy_enabled(q, pol)) {
 			blkg_free(new_blkg);
 			ret = -EOPNOTSUPP;
 			goto fail_unlock;
 		}
 
+		rcu_read_lock();
 		blkg = blkg_lookup(pos, q);
+		rcu_read_unlock();
 		if (blkg) {
 			blkg_free(new_blkg);
 		} else {
 			blkg = blkg_create(pos, disk, new_blkg);
 			if (IS_ERR(blkg)) {
@@ -920,17 +904,14 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
 
 		if (pos == blkcg)
 			goto success;
 	}
 success:
-	mutex_unlock(&q->blkcg_mutex);
 	ctx->blkg = blkg;
 	return 0;
 
 fail_unlock:
-	spin_unlock_irq(&q->queue_lock);
-fail_exit:
 	mutex_unlock(&q->blkcg_mutex);
 	/*
 	 * If queue was bypassing, we should retry.  Do so after a
 	 * short msleep().  It isn't strictly necessary but queue
 	 * can be bypassing for some time and it's always nice to
@@ -949,11 +930,11 @@ EXPORT_SYMBOL_GPL(blkg_conf_prep);
  * @ctx: blkg_conf_ctx initialized with blkg_conf_init()
  */
 void blkg_conf_unprep(struct blkg_conf_ctx *ctx)
 {
 	WARN_ON_ONCE(!ctx->blkg);
-	spin_unlock_irq(&ctx->bdev->bd_disk->queue->queue_lock);
+	mutex_unlock(&ctx->bdev->bd_disk->queue->blkcg_mutex);
 	ctx->blkg = NULL;
 }
 EXPORT_SYMBOL_GPL(blkg_conf_unprep);
 
 /**
@@ -1269,12 +1250,13 @@ static struct blkcg_gq *blkcg_get_first_blkg(struct blkcg *blkcg)
 
 /**
  * blkcg_destroy_blkgs - responsible for shooting down blkgs
  * @blkcg: blkcg of interest
  *
- * blkgs should be removed while holding both q and blkcg locks.  As blkcg lock
- * is nested inside q lock, this function performs reverse double lock dancing.
+ * blkgs should be removed while holding both q->blkcg_mutex and blkcg->lock.
+ * As blkcg->lock is nested inside q->blkcg_mutex, this function performs
+ * reverse double lock dancing.
  * Destroying the blkgs releases the reference held on the blkcg's css allowing
  * blkcg_css_free to eventually be called.
  *
  * This is the blkcg counterpart of ioc_release_fn().
  */
@@ -1285,17 +1267,17 @@ static void blkcg_destroy_blkgs(struct blkcg *blkcg)
 	might_sleep();
 
 	while ((blkg = blkcg_get_first_blkg(blkcg))) {
 		struct request_queue *q = blkg->q;
 
-		spin_lock_irq(&q->queue_lock);
-		spin_lock(&blkcg->lock);
+		mutex_lock(&q->blkcg_mutex);
+		spin_lock_irq(&blkcg->lock);
 
 		blkg_destroy(blkg);
 
-		spin_unlock(&blkcg->lock);
-		spin_unlock_irq(&q->queue_lock);
+		spin_unlock_irq(&blkcg->lock);
+		mutex_unlock(&q->blkcg_mutex);
 
 		blkg_put(blkg);
 		cond_resched();
 	}
 }
@@ -1499,22 +1481,21 @@ int blkcg_init_disk(struct gendisk *disk)
 	new_blkg = blkg_alloc(&blkcg_root, disk, GFP_KERNEL);
 	if (!new_blkg)
 		return -ENOMEM;
 
 	/* Make sure the root blkg exists. */
-	/* spin_lock_irq can serve as RCU read-side critical section. */
-	spin_lock_irq(&q->queue_lock);
+	mutex_lock(&q->blkcg_mutex);
 	blkg = blkg_create(&blkcg_root, disk, new_blkg);
 	if (IS_ERR(blkg))
 		goto err_unlock;
 	q->root_blkg = blkg;
-	spin_unlock_irq(&q->queue_lock);
+	mutex_unlock(&q->blkcg_mutex);
 
 	return 0;
 
 err_unlock:
-	spin_unlock_irq(&q->queue_lock);
+	mutex_unlock(&q->blkcg_mutex);
 	return PTR_ERR(blkg);
 }
 
 void blkcg_exit_disk(struct gendisk *disk)
 {
@@ -1548,51 +1529,37 @@ struct cgroup_subsys io_cgrp_subsys = {
 	.depends_on = 1 << memory_cgrp_id,
 #endif
 };
 EXPORT_SYMBOL_GPL(io_cgrp_subsys);
 
-static struct blkg_policy_data *
-blkcg_policy_detach_pd(struct request_queue *q,
-		       const struct blkcg_policy *pol)
+/*
+ * Tear down per-blkg policy data for @pol on @q.
+ */
+static void blkcg_policy_teardown_pds(struct request_queue *q,
+				      const struct blkcg_policy *pol)
 {
-	struct blkg_policy_data *pd = NULL;
 	struct blkcg_gq *blkg;
 
 	lockdep_assert_held(&q->blkcg_mutex);
 
-	spin_lock_irq(&q->queue_lock);
 	list_for_each_entry(blkg, &q->blkg_list, q_node) {
 		struct blkcg *blkcg = blkg->blkcg;
+		struct blkg_policy_data *pd;
 
-		spin_lock(&blkcg->lock);
+		spin_lock_irq(&blkcg->lock);
 		pd = blkg->pd[pol->plid];
 		if (pd) {
 			if (pd->online && pol->pd_offline_fn)
 				pol->pd_offline_fn(pd);
 			pd->online = false;
 			WRITE_ONCE(blkg->pd[pol->plid], NULL);
 		}
-		spin_unlock(&blkcg->lock);
+		spin_unlock_irq(&blkcg->lock);
 
 		if (pd)
-			break;
+			pol->pd_free_fn(pd);
 	}
-	spin_unlock_irq(&q->queue_lock);
-
-	return pd;
-}
-
-/*
- * Tear down per-blkg policy data for @pol on @q.
- */
-static void blkcg_policy_teardown_pds(struct request_queue *q,
-				      const struct blkcg_policy *pol)
-{
-	struct blkg_policy_data *pd;
-
-	while ((pd = blkcg_policy_detach_pd(q, pol)))
-		pol->pd_free_fn(pd);
 }
 
 /**
  * blkcg_activate_policy - activate a blkcg policy on a gendisk
  * @disk: gendisk of interest
@@ -1600,13 +1567,13 @@ static void blkcg_policy_teardown_pds(struct request_queue *q,
  *
  * Activate @pol on @disk.  Requires %GFP_KERNEL context.  @disk goes through
  * bypass mode to populate its blkgs with policy_data for @pol.
  *
  * Activation happens with @disk bypassed, so nobody would be accessing blkgs
- * from IO path.  Update of each blkg is protected by both queue and blkcg
- * locks so that holding either lock and testing blkcg_policy_enabled() is
- * always enough for dereferencing policy data.
+ * from IO path.  Update of each blkg is protected by q->blkcg_mutex and
+ * blkcg->lock so that holding either lock and testing blkcg_policy_enabled()
+ * is always enough for dereferencing policy data.
  *
  * The caller is responsible for synchronizing [de]activations and policy
  * [un]registerations.  Returns 0 on success, -errno on failure.
  */
 int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
@@ -1631,12 +1598,10 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
 	if (queue_is_mq(q))
 		memflags = blk_mq_freeze_queue(q);
 
 	mutex_lock(&q->blkcg_mutex);
 retry:
-	spin_lock_irq(&q->queue_lock);
-
 	/* blkg_list is pushed at the head, reverse walk to initialize parents first */
 	list_for_each_entry_reverse(blkg, &q->blkg_list, q_node) {
 		struct blkg_policy_data *pd;
 
 		if (blkg->pd[pol->plid])
@@ -1661,23 +1626,24 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
 			if (pinned_blkg)
 				blkg_put(pinned_blkg);
 			blkg_get(blkg);
 			pinned_blkg = blkg;
 
-			spin_unlock_irq(&q->queue_lock);
+			mutex_unlock(&q->blkcg_mutex);
 
 			if (pd_prealloc)
 				pol->pd_free_fn(pd_prealloc);
 			pd_prealloc = pol->pd_alloc_fn(disk, blkg->blkcg,
 						       GFP_KERNEL);
+			mutex_lock(&q->blkcg_mutex);
 			if (pd_prealloc)
 				goto retry;
 			else
 				goto enomem;
 		}
 
-		spin_lock(&blkg->blkcg->lock);
+		spin_lock_irq(&blkg->blkcg->lock);
 
 		pd->blkg = blkg;
 		pd->plid = pol->plid;
 		WRITE_ONCE(blkg->pd[pol->plid], pd);
 
@@ -1686,17 +1652,16 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
 
 		if (pol->pd_online_fn)
 			pol->pd_online_fn(pd);
 		pd->online = true;
 
-		spin_unlock(&blkg->blkcg->lock);
+		spin_unlock_irq(&blkg->blkcg->lock);
 	}
 
 	__set_bit(pol->plid, q->blkcg_pols);
 	ret = 0;
 
-	spin_unlock_irq(&q->queue_lock);
 out:
 	mutex_unlock(&q->blkcg_mutex);
 	if (queue_is_mq(q))
 		blk_mq_unfreeze_queue(q, memflags);
 	if (pinned_blkg)
@@ -1732,15 +1697,12 @@ void blkcg_deactivate_policy(struct gendisk *disk,
 
 	if (queue_is_mq(q))
 		memflags = blk_mq_freeze_queue(q);
 
 	mutex_lock(&q->blkcg_mutex);
-	spin_lock_irq(&q->queue_lock);
 
 	__clear_bit(pol->plid, q->blkcg_pols);
-	spin_unlock_irq(&q->queue_lock);
-
 	blkcg_policy_teardown_pds(q, pol);
 	mutex_unlock(&q->blkcg_mutex);
 
 	if (queue_is_mq(q))
 		blk_mq_unfreeze_queue(q, memflags);
@@ -2162,23 +2124,34 @@ struct blkcg_gq *bio_blkg(struct bio *bio)
 {
 	struct blkcg *blkcg = bio_blkcg(bio);
 	struct gendisk *disk;
 	struct request_queue *q;
 	struct blkcg_gq *blkg;
+	int ret;
 
 	if (!blkcg || !bio->bi_bdev)
 		return NULL;
 
 	if (bio_flagged(bio, BIO_BLKG_REF))
 		return bio_pinned_blkg(bio);
 
 	disk = bio->bi_bdev->bd_disk;
 	q = disk->queue;
 
-	spin_lock_irq(&q->queue_lock);
+	rcu_read_lock();
+	blkg = blkg_lookup(blkcg, q);
+	if (blkg)
+		blkg = blkg_lookup_tryget(blkg);
+	rcu_read_unlock();
+	if (blkg) {
+		bio_set_blkg_ref(bio, blkg);
+		return blkg;
+	}
+
+	mutex_lock(&q->blkcg_mutex);
 	blkg = blkg_lookup_create(blkcg, disk);
-	spin_unlock_irq(&q->queue_lock);
+	mutex_unlock(&q->blkcg_mutex);
 
 	bio_set_blkg_ref(bio, blkg);
 	return blkg;
 }
 EXPORT_SYMBOL_GPL(bio_blkg);
diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h
index 1925420154c1..dcba4eda9826 100644
--- a/block/blk-cgroup.h
+++ b/block/blk-cgroup.h
@@ -68,11 +68,11 @@ struct blkcg_gq {
 	struct blkcg_gq			*parent;
 
 	/* reference count */
 	struct percpu_ref		refcnt;
 
-	/* is this blkg online? protected by both blkcg and q locks */
+	/* is this blkg online? protected by blkcg->lock and q->blkcg_mutex */
 	bool				online;
 
 	struct blkg_iostat_set __percpu	*iostat_cpu;
 	struct blkg_iostat_set		iostat;
 
@@ -229,13 +229,13 @@ struct blkg_conf_ctx {
 void blkg_conf_init(struct blkg_conf_ctx *ctx, char *input);
 int blkg_conf_open_bdev(struct blkg_conf_ctx *ctx)
 	__cond_acquires(0, &ctx->bdev->bd_queue->rq_qos_mutex);
 int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
 		   struct blkg_conf_ctx *ctx)
-	__cond_acquires(0, &ctx->bdev->bd_disk->queue->queue_lock);
+	__cond_acquires(0, &ctx->bdev->bd_disk->queue->blkcg_mutex);
 void blkg_conf_unprep(struct blkg_conf_ctx *ctx)
-	__releases(ctx->bdev->bd_disk->queue->queue_lock);
+	__releases(ctx->bdev->bd_disk->queue->blkcg_mutex);
 void blkg_conf_close_bdev(struct blkg_conf_ctx *ctx)
 	__releases(&ctx->bdev->bd_queue->rq_qos_mutex);
 
 /**
  * bio_issue_as_root_blkg - see if this bio needs to be issued as root blkg
@@ -388,13 +388,12 @@ static inline void bio_clear_blkcg(struct bio *bio)
  * @d_blkg: loop cursor pointing to the current descendant
  * @pos_css: used for iteration
  * @p_blkg: target blkg to walk descendants of
  *
  * Walk @c_blkg through the descendants of @p_blkg.  Must be used with RCU
- * read locked.  If called under either blkcg or queue lock, the iteration
- * is guaranteed to include all and only online blkgs.  The caller may
- * update @pos_css by calling css_rightmost_descendant() to skip subtree.
+ * read locked.  The caller may update @pos_css by calling
+ * css_rightmost_descendant() to skip subtree.
  * @p_blkg is included in the iteration and the first node to be visited.
  */
 #define blkg_for_each_descendant_pre(d_blkg, pos_css, p_blkg)		\
 	css_for_each_descendant_pre((pos_css), &(p_blkg)->blkcg->css)	\
 		if (((d_blkg) = blkg_lookup(css_to_blkcg(pos_css),	\
diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index 57f2b4d4af20..31419add4340 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -2773,11 +2773,12 @@ static void ioc_rqos_throttle(struct rq_qos *rqos, struct bio *bio)
 }
 
 static void ioc_rqos_merge(struct rq_qos *rqos, struct request *rq,
 			   struct bio *bio)
 {
-	struct ioc_gq *iocg = blkg_to_iocg(bio_blkg(bio));
+	struct blkcg_gq *blkg = bio_blkg_lookup(rq->bio);
+	struct ioc_gq *iocg = blkg_to_iocg(blkg);
 	struct ioc *ioc = rqos_to_ioc(rqos);
 	sector_t bio_end = bio_end_sector(bio);
 	struct ioc_now now;
 	u64 vtime, abs_cost, cost;
 	unsigned long flags;
@@ -3152,10 +3153,11 @@ static ssize_t ioc_weight_write(struct kernfs_open_file *of, char *buf,
 	struct blkcg *blkcg = css_to_blkcg(of_css(of));
 	struct ioc_cgrp *iocc = blkcg_to_iocc(blkcg);
 	struct blkg_conf_ctx ctx;
 	struct ioc_now now;
 	struct ioc_gq *iocg;
+	unsigned long flags;
 	u32 v;
 	int ret;
 
 	if (!strchr(buf, ':')) {
 		struct blkcg_gq *blkg;
@@ -3204,15 +3206,15 @@ static ssize_t ioc_weight_write(struct kernfs_open_file *of, char *buf,
 			goto unprep;
 		if (v < CGROUP_WEIGHT_MIN || v > CGROUP_WEIGHT_MAX)
 			goto unprep;
 	}
 
-	spin_lock(&iocg->ioc->lock);
+	spin_lock_irqsave(&iocg->ioc->lock, flags);
 	iocg->cfg_weight = v * WEIGHT_ONE;
 	ioc_now(iocg->ioc, &now);
 	weight_updated(iocg, &now);
-	spin_unlock(&iocg->ioc->lock);
+	spin_unlock_irqrestore(&iocg->ioc->lock, flags);
 
 	ret = 0;
 
 unprep:
 	blkg_conf_unprep(&ctx);
diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c
index 7220edafd72b..b4bed73b645b 100644
--- a/block/blk-iolatency.c
+++ b/block/blk-iolatency.c
@@ -640,10 +640,11 @@ static void blkcg_iolatency_exit(struct rq_qos *rqos)
 	struct blk_iolatency *blkiolat = BLKIOLATENCY(rqos);
 
 	timer_shutdown_sync(&blkiolat->timer);
 	flush_work(&blkiolat->enable_work);
 	blkcg_deactivate_policy(rqos->disk, &blkcg_policy_iolatency);
+	flush_work(&blkiolat->enable_work);
 	kfree(blkiolat);
 }
 
 static const struct rq_qos_ops blkcg_iolatency_ops = {
 	.throttle = blkcg_iolatency_throttle,
@@ -812,20 +813,22 @@ static void iolatency_set_min_lat_nsec(struct blkcg_gq *blkg, u64 val)
 static void iolatency_clear_scaling(struct blkcg_gq *blkg)
 {
 	if (blkg->parent) {
 		struct iolatency_grp *iolat = blkg_to_lat(blkg->parent);
 		struct child_latency_info *lat_info;
+		unsigned long flags;
+
 		if (!iolat)
 			return;
 
 		lat_info = &iolat->child_lat;
-		spin_lock(&lat_info->lock);
+		spin_lock_irqsave(&lat_info->lock, flags);
 		atomic_set(&lat_info->scale_cookie, DEFAULT_SCALE_COOKIE);
 		lat_info->last_scale_event = 0;
 		lat_info->scale_grp = NULL;
 		lat_info->scale_lat = 0;
-		spin_unlock(&lat_info->lock);
+		spin_unlock_irqrestore(&lat_info->lock, flags);
 	}
 }
 
 static ssize_t iolatency_set_limit(struct kernfs_open_file *of, char *buf,
 			     size_t nbytes, loff_t off)
diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 2ff30700e84e..045eeab38646 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -1775,14 +1775,14 @@ void blk_throtl_cancel_bios(struct gendisk *disk)
 	int rw;
 
 	if (!blk_throtl_activated(q))
 		return;
 
-	spin_lock_irq(&q->queue_lock);
-	spin_lock(&td->lock);
+	mutex_lock(&q->blkcg_mutex);
+	spin_lock_irq(&td->lock);
 	/*
-	 * queue_lock is held, rcu lock is not needed here technically.
+	 * blkcg_mutex is held, rcu lock is not needed here technically.
 	 * However, rcu lock is still held to emphasize that following
 	 * path need RCU protection and to prevent warning from lockdep.
 	 */
 	rcu_read_lock();
 	blkg_for_each_descendant_post(blkg, pos_css, q->root_blkg) {
@@ -1795,12 +1795,12 @@ void blk_throtl_cancel_bios(struct gendisk *disk)
 		 * del_gendisk.
 		 */
 		tg_cancel_writeback_bios(blkg_to_tg(blkg), cancel_bios);
 	}
 	rcu_read_unlock();
-	spin_unlock(&td->lock);
-	spin_unlock_irq(&q->queue_lock);
+	spin_unlock_irq(&td->lock);
+	mutex_unlock(&q->blkcg_mutex);
 
 	for (rw = READ; rw <= WRITE; rw++) {
 		struct bio *bio;
 		while ((bio = bio_list_pop(&cancel_bios[rw])))
 			bio_io_error(bio);
-- 
2.51.0