| Message ID | D8F5396BD45124E9+20250213033545.993799-2-chenlinxuan@uniontech.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] blk-cgroup: improve policy registration error handling | expand |
在 2025/02/13 11:35, Chen Linxuan 写道: > This patch improve the returned error code of blkcg_policy_register(). > > 1. Move the validation check for cpd/pd_alloc_fn and cpd/pd_free_fn > function pairs to the start of blkcg_policy_register(). This ensures > we immediately return -EINVAL if the function pairs are not correctly > provided, rather than returning -ENOSPC after locking and unlocking > mutexes unnecessarily. > > Those locks should not contention any problems, as error of policy > registration is a super cold path. > > 2. Return -ENOMEM when cpd_alloc_fn() failed. > > Co-authored-by: Wen Tao <wentao@uniontech.com> > Signed-off-by: Wen Tao <wentao@uniontech.com> > Signed-off-by: Chen Linxuan <chenlinxuan@uniontech.com> > --- > > v1->v2: Also change the return value to -ENOMEM from error path err_free_cpds > > --- > block/blk-cgroup.c | 22 ++++++++++++---------- > 1 file changed, 12 insertions(+), 10 deletions(-) > LGTM Reviewed-by: Yu Kuai <yukuai3@huawei.com> > diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c > index 9ed93d91d754..2609f7294427 100644 > --- a/block/blk-cgroup.c > +++ b/block/blk-cgroup.c > @@ -1727,27 +1727,27 @@ int blkcg_policy_register(struct blkcg_policy *pol) > struct blkcg *blkcg; > int i, ret; > > + /* > + * Make sure cpd/pd_alloc_fn and cpd/pd_free_fn in pairs, and policy > + * without pd_alloc_fn/pd_free_fn can't be activated. > + */ > + if ((!pol->cpd_alloc_fn ^ !pol->cpd_free_fn) || > + (!pol->pd_alloc_fn ^ !pol->pd_free_fn)) > + return -EINVAL; > + > mutex_lock(&blkcg_pol_register_mutex); > mutex_lock(&blkcg_pol_mutex); > > /* find an empty slot */ > - ret = -ENOSPC; > for (i = 0; i < BLKCG_MAX_POLS; i++) > if (!blkcg_policy[i]) > break; > if (i >= BLKCG_MAX_POLS) { > pr_warn("blkcg_policy_register: BLKCG_MAX_POLS too small\n"); > + ret = -ENOSPC; > goto err_unlock; > } > > - /* > - * Make sure cpd/pd_alloc_fn and cpd/pd_free_fn in pairs, and policy > - * without pd_alloc_fn/pd_free_fn can't be activated. > - */ > - if ((!pol->cpd_alloc_fn ^ !pol->cpd_free_fn) || > - (!pol->pd_alloc_fn ^ !pol->pd_free_fn)) > - goto err_unlock; > - > /* register @pol */ > pol->plid = i; > blkcg_policy[pol->plid] = pol; > @@ -1758,8 +1758,10 @@ int blkcg_policy_register(struct blkcg_policy *pol) > struct blkcg_policy_data *cpd; > > cpd = pol->cpd_alloc_fn(GFP_KERNEL); > - if (!cpd) > + if (!cpd) { > + ret = -ENOMEM; > goto err_free_cpds; > + } > > blkcg->cpd[pol->plid] = cpd; > cpd->blkcg = blkcg; >
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 9ed93d91d754..2609f7294427 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -1727,27 +1727,27 @@ int blkcg_policy_register(struct blkcg_policy *pol) struct blkcg *blkcg; int i, ret; + /* + * Make sure cpd/pd_alloc_fn and cpd/pd_free_fn in pairs, and policy + * without pd_alloc_fn/pd_free_fn can't be activated. + */ + if ((!pol->cpd_alloc_fn ^ !pol->cpd_free_fn) || + (!pol->pd_alloc_fn ^ !pol->pd_free_fn)) + return -EINVAL; + mutex_lock(&blkcg_pol_register_mutex); mutex_lock(&blkcg_pol_mutex); /* find an empty slot */ - ret = -ENOSPC; for (i = 0; i < BLKCG_MAX_POLS; i++) if (!blkcg_policy[i]) break; if (i >= BLKCG_MAX_POLS) { pr_warn("blkcg_policy_register: BLKCG_MAX_POLS too small\n"); + ret = -ENOSPC; goto err_unlock; } - /* - * Make sure cpd/pd_alloc_fn and cpd/pd_free_fn in pairs, and policy - * without pd_alloc_fn/pd_free_fn can't be activated. - */ - if ((!pol->cpd_alloc_fn ^ !pol->cpd_free_fn) || - (!pol->pd_alloc_fn ^ !pol->pd_free_fn)) - goto err_unlock; - /* register @pol */ pol->plid = i; blkcg_policy[pol->plid] = pol; @@ -1758,8 +1758,10 @@ int blkcg_policy_register(struct blkcg_policy *pol) struct blkcg_policy_data *cpd; cpd = pol->cpd_alloc_fn(GFP_KERNEL); - if (!cpd) + if (!cpd) { + ret = -ENOMEM; goto err_free_cpds; + } blkcg->cpd[pol->plid] = cpd; cpd->blkcg = blkcg;