Commit dfd6200a authored by Yu Kuai's avatar Yu Kuai Committed by Jens Axboe
Browse files

blk-cgroup: support to track if policy is online 



A new field 'online' is added to blkg_policy_data to fix following
2 problem:

1) In blkcg_activate_policy(), if pd_alloc_fn() with 'GFP_NOWAIT'
   failed, 'queue_lock' will be dropped and pd_alloc_fn() will try again
   without 'GFP_NOWAIT'. In the meantime, remove cgroup can race with
   it, and pd_offline_fn() will be called without pd_init_fn() and
   pd_online_fn(). This way null-ptr-deference can be triggered.

2) In order to synchronize pd_free_fn() from blkg_free_workfn() and
   blkcg_deactivate_policy(), 'list_del_init(&blkg->q_node)' will be
   delayed to blkg_free_workfn(), hence pd_offline_fn() can be called
   first in blkg_destroy(), and then blkcg_deactivate_policy() will
   call it again, we must prevent it.

The new field 'online' will be set after pd_online_fn() and will be
cleared after pd_offline_fn(), in the meantime pd_offline_fn() will only
be called if 'online' is set.

Signed-off-by: default avatarYu Kuai <yukuai3@huawei.com>
Acked-by: default avatarTejun Heo <tj@kernel.org>
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Link: https://lore.kernel.org/r/20230119110350.2287325-3-yukuai1@huaweicloud.com


Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent c7241bab

block/blk-cgroup.c

+17 −7
Original line number Diff line number Diff line
@@ -288,6 +288,7 @@ static struct blkcg_gq *blkg_alloc(struct blkcg *blkcg, struct gendisk *disk, 
		blkg->pd[i] = pd;

		pd->blkg = blkg;

		pd->plid = i;

		pd->online = false;

	}



	return blkg;
@@ -359,8 +360,11 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg, struct gendisk *disk, 
		for (i = 0; i < BLKCG_MAX_POLS; i++) {

			struct blkcg_policy *pol = blkcg_policy[i];



			if (blkg->pd[i] && pol->pd_online_fn)

			if (blkg->pd[i]) {

				if (pol->pd_online_fn)

					pol->pd_online_fn(blkg->pd[i]);

				blkg->pd[i]->online = true;

			}

		}

	}

	blkg->online = true;
@@ -465,8 +469,11 @@ static void blkg_destroy(struct blkcg_gq *blkg) 
	for (i = 0; i < BLKCG_MAX_POLS; i++) {

		struct blkcg_policy *pol = blkcg_policy[i];



		if (blkg->pd[i] && pol->pd_offline_fn)

		if (blkg->pd[i] && blkg->pd[i]->online) {

			if (pol->pd_offline_fn)

				pol->pd_offline_fn(blkg->pd[i]);

			blkg->pd[i]->online = false;

		}

	}



	blkg->online = false;
@@ -1448,6 +1455,7 @@ int blkcg_activate_policy(struct request_queue *q, 
		blkg->pd[pol->plid] = pd;

		pd->blkg = blkg;

		pd->plid = pol->plid;

		pd->online = false;

	}



	/* all allocated, init in the same order */
@@ -1455,9 +1463,11 @@ int blkcg_activate_policy(struct request_queue *q, 
		list_for_each_entry_reverse(blkg, &q->blkg_list, q_node)

			pol->pd_init_fn(blkg->pd[pol->plid]);



	list_for_each_entry_reverse(blkg, &q->blkg_list, q_node) {

		if (pol->pd_online_fn)

		list_for_each_entry_reverse(blkg, &q->blkg_list, q_node)

			pol->pd_online_fn(blkg->pd[pol->plid]);

		blkg->pd[pol->plid]->online = true;

	}



	__set_bit(pol->plid, q->blkcg_pols);

	ret = 0;
@@ -1519,7 +1529,7 @@ void blkcg_deactivate_policy(struct request_queue *q, 


		spin_lock(&blkcg->lock);

		if (blkg->pd[pol->plid]) {

			if (pol->pd_offline_fn)

			if (blkg->pd[pol->plid]->online && pol->pd_offline_fn)

				pol->pd_offline_fn(blkg->pd[pol->plid]);

			pol->pd_free_fn(blkg->pd[pol->plid]);

			blkg->pd[pol->plid] = NULL;

block/blk-cgroup.h

+1 −0
Original line number Diff line number Diff line
@@ -135,6 +135,7 @@ struct blkg_policy_data { 
	/* the blkg and policy id this per-policy data belongs to */

	struct blkcg_gq			*blkg;

	int				plid;

	bool				online;

};



/*