Hi Chandra,
This is the last comment from my code review. (It's inlined.)
As a next step, I will try to port kurosawa-san's cpu resource
controller to f0 ckrm to come up with further comments.
As a result of the review, however, original f0 patch does not
seem to work fine to integrate with a resource controller.
Are you going to release the revised f0 patch soon?
If not, I will hack the code for now.
Thanks,
MAEDA Naoaki
Chandra Seetharaman wrote:
> Index: linux-2.6.13/kernel/ckrm/ckrmutils.c
> ===================================================================
(snip)
> +/*
> + * Caller is responsible for holding any lock to protect the data
> + * structures passed to this function
> + */
> +int
> +set_shares(struct ckrm_shares *new, struct ckrm_shares *cur,
> + struct ckrm_shares *par)
> +{
> + int rc = -EINVAL;
> + int cur_usage_guar = cur->total_guarantee - cur->unused_guarantee;
> + int increase_by;
> +
> + if (cur->my_guarantee < 0) /* DONTCARE or UNCHANGED */
> + increase_by = new->my_guarantee;
> + else
> + increase_by = new->my_guarantee - cur->my_guarantee;
> +
> + /* Check total_guarantee for correctness */
> + if (new->total_guarantee <= CKRM_SHARE_DONTCARE)
> + goto set_share_err;
> + else if (new->total_guarantee == CKRM_SHARE_UNCHANGED)
> + /* do nothing */;
> + else if (cur_usage_guar > new->total_guarantee)
> + goto set_share_err;
> +
> + /* Check max_limit for correctness */
> + if (new->max_limit <= CKRM_SHARE_DONTCARE)
> + goto set_share_err;
> + else if (new->max_limit == CKRM_SHARE_UNCHANGED)
> + /* do nothing */;
> + else if (cur->cur_max_limit > new->max_limit)
> + goto set_share_err;
> +
> + /* Check my_guarantee for correctness */
> + if (new->my_guarantee == CKRM_SHARE_UNCHANGED)
> + /* do nothing */;
> + else if (new->my_guarantee == CKRM_SHARE_DONTCARE)
> + /* do nothing */;
> + else if (par && increase_by > par->unused_guarantee)
> + goto set_share_err;
> +
> + /* Check my_limit for correctness */
> + if (new->my_limit == CKRM_SHARE_UNCHANGED)
> + /* do nothing */;
> + else if (new->my_limit == CKRM_SHARE_DONTCARE)
> + /* do nothing */;
> + else if (par && new->my_limit > par->max_limit)
> + /* I can't get more limit than my parent's limit */
> + goto set_share_err;
> +
> + /* make sure guarantee is lesser than limit */
> + if (new->my_limit == CKRM_SHARE_DONTCARE)
> + /* do nothing */;
> + else if (new->my_limit == CKRM_SHARE_UNCHANGED) {
> + if (new->my_guarantee == CKRM_SHARE_DONTCARE)
> + /* do nothing */;
> + else if (new->my_guarantee == CKRM_SHARE_UNCHANGED)
> + /*
> + * do nothing; earlier setting would have
> + * taken care of it
> + */;
> + else if (new->my_guarantee > cur->my_limit)
> + goto set_share_err;
> + } else /* new->my_limit has a valid value */
> + if (new->my_guarantee == CKRM_SHARE_DONTCARE)
> + /* do nothing */;
> + else if (new->my_guarantee == CKRM_SHARE_UNCHANGED) {
> + if (cur->my_guarantee > new->my_limit)
> + goto set_share_err;
> + } else if (new->my_guarantee > new->my_limit)
> + goto set_share_err;
"{" and "}" is missing around the body of the following
three if statements.
> + if (new->my_guarantee != CKRM_SHARE_UNCHANGED)
> + child_guarantee_changed(par, cur->my_guarantee,
> + new->my_guarantee);
> + cur->my_guarantee = new->my_guarantee;
> +
> + if (new->my_limit != CKRM_SHARE_UNCHANGED)
> + child_maxlimit_changed(par, new->my_limit);
> + cur->my_limit = new->my_limit;
> +
> + if (new->total_guarantee != CKRM_SHARE_UNCHANGED)
> + cur->unused_guarantee = new->total_guarantee - cur_usage_guar;
> + cur->total_guarantee = new->total_guarantee;
> +
> + if (new->max_limit != CKRM_SHARE_UNCHANGED)
> + cur->max_limit = new->max_limit;
> +
> + rc = 0;
> +set_share_err:
> + return rc;
> +}
-------------------------------------------------------
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
