On Fri, 2005-09-16 at 16:18 +0900, MAEDA Naoaki wrote:
Thanks for your review and comments.
> > +
> > + /* Check for legal values */
> > + if (!legalshare(shares.my_guarantee) || !legalshare(shares.my_limit)
> > + || !legalshare(shares.total_guarantee)
> > + || !legalshare(shares.max_limit))
> > + goto out;
> > +
> > + rc = (*rcbs->set_share_values) (cls->res_class[rcbs->resid], &shares);
>
> Is it safe to call (*rcbs->set_share_values)() without incrementing
> nr_resusers[rcbs->resid] reference count?
on the first pass, looked that it is needed. But thought a little more
and realized it is not needed anywhere in this file.
The reason is: When a class is created a reference is counted for in for
each resource controller. And now(with Paul's fix), each of these files
hold a reference to the class. So, a resource controller cannot
deregister as long a directory(or a file in the directory) exist.
Hence removed all the incr/decr or nr_resusers, except the ones that are
in class creation/deletion.
>
> > +out:
> > + kfree(resname);
>
> Shouldn't we check whether "resname" has been set before calling
> kfree(resname)? If shares_parse() is failed, "resname" may still
> left unset.
not needed, resname is initialized to NULL and kfree takes care of NULL.
<snip>
>
> > +
> > +#define set_info(cls, cfgstr, fn) \
> > +do {
> > \
> > + int rc; \
> > + char *resname = NULL, *otherstr = NULL; \
> > + struct ckrm_res_ctlr *rcbs; \
> > + \
> > + if (ckrm_generic_parse(cfgstr, &resname, &otherstr) == 0) \
> > + return 0; \
> > + rcbs = ckrm_resctlr_byname((resname)); \
> > + \
> > + if (rcbs == NULL || rcbs->fn == NULL) \
>
> It seems that "resname" and "otherstr" must be kfreed before returning.
yup. will fix.
>
> > + return -EINVAL; \
> > + rc = (*rcbs->fn) ((cls)->res_class[rcbs->resid], (otherstr)); \
>
> Is it safe to call (*rcbs->fn)() without incrementing
> nr_resusers[rcbs->resid] reference count?
not needed (explanation above).
>
> > + kfree(resname); \
> > + kfree(otherstr); \
> > + return rc; \
> > +} while(0)
> > +
>
> Thanks,
> MAEDA Naoaki
>
>
--
----------------------------------------------------------------------
Chandra Seetharaman | Be careful what you choose....
- [EMAIL PROTECTED] | .......you may get it.
----------------------------------------------------------------------
-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. Download
it for free - -and be entered to win a 42" plasma tv or your very own
Sony(tm)PSP. Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
ckrm-tech mailing list
https://lists.sourceforge.net/lists/listinfo/ckrm-tech
