Commit 4e54a249 authored by Jan Kara's avatar Jan Kara Committed by Jens Axboe

bfq: Get rid of __bio_blkcg() usage

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.czSigned-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent 09f87186
...@@ -586,27 +586,11 @@ static void bfq_group_set_parent(struct bfq_group *bfqg, ...@@ -586,27 +586,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; struct bfq_group *parent;
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_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
...@@ -623,8 +607,15 @@ struct bfq_group *bfq_find_set_group(struct bfq_data *bfqd, ...@@ -623,8 +607,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);
} }
/** /**
...@@ -714,25 +705,15 @@ void bfq_bfqq_move(struct bfq_data *bfqd, struct bfq_queue *bfqq, ...@@ -714,25 +705,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;
...@@ -784,20 +765,24 @@ static struct bfq_group *__bfq_bic_change_cgroup(struct bfq_data *bfqd, ...@@ -784,20 +765,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
...@@ -850,8 +835,6 @@ void bfq_bic_update_cgroup(struct bfq_io_cq *bic, struct bio *bio) ...@@ -850,8 +835,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();
} }
/** /**
...@@ -1469,7 +1452,7 @@ void bfq_end_wr_async(struct bfq_data *bfqd) ...@@ -1469,7 +1452,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;
} }
......
...@@ -5726,14 +5726,7 @@ static struct bfq_queue *bfq_get_queue(struct bfq_data *bfqd, ...@@ -5726,14 +5726,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);
...@@ -5779,8 +5772,6 @@ static struct bfq_queue *bfq_get_queue(struct bfq_data *bfqd, ...@@ -5779,8 +5772,6 @@ static struct bfq_queue *bfq_get_queue(struct bfq_data *bfqd,
if (bfqq != &bfqd->oom_bfqq && is_sync && !respawn) if (bfqq != &bfqd->oom_bfqq && is_sync && !respawn)
bfqq = bfq_do_or_sched_stable_merge(bfqd, bfqq, bic); bfqq = bfq_do_or_sched_stable_merge(bfqd, bfqq, bic);
rcu_read_unlock();
return bfqq; return bfqq;
} }
......
...@@ -1009,8 +1009,7 @@ void bfq_bfqq_move(struct bfq_data *bfqd, struct bfq_queue *bfqq, ...@@ -1009,8 +1009,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);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment