diff mbox

[v2] blkcg: add sanity check for blkcg policy operations

Message ID 20171011094619.GA37417@localhost.didichuxing.com (mailing list archive)
State New, archived
Headers show

Commit Message

weiping zhang Oct. 11, 2017, 9:46 a.m. UTC
blkcg policy should keep cpd/pd's alloc_fn and free_fn in pairs,
otherwise policy would register fail.

Signed-off-by: weiping zhang <zhangweiping@didichuxing.com>
---
 block/blk-cgroup.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Jens Axboe Oct. 11, 2017, 4:51 p.m. UTC | #1
On 10/11/2017 03:46 AM, weiping zhang wrote:
> blkcg policy should keep cpd/pd's alloc_fn and free_fn in pairs,
> otherwise policy would register fail.
> 
> Signed-off-by: weiping zhang <zhangweiping@didichuxing.com>
> ---
>  block/blk-cgroup.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
> index e7ec676..67b01c5 100644
> --- a/block/blk-cgroup.c
> +++ b/block/blk-cgroup.c
> @@ -1419,6 +1419,16 @@ int blkcg_policy_register(struct blkcg_policy *pol)
>  	if (i >= BLKCG_MAX_POLS)
>  		goto err_unlock;
>  
> +	/* Make sure cpd_alloc_fn and cpd_free_fn in pairs */
> +	if ((pol->cpd_alloc_fn && !pol->cpd_free_fn) ||
> +		(!pol->cpd_alloc_fn && pol->cpd_free_fn))
> +		goto err_unlock;

This might be cleaner (and more readable) as:

        if (!pol->cpd_alloc_fn ^ !pol->cpd_free_fn)
                goto err_unlock;

Ditto for the pd part.
Weiping Zhang Oct. 12, 2017, 6:26 a.m. UTC | #2
On Wed, Oct 11, 2017 at 10:51:32AM -0600, Jens Axboe wrote:
> On 10/11/2017 03:46 AM, weiping zhang wrote:
> > blkcg policy should keep cpd/pd's alloc_fn and free_fn in pairs,
> > otherwise policy would register fail.
> > 
> > Signed-off-by: weiping zhang <zhangweiping@didichuxing.com>
> > ---
> >  block/blk-cgroup.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
> > index e7ec676..67b01c5 100644
> > --- a/block/blk-cgroup.c
> > +++ b/block/blk-cgroup.c
> > @@ -1419,6 +1419,16 @@ int blkcg_policy_register(struct blkcg_policy *pol)
> >  	if (i >= BLKCG_MAX_POLS)
> >  		goto err_unlock;
> >  
> > +	/* Make sure cpd_alloc_fn and cpd_free_fn in pairs */
> > +	if ((pol->cpd_alloc_fn && !pol->cpd_free_fn) ||
> > +		(!pol->cpd_alloc_fn && pol->cpd_free_fn))
> > +		goto err_unlock;
> 
> This might be cleaner (and more readable) as:
> 
>         if (!pol->cpd_alloc_fn ^ !pol->cpd_free_fn)
>                 goto err_unlock;
> 
> Ditto for the pd part.
> 
Really nice, I'll send v3.

Thanks
diff mbox

Patch

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index e7ec676..67b01c5 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -1419,6 +1419,16 @@  int blkcg_policy_register(struct blkcg_policy *pol)
 	if (i >= BLKCG_MAX_POLS)
 		goto err_unlock;
 
+	/* Make sure cpd_alloc_fn and cpd_free_fn in pairs */
+	if ((pol->cpd_alloc_fn && !pol->cpd_free_fn) ||
+		(!pol->cpd_alloc_fn && pol->cpd_free_fn))
+		goto err_unlock;
+
+	/* Make sure pd_alloc_fn and pd_free_fn in pairs */
+	if ((pol->pd_alloc_fn && !pol->pd_free_fn) ||
+		(!pol->pd_alloc_fn && pol->pd_free_fn))
+		goto err_unlock;
+
 	/* register @pol */
 	pol->plid = i;
 	blkcg_policy[pol->plid] = pol;