From cb5e5543e67afb6ee9670075531107884021da3f Mon Sep 17 00:00:00 2001

From: Jan Kara <jack@suse.cz>

Date: Fri, 4 Mar 2022 18:57:35 +0100

Subject: [PATCH 8/9] bfq: Get rid of __bio_blkcg() usage

References: bsc#1197926

Patch-mainline: submitted, in linux-block tree

BFQ usage of __bio_blkcg() is a relict from the past. Furthermore if bio

would not be associated with any blkcg, the usage of __bio_blkcg() in

BFQ is prone to races with the task being migrated between cgroups as

__bio_blkcg() calls at different places could return different blkcgs.

Convert BFQ to the new situation where bio->bi_blkg is initialized in

bio_set_dev() and thus practically always valid. This allows us to save

blkcg_gq lookup and noticeably simplify the code.

Cc: stable@vger.kernel.org

Fixes: 0fe061b9f03c ("blkcg: fix ref count issue with bio_blkcg() using task_css")

Tested-by: "yukuai (C)" <yukuai3@huawei.com>

Signed-off-by: Jan Kara <jack@suse.cz>

---

 block/bfq-cgroup.c  |   63 ++++++++++++++++++----------------------------------

 block/bfq-iosched.c |   10 --------

 block/bfq-iosched.h |    3 --

 3 files changed, 25 insertions(+), 51 deletions(-)

--- a/block/bfq-cgroup.c

+++ b/block/bfq-cgroup.c

@@ -582,27 +582,11 @@ static void bfq_group_set_parent(struct

 	entity->sched_data = &parent->sched_data;

 }

-static struct bfq_group *bfq_lookup_bfqg(struct bfq_data *bfqd,

-					 struct blkcg *blkcg)

+static void bfq_link_bfqg(struct bfq_data *bfqd, struct bfq_group *bfqg)

 {

-	struct blkcg_gq *blkg;

-

-	blkg = blkg_lookup(blkcg, bfqd->queue);

-	if (likely(blkg))

-		return blkg_to_bfqg(blkg);

-	return NULL;

-}

-

-struct bfq_group *bfq_find_set_group(struct bfq_data *bfqd,

-				     struct blkcg *blkcg)

-{

-	struct bfq_group *bfqg, *parent;

+	struct bfq_group *parent;

 	struct bfq_entity *entity;

-	bfqg = bfq_lookup_bfqg(bfqd, blkcg);

-	if (unlikely(!bfqg))

-		return NULL;

-

 	/*

 	 * Update chain of bfq_groups as we might be handling a leaf group

 	 * which, along with some of its relatives, has not been hooked yet

@@ -619,8 +603,15 @@ struct bfq_group *bfq_find_set_group(str

 			bfq_group_set_parent(curr_bfqg, parent);

 		}

 	}

+}

-	return bfqg;

+struct bfq_group *bfq_bio_bfqg(struct bfq_data *bfqd, struct bio *bio)

+{

+	struct blkcg_gq *blkg = bio->bi_blkg;

+

+	if (!blkg)

+		return bfqd->root_group;

+	return blkg_to_bfqg(blkg);

 }

 /**

@@ -690,25 +681,15 @@ void bfq_bfqq_move(struct bfq_data *bfqd

  * Move bic to blkcg, assuming that bfqd->lock is held; which makes

  * sure that the reference to cgroup is valid across the call (see

  * comments in bfq_bic_update_cgroup on this issue)

- *

- * NOTE: an alternative approach might have been to store the current

- * cgroup in bfqq and getting a reference to it, reducing the lookup

- * time here, at the price of slightly more complex code.

  */

-static struct bfq_group *__bfq_bic_change_cgroup(struct bfq_data *bfqd,

-						struct bfq_io_cq *bic,

-						struct blkcg *blkcg)

+static void *__bfq_bic_change_cgroup(struct bfq_data *bfqd,

+				     struct bfq_io_cq *bic,

+				     struct bfq_group *bfqg)

 {

 	struct bfq_queue *async_bfqq = bic_to_bfqq(bic, 0);

 	struct bfq_queue *sync_bfqq = bic_to_bfqq(bic, 1);

-	struct bfq_group *bfqg;

 	struct bfq_entity *entity;

-	bfqg = bfq_find_set_group(bfqd, blkcg);

-

-	if (unlikely(!bfqg))

-		bfqg = bfqd->root_group;

-

 	if (async_bfqq) {

 		entity = &async_bfqq->entity;

@@ -760,20 +741,24 @@ static struct bfq_group *__bfq_bic_chang

 void bfq_bic_update_cgroup(struct bfq_io_cq *bic, struct bio *bio)

 {

 	struct bfq_data *bfqd = bic_to_bfqd(bic);

-	struct bfq_group *bfqg = NULL;

+	struct bfq_group *bfqg = bfq_bio_bfqg(bfqd, bio);

 	uint64_t serial_nr;

-	rcu_read_lock();

-	serial_nr = __bio_blkcg(bio)->css.serial_nr;

+	serial_nr = bfqg_to_blkg(bfqg)->blkcg->css.serial_nr;

 	/*

 	 * Check whether blkcg has changed.  The condition may trigger

 	 * spuriously on a newly created cic but there's no harm.

 	 */

 	if (unlikely(!bfqd) || likely(bic->blkcg_serial_nr == serial_nr))

-		goto out;

+		return;

-	bfqg = __bfq_bic_change_cgroup(bfqd, bic, __bio_blkcg(bio));

+	/*

+	 * New cgroup for this process. Make sure it is linked to bfq internal

+	 * cgroup hierarchy.

+	 */

+	bfq_link_bfqg(bfqd, bfqg);

+	__bfq_bic_change_cgroup(bfqd, bic, bfqg);

 	/*

 	 * Update blkg_path for bfq_log_* functions. We cache this

 	 * path, and update it here, for the following

@@ -826,8 +811,6 @@ void bfq_bic_update_cgroup(struct bfq_io

 	 */

 	blkg_path(bfqg_to_blkg(bfqg), bfqg->blkg_path, sizeof(bfqg->blkg_path));

 	bic->blkcg_serial_nr = serial_nr;

-out:

-	rcu_read_unlock();

 }

 /**

@@ -1445,7 +1428,7 @@ void bfq_end_wr_async(struct bfq_data *b

 	bfq_end_wr_async_queues(bfqd, bfqd->root_group);

 }

-struct bfq_group *bfq_find_set_group(struct bfq_data *bfqd, struct blkcg *blkcg)

+struct bfq_group *bfq_bio_bfqg(struct bfq_data *bfqd, struct bio *bio)

 {

 	return bfqd->root_group;

 }

--- a/block/bfq-iosched.c

+++ b/block/bfq-iosched.c

@@ -5267,14 +5267,7 @@ static struct bfq_queue *bfq_get_queue(s

 	struct bfq_queue *bfqq;

 	struct bfq_group *bfqg;

-	rcu_read_lock();

-

-	bfqg = bfq_find_set_group(bfqd, __bio_blkcg(bio));

-	if (!bfqg) {

-		bfqq = &bfqd->oom_bfqq;

-		goto out;

-	}

-

+	bfqg = bfq_bio_bfqg(bfqd, bio);

 	if (!is_sync) {

 		async_bfqq = bfq_async_queue_prio(bfqd, bfqg, ioprio_class,

 						  ioprio);

@@ -5318,7 +5311,6 @@ static struct bfq_queue *bfq_get_queue(s

 out:

 	bfqq->ref++; /* get a process reference to this queue */

 	bfq_log_bfqq(bfqd, bfqq, "get_queue, at end: %p, %d", bfqq, bfqq->ref);

-	rcu_read_unlock();

 	return bfqq;

 }

--- a/block/bfq-iosched.h

+++ b/block/bfq-iosched.h

@@ -985,8 +985,7 @@ void bfq_bfqq_move(struct bfq_data *bfqd

 void bfq_init_entity(struct bfq_entity *entity, struct bfq_group *bfqg);

 void bfq_bic_update_cgroup(struct bfq_io_cq *bic, struct bio *bio);

 void bfq_end_wr_async(struct bfq_data *bfqd);

-struct bfq_group *bfq_find_set_group(struct bfq_data *bfqd,

-				     struct blkcg *blkcg);

+struct bfq_group *bfq_bio_bfqg(struct bfq_data *bfqd, struct bio *bio);

 struct blkcg_gq *bfqg_to_blkg(struct bfq_group *bfqg);

 struct bfq_group *bfqq_group(struct bfq_queue *bfqq);

 struct bfq_group *bfq_create_group_hierarchy(struct bfq_data *bfqd, int node);