From: bin.lan.cn@windriver.com
To: gregkh@linuxfoundation.org, stable@vger.kernel.org
Cc: hch@lst.de, yukuai3@huawei.com, tj@kernel.org,
bin.lan.cn@windriver.com, axboe@kernel.dk
Subject: [PATCH 5.15/5.10 1/2] blk-cgroup: support to track if policy is online
Date: Thu, 17 Apr 2025 15:33:51 +0800 [thread overview]
Message-ID: <20250417073352.2675645-1-bin.lan.cn@windriver.com> (raw)
From: Yu Kuai <yukuai3@huawei.com>
[ Upstream commit dfd6200a095440b663099d8d42f1efb0175a1ce3 ]
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: Yu Kuai <yukuai3@huawei.com>
Acked-by: Tejun Heo <tj@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Link: https://lore.kernel.org/r/20230119110350.2287325-3-yukuai1@huaweicloud.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Bin Lan <bin.lan.cn@windriver.com>
Signed-off-by: He Zhe <zhe.he@windriver.com>
---
Build test passed.
---
block/blk-cgroup.c | 24 +++++++++++++++++-------
include/linux/blk-cgroup.h | 1 +
2 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 6180c680136b..e372a3fc264e 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -192,6 +192,7 @@ static struct blkcg_gq *blkg_alloc(struct blkcg *blkcg, struct request_queue *q,
blkg->pd[i] = pd;
pd->blkg = blkg;
pd->plid = i;
+ pd->online = false;
}
return blkg;
@@ -289,8 +290,11 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg,
for (i = 0; i < BLKCG_MAX_POLS; i++) {
struct blkcg_policy *pol = blkcg_policy[i];
- if (blkg->pd[i] && pol->pd_online_fn)
- pol->pd_online_fn(blkg->pd[i]);
+ if (blkg->pd[i]) {
+ if (pol->pd_online_fn)
+ pol->pd_online_fn(blkg->pd[i]);
+ blkg->pd[i]->online = true;
+ }
}
}
blkg->online = true;
@@ -390,8 +394,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)
- pol->pd_offline_fn(blkg->pd[i]);
+ 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;
@@ -1367,6 +1374,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 */
@@ -1374,9 +1382,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]);
- if (pol->pd_online_fn)
- list_for_each_entry_reverse(blkg, &q->blkg_list, q_node)
+ list_for_each_entry_reverse(blkg, &q->blkg_list, q_node) {
+ if (pol->pd_online_fn)
pol->pd_online_fn(blkg->pd[pol->plid]);
+ blkg->pd[pol->plid]->online = true;
+ }
__set_bit(pol->plid, q->blkcg_pols);
ret = 0;
@@ -1438,7 +1448,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;
diff --git a/include/linux/blk-cgroup.h b/include/linux/blk-cgroup.h
index 27c363f6b281..c5eda86e4118 100644
--- a/include/linux/blk-cgroup.h
+++ b/include/linux/blk-cgroup.h
@@ -92,6 +92,7 @@ struct blkg_policy_data {
/* the blkg and policy id this per-policy data belongs to */
struct blkcg_gq *blkg;
int plid;
+ bool online;
};
/*
--
2.34.1
next reply other threads:[~2025-04-17 7:34 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-17 7:33 bin.lan.cn [this message]
2025-04-17 7:33 ` [PATCH 5.15/5.10 2/2] blk-iocost: do not WARN if iocg was already offlined bin.lan.cn
2025-04-18 15:57 ` Sasha Levin
2025-04-18 15:42 ` [PATCH 5.15/5.10 1/2] blk-cgroup: support to track if policy is online Sasha Levin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250417073352.2675645-1-bin.lan.cn@windriver.com \
--to=bin.lan.cn@windriver.com \
--cc=axboe@kernel.dk \
--cc=gregkh@linuxfoundation.org \
--cc=hch@lst.de \
--cc=stable@vger.kernel.org \
--cc=tj@kernel.org \
--cc=yukuai3@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox