Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 8afc13b9 authored by Jan Kara's avatar Jan Kara Committed by Greg Kroah-Hartman
Browse files

bfq: Get rid of __bio_blkcg() usage 



commit 4e54a2493e582361adc3bfbf06c7d50d19d18837 upstream.

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: 0fe061b9 ("blkcg: fix ref count issue with bio_blkcg() using task_css")
Tested-by: default avatar"yukuai (C)" <yukuai3@huawei.com>
Signed-off-by: default avatarJan Kara <jack@suse.cz>
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Link: https://lore.kernel.org/r/20220401102752.8599-8-jack@suse.cz


Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent be1b78f9

block/bfq-cgroup.c

+23 −40
Original line number Original line Diff line number Diff line
@@ -565,27 +565,11 @@ static void bfq_group_set_parent(struct bfq_group *bfqg, 
	entity->sched_data = &parent->sched_data;

	entity->sched_data = &parent->sched_data;

}

}





static struct bfq_group *bfq_lookup_bfqg(struct bfq_data *bfqd,

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

					 struct blkcg *blkcg)

{

	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;

	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

	 * 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

	 * which, along with some of its relatives, has not been hooked yet
@@ -602,8 +586,15 @@ struct bfq_group *bfq_find_set_group(struct bfq_data *bfqd, 
			bfq_group_set_parent(curr_bfqg, parent);

			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);

}

}





/**

/**
@@ -679,25 +670,15 @@ void bfq_bfqq_move(struct bfq_data *bfqd, struct bfq_queue *bfqq, 
 * Move bic to blkcg, assuming that bfqd->lock is held; which makes

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

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

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

 * comments in bfq_bic_update_cgroup on this issue)

 * 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,

static void *__bfq_bic_change_cgroup(struct bfq_data *bfqd,

				     struct bfq_io_cq *bic,

				     struct bfq_io_cq *bic,

						struct blkcg *blkcg)

				     struct bfq_group *bfqg)

{

{

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

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

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

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

	struct bfq_group *bfqg;

	struct bfq_entity *entity;

	struct bfq_entity *entity;





	bfqg = bfq_find_set_group(bfqd, blkcg);



	if (unlikely(!bfqg))

		bfqg = bfqd->root_group;



	if (async_bfqq) {

	if (async_bfqq) {

		entity = &async_bfqq->entity;

		entity = &async_bfqq->entity;
@@ -749,20 +730,24 @@ static struct bfq_group *__bfq_bic_change_cgroup(struct bfq_data *bfqd, 
void bfq_bic_update_cgroup(struct bfq_io_cq *bic, struct bio *bio)

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

{

{

	struct bfq_data *bfqd = bic_to_bfqd(bic);

	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;

	uint64_t serial_nr;





	rcu_read_lock();

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

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





	/*

	/*

	 * Check whether blkcg has changed.  The condition may trigger

	 * Check whether blkcg has changed.  The condition may trigger

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

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

	 */

	 */

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

	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

	 * Update blkg_path for bfq_log_* functions. We cache this

	 * path, and update it here, for the following

	 * path, and update it here, for the following
@@ -815,8 +800,6 @@ void bfq_bic_update_cgroup(struct bfq_io_cq *bic, struct bio *bio) 
	 */

	 */

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

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

	bic->blkcg_serial_nr = serial_nr;

	bic->blkcg_serial_nr = serial_nr;

out:

	rcu_read_unlock();

}

}





/**

/**
@@ -1433,7 +1416,7 @@ void bfq_end_wr_async(struct bfq_data *bfqd) 
	bfq_end_wr_async_queues(bfqd, bfqd->root_group);

	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;

	return bfqd->root_group;

}

}

block/bfq-iosched.c

+1 −9
Original line number Original line Diff line number Diff line
@@ -5158,14 +5158,7 @@ static struct bfq_queue *bfq_get_queue(struct bfq_data *bfqd, 
	struct bfq_queue *bfqq;

	struct bfq_queue *bfqq;

	struct bfq_group *bfqg;

	struct bfq_group *bfqg;





	rcu_read_lock();

	bfqg = bfq_bio_bfqg(bfqd, bio);



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

	if (!bfqg) {

		bfqq = &bfqd->oom_bfqq;

		goto out;

	}



	if (!is_sync) {

	if (!is_sync) {

		async_bfqq = bfq_async_queue_prio(bfqd, bfqg, ioprio_class,

		async_bfqq = bfq_async_queue_prio(bfqd, bfqg, ioprio_class,

						  ioprio);

						  ioprio);
@@ -5209,7 +5202,6 @@ static struct bfq_queue *bfq_get_queue(struct bfq_data *bfqd, 
out:

out:

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

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

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

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

	rcu_read_unlock();

	return bfqq;

	return bfqq;

}

}

block/bfq-iosched.h

+1 −2
Original line number Original line Diff line number Diff line
@@ -978,8 +978,7 @@ void bfq_bfqq_move(struct bfq_data *bfqd, struct bfq_queue *bfqq, 
void bfq_init_entity(struct bfq_entity *entity, struct bfq_group *bfqg);

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_bic_update_cgroup(struct bfq_io_cq *bic, struct bio *bio);

void bfq_end_wr_async(struct bfq_data *bfqd);

void bfq_end_wr_async(struct bfq_data *bfqd);

struct bfq_group *bfq_find_set_group(struct bfq_data *bfqd,

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

				     struct blkcg *blkcg);

struct blkcg_gq *bfqg_to_blkg(struct bfq_group *bfqg);

struct blkcg_gq *bfqg_to_blkg(struct bfq_group *bfqg);

struct bfq_group *bfqq_group(struct bfq_queue *bfqq);

struct bfq_group *bfqq_group(struct bfq_queue *bfqq);

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

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