Re: [PATCH] blkcg: add sanity check for blkcg policy operations

2017-10-11 Thread weiping zhang 
On Wed, Oct 11, 2017 at 09:58:33AM +0200, Johannes Thumshirn wrote:
> On Wed, Oct 11, 2017 at 12:00:55AM +0800, weiping zhang wrote:
> > + * blkcg_policy_check_ops - check policy's operations
> > + * @pol: blkcg policy to check
> > + *
> > + * Make sure cpd/pd_alloc_fn and cpd/pd_free_fn in pairs.
> > + * Return true on success and false on failure.
> > + */
> > +static bool blkcg_policy_check_ops(struct blkcg_policy *pol)
> > +{
> > +   if ((pol->cpd_alloc_fn && !pol->cpd_free_fn) ||
> > +   (!pol->cpd_alloc_fn && pol->cpd_free_fn))
> > +   return false;
> > +
> > +   if ((pol->pd_alloc_fn && !pol->pd_free_fn) ||
> > +   (!pol->pd_alloc_fn && pol->pd_free_fn))
> > +   return false;
> > +
> > +   return true;
> > +}
> > +
> > +/**
> >   * blkcg_policy_register - register a blkcg policy
> >   * @pol: blkcg policy to register
> >   *
> > @@ -1419,6 +1439,9 @@ int blkcg_policy_register(struct blkcg_policy *pol)
> > if (i >= BLKCG_MAX_POLS)
> > goto err_unlock;
> >  
> > +   if (!blkcg_policy_check_ops(pol))
> > +   goto err_unlock;
> > +
> 
> Can you merge it into the caller?
> 
> + if ((pol->cpd_alloc_fn && !pol->cpd_free_fn) ||
> + (!pol->cpd_alloc_fn && pol->cpd_free_fn))
> + goto err_unlock;
> +
> + if ((pol->pd_alloc_fn && !pol->pd_free_fn) ||
> + (!pol->pd_alloc_fn && pol->pd_free_fn))
> + goto err_unlock;
> 
> 
It's ok for me, I send V2 later.

Thanks
Weiping

Re: [PATCH] blkcg: add sanity check for blkcg policy operations

2017-10-11 Thread Johannes Thumshirn 
On Wed, Oct 11, 2017 at 12:00:55AM +0800, weiping zhang wrote:
> + * blkcg_policy_check_ops - check policy's operations
> + * @pol: blkcg policy to check
> + *
> + * Make sure cpd/pd_alloc_fn and cpd/pd_free_fn in pairs.
> + * Return true on success and false on failure.
> + */
> +static bool blkcg_policy_check_ops(struct blkcg_policy *pol)
> +{
> + if ((pol->cpd_alloc_fn && !pol->cpd_free_fn) ||
> + (!pol->cpd_alloc_fn && pol->cpd_free_fn))
> + return false;
> +
> + if ((pol->pd_alloc_fn && !pol->pd_free_fn) ||
> + (!pol->pd_alloc_fn && pol->pd_free_fn))
> + return false;
> +
> + return true;
> +}
> +
> +/**
>   * blkcg_policy_register - register a blkcg policy
>   * @pol: blkcg policy to register
>   *
> @@ -1419,6 +1439,9 @@ int blkcg_policy_register(struct blkcg_policy *pol)
>   if (i >= BLKCG_MAX_POLS)
>   goto err_unlock;
>  
> + if (!blkcg_policy_check_ops(pol))
> + goto err_unlock;
> +

Can you merge it into the caller?

+   if ((pol->cpd_alloc_fn && !pol->cpd_free_fn) ||
+   (!pol->cpd_alloc_fn && pol->cpd_free_fn))
+   goto err_unlock;
+
+   if ((pol->pd_alloc_fn && !pol->pd_free_fn) ||
+   (!pol->pd_alloc_fn && pol->pd_free_fn))
+   goto err_unlock;



-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

2017-10-10 Thread weiping zhang 
blkcg policy should keep cpd/pd's alloc_fn and free_fn in pairs,
otherwise policy would register fail.

Signed-off-by: weiping zhang 
---
 block/blk-cgroup.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index e7ec676..557c122 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -1397,6 +1397,26 @@ void blkcg_deactivate_policy(struct request_queue *q,
 EXPORT_SYMBOL_GPL(blkcg_deactivate_policy);
 
 /**
+ * blkcg_policy_check_ops - check policy's operations
+ * @pol: blkcg policy to check
+ *
+ * Make sure cpd/pd_alloc_fn and cpd/pd_free_fn in pairs.
+ * Return true on success and false on failure.
+ */
+static bool blkcg_policy_check_ops(struct blkcg_policy *pol)
+{
+   if ((pol->cpd_alloc_fn && !pol->cpd_free_fn) ||
+   (!pol->cpd_alloc_fn && pol->cpd_free_fn))
+   return false;
+
+   if ((pol->pd_alloc_fn && !pol->pd_free_fn) ||
+   (!pol->pd_alloc_fn && pol->pd_free_fn))
+   return false;
+
+   return true;
+}
+
+/**
  * blkcg_policy_register - register a blkcg policy
  * @pol: blkcg policy to register
  *
@@ -1419,6 +1439,9 @@ int blkcg_policy_register(struct blkcg_policy *pol)
if (i >= BLKCG_MAX_POLS)
goto err_unlock;
 
+   if (!blkcg_policy_check_ops(pol))
+   goto err_unlock;
+
/* register @pol */
pol->plid = i;
blkcg_policy[pol->plid] = pol;
-- 
2.9.4