Commit 27e6fa99 authored by Dennis Zhou (Facebook)'s avatar Dennis Zhou (Facebook) Committed by Jens Axboe
Browse files

blkcg: fix ref count issue with bio_blkcg using task_css 



The accessor function bio_blkcg either returns the blkcg associated with
the bio or finds one in the current context. This can cause an issue
when trying to associate a bio with a blkcg. Particularly, it's the
third case that is problematic:

	return css_to_blkcg(task_css(current, io_cgrp_id));

As the above may race against task migration and the cgroup exiting, it
is not always ok to take a reference on the blkcg returned from
bio_blkcg.

This patch adds association ahead of calling bio_blkcg rather than
after. This makes association a required and explicit step along the
code paths for calling bio_blkcg. blk_get_rl is modified as well to get
a reference to the blkcg it may use and blk_put_rl will always put the
reference back. Association is also moved above the bio_blkcg call to
ensure it will not return NULL in blk-iolatency.

BFQ and CFQ utilize this flaw, but due to the complexity, I do not want
to address this in this series. I've created a private version of the
function with notes not to use it describing the flaw. Hopefully soon,
that code can be cleaned up.

Signed-off-by: default avatarDennis Zhou <dennisszhou@gmail.com>
Acked-by: default avatarTejun Heo <tj@kernel.org>
Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent 9ff01255

block/bfq-cgroup.c

+2 −2
Original line number Diff line number Diff line
@@ -642,7 +642,7 @@ void bfq_bic_update_cgroup(struct bfq_io_cq *bic, struct bio *bio) 
	uint64_t serial_nr;



	rcu_read_lock();

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

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



	/*

	 * Check whether blkcg has changed.  The condition may trigger
@@ -651,7 +651,7 @@ void bfq_bic_update_cgroup(struct bfq_io_cq *bic, struct bio *bio) 
	if (unlikely(!bfqd) || likely(bic->blkcg_serial_nr == serial_nr))

		goto out;



	bfqg = __bfq_bic_change_cgroup(bfqd, bic, bio_blkcg(bio));

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

	/*

	 * Update blkg_path for bfq_log_* functions. We cache this

	 * path, and update it here, for the following

block/bfq-iosched.c

+1 −1
Original line number Diff line number Diff line
@@ -4359,7 +4359,7 @@ static struct bfq_queue *bfq_get_queue(struct bfq_data *bfqd, 


	rcu_read_lock();



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

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

	if (!bfqg) {

		bfqq = &bfqd->oom_bfqq;

		goto out;

block/bio.c

+8 −2
Original line number Diff line number Diff line
@@ -1988,13 +1988,19 @@ int bio_associate_blkcg_from_page(struct bio *bio, struct page *page) 
 *

 * This function takes an extra reference of @blkcg_css which will be put

 * when @bio is released.  The caller must own @bio and is responsible for

 * synchronizing calls to this function.

 * synchronizing calls to this function.  If @blkcg_css is NULL, a call to

 * blkcg_get_css finds the current css from the kthread or task.

 */

int bio_associate_blkcg(struct bio *bio, struct cgroup_subsys_state *blkcg_css)

{

	if (unlikely(bio->bi_css))

		return -EBUSY;



	if (blkcg_css)

		css_get(blkcg_css);

	else

		blkcg_css = blkcg_get_css();



	bio->bi_css = blkcg_css;

	return 0;

}

block/blk-iolatency.c

+1 −1
Original line number Diff line number Diff line
@@ -401,8 +401,8 @@ static void blkcg_iolatency_throttle(struct rq_qos *rqos, struct bio *bio, 
		return;



	rcu_read_lock();

	bio_associate_blkcg(bio, NULL);

	blkcg = bio_blkcg(bio);

	bio_associate_blkcg(bio, &blkcg->css);

	blkg = blkg_lookup(blkcg, q);

	if (unlikely(!blkg)) {

		if (!lock)

block/cfq-iosched.c

+2 −2
Original line number Diff line number Diff line
@@ -3753,7 +3753,7 @@ static void check_blkcg_changed(struct cfq_io_cq *cic, struct bio *bio) 
	uint64_t serial_nr;



	rcu_read_lock();

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

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

	rcu_read_unlock();



	/*
@@ -3818,7 +3818,7 @@ cfq_get_queue(struct cfq_data *cfqd, bool is_sync, struct cfq_io_cq *cic, 
	struct cfq_group *cfqg;



	rcu_read_lock();

	cfqg = cfq_lookup_cfqg(cfqd, bio_blkcg(bio));

	cfqg = cfq_lookup_cfqg(cfqd, __bio_blkcg(bio));

	if (!cfqg) {

		cfqq = &cfqd->oom_cfqq;

		goto out;

CRA Git | Maintained and supported by SUSTech CRA and CCSE