[RFC PATCH v3 1/6] blk-cgroup: call pd_free_fn() outside spinlocks

[Yu Kuai]

From: Yu Kuai <yukuai@xxxxxxx>

blkcg_policy_teardown_pds() calls pd_free_fn() while holding both
q->queue_lock and blkcg->lock.  This is not safe for policies such as
iocost, whose ioc_pd_free() calls hrtimer_cancel().  On PREEMPT_RT the
hrtimer cancellation slow path can sleep while waiting for a soft hrtimer
callback to finish.

Keep the offline callback and policy data detachment protected by the
existing spinlocks, but tear down one policy data object at a time and drop
the locks before invoking pd_free_fn().  q->blkcg_mutex serializes the
operation against blkg_free_workfn(), so the associated blkg remains valid
while the callback runs.

Fixes: 7caa47151ab2 ("blkcg: implement blk-iocost")
Signed-off-by: Yu Kuai <yukuai@xxxxxxx>
---
 block/blk-cgroup.c | 37 +++++++++++++++++++++++++++----------
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 1bd91223367c..5b51be2fefc1 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -1548,33 +1548,51 @@ struct cgroup_subsys io_cgrp_subsys = {
 	.depends_on = 1 << memory_cgrp_id,
 #endif
 };
 EXPORT_SYMBOL_GPL(io_cgrp_subsys);
 
-/*
- * 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)
+static struct blkg_policy_data *
+blkcg_policy_detach_pd(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);
 		pd = blkg->pd[pol->plid];
 		if (pd) {
 			if (pd->online && pol->pd_offline_fn)
 				pol->pd_offline_fn(pd);
 			pd->online = false;
-			pol->pd_free_fn(pd);
 			WRITE_ONCE(blkg->pd[pol->plid], NULL);
 		}
 		spin_unlock(&blkcg->lock);
+
+		if (pd)
+			break;
 	}
+	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
@@ -1687,13 +1705,11 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
 		pol->pd_free_fn(pd_prealloc);
 	return ret;
 
 enomem:
 	/* alloc failed, take down everything */
-	spin_lock_irq(&q->queue_lock);
 	blkcg_policy_teardown_pds(q, pol);
-	spin_unlock_irq(&q->queue_lock);
 	ret = -ENOMEM;
 	goto out;
 }
 EXPORT_SYMBOL_GPL(blkcg_activate_policy);
 
@@ -1719,12 +1735,13 @@ void blkcg_deactivate_policy(struct gendisk *disk,
 
 	mutex_lock(&q->blkcg_mutex);
 	spin_lock_irq(&q->queue_lock);
 
 	__clear_bit(pol->plid, q->blkcg_pols);
-	blkcg_policy_teardown_pds(q, pol);
 	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);
 }
-- 
2.51.0