| Message ID | 3E333A73B6B6DFC0+20250317022924.150907-1-chenlinxuan@uniontech.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [RESEND,v2] blk-cgroup: improve policy registration error handling | expand |
| Context | Check | Description |
|---|---|---|
| shin/vmtest-linus-master-PR | success | PR summary |
| shin/vmtest-linus-master-VM_Test-0 | success | Logs for build-kernel |
| shin/vmtest-linus-master-VM_Test-1 | success | Logs for build-kernel |
On Mon, Mar 17, 2025 at 10:29:24AM +0800, Chen Linxuan wrote: > 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> Acked-by: Tejun Heo <tj@kernel.org> Thanks.
On Mon, Mar 17, 2025 at 10:29:24AM +0800, Chen Linxuan <chenlinxuan@uniontech.com> wrote: > 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> > --- > block/blk-cgroup.c | 22 ++++++++++++---------- > 1 file changed, 12 insertions(+), 10 deletions(-) Reviewed-by: Michal Koutný <mkoutny@suse.com>
On Mon, 17 Mar 2025 10:29:24 +0800, Chen Linxuan wrote: > 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. > > [...] Applied, thanks! [1/1] blk-cgroup: improve policy registration error handling commit: e1a0202c6bfda24002a3ae2115154fa90104c649 Best regards,
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;