blk-cgroup: support to track if policy is online

commit dfd6200a095440b663099d8d42f1efb0175a1ce3 upstream.

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>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 13a17ad646e00fec433d51f3d76de16982e96726..dbd18b75ec9155f992812da4fb174dcd30b486c7 100644 (file)
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -191,6 +191,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;
@@ -288,8 +289,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;
@@ -389,8 +393,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;
@@ -1364,6 +1371,7 @@ retry:
                blkg->pd[pol->plid] = pd;
                pd->blkg = blkg;
                pd->plid = pol->plid;
+               pd->online = false;
        }
 
        /* all allocated, init in the same order */
@@ -1371,9 +1379,11 @@ retry:
                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;
@@ -1435,7 +1445,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 b89099360a8673053c07e279331e720c0e837aba..d0a5e62cf520d68e643875b70d4713e0e7890825 100644 (file)
--- a/include/linux/blk-cgroup.h
+++ b/include/linux/blk-cgroup.h
@@ -87,6 +87,7 @@ struct blkg_policy_data {
        /* the blkg and policy id this per-policy data belongs to */
        struct blkcg_gq                 *blkg;
        int                             plid;
+       bool                            online;
 };
 
 /*