|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 3/4] libxl: add rt scheduler
2014-09-08 11:19 GMT-04:00 George Dunlap <george.dunlap@xxxxxxxxxxxxx>:
> On 09/07/2014 08:41 PM, Meng Xu wrote:
>>
>> Add libxl functions to set/get domain's parameters for rt scheduler
>> Note: VCPU's information (period, budget) is in microsecond (us).
>>
>> Signed-off-by: Meng Xu <mengxu@xxxxxxxxxxxxx>
>> Signed-off-by: Sisu Xi <xisisu@xxxxxxxxx>
>> ---
>> tools/libxl/libxl.c | 75
>> +++++++++++++++++++++++++++++++++++++++++++
>> tools/libxl/libxl.h | 1 +
>> tools/libxl/libxl_types.idl | 2 ++
>> 3 files changed, 78 insertions(+)
>>
>> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
>> index 2ae5fca..6840c92 100644
>> --- a/tools/libxl/libxl.c
>> +++ b/tools/libxl/libxl.c
>> @@ -5155,6 +5155,75 @@ static int sched_sedf_domain_set(libxl__gc *gc,
>> uint32_t domid,
>> return 0;
>> }
>> +static int sched_rt_domain_get(libxl__gc *gc, uint32_t domid,
>> + libxl_domain_sched_params *scinfo)
>> +{
>> + struct xen_domctl_sched_rt sdom;
>> + int rc;
>> +
>> + rc = xc_sched_rt_domain_get(CTX->xch, domid, &sdom);
>> + if (rc != 0) {
>> + LOGE(ERROR, "getting domain sched rt");
>> + return ERROR_FAIL;
>> + }
>> +
>> + libxl_domain_sched_params_init(scinfo);
>> +
>> + scinfo->sched = LIBXL_SCHEDULER_RT_DS;
>> + scinfo->period = sdom.period;
>> + scinfo->budget = sdom.budget;
>> +
>> + return 0;
>> +}
>> +
>> +#define SCHED_RT_DS_VCPU_PERIOD_UINT_MAX 4294967295U /* 2^32 - 1 us */
>> +#define SCHED_RT_DS_VCPU_BUDGET_UINT_MAX
>> SCHED_RT_DS_VCPU_PERIOD_UINT_MAX
>
>
> I think what Dario was looking for was this:
>
> #define SCHED_RT_DS_VCPU_PERIOD_MAX UINT_MAX
>
> I.e., use the already-defined #defines with meaningful names (line
> UINT_MAX), and avoid open-coding (i.e., typing out a "magic" number, like
> 429....U).
Ah, I see. I misunderstood. :-( Thank you very much, George, for
clarification! :-)
>
>> +
>> +static int sched_rt_domain_set(libxl__gc *gc, uint32_t domid,
>> + const libxl_domain_sched_params *scinfo)
>> +{
>> + struct xen_domctl_sched_rt sdom;
>> + int rc;
>> +
>> + rc = xc_sched_rt_domain_get(CTX->xch, domid, &sdom);
>
>
> You need to check the return value here and bail out on an error.
Right, will do.
>
>> +
>> + if (scinfo->period != LIBXL_DOMAIN_SCHED_PARAM_PERIOD_DEFAULT) {
>> + if (scinfo->period < 1 ||
>> + scinfo->period > SCHED_RT_DS_VCPU_PERIOD_UINT_MAX) {
>
>
> ...but this isn't right anyway, right? scinfo->period is a signed integer.
> You shouldn't be comparing it to an unsigned int; and this can never be
> false anyway, because even if it's automatically cast to be unsigned, the
> type isn't big enough to be bigger than UINT_MAX anyway.
>
> If period is allowed to be anything up to INT_MAX, then there's no need to
> check the upper bound. Checking to make sure it's >= 1 should be
> sufficient. Then you can just get rid of the #defines above.
I see and will change it as you said.
>
>> + LOG(ERROR, "VCPU period is not set or out of range, "
>> + "valid values are within range from 0 to %u",
>> + SCHED_RT_DS_VCPU_PERIOD_UINT_MAX);
>> + return ERROR_INVAL;
>> + }
>> + sdom.period = scinfo->period;
>> + }
>> +
>> + if (scinfo->budget != LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT) {
>> + if (scinfo->budget < 1 ||
>> + scinfo->budget > SCHED_RT_DS_VCPU_BUDGET_UINT_MAX) {
>
>
> Same here.
Will change, Thanks!
>
>
>> + LOG(ERROR, "VCPU budget is not set or out of range, "
>> + "valid values are within range from 0 to %u",
>> + SCHED_RT_DS_VCPU_BUDGET_UINT_MAX);
>> + return ERROR_INVAL;
>> + }
>> + sdom.budget = scinfo->budget;
>> + }
>> +
>> + if (sdom.budget > sdom.period) {
>> + LOG(ERROR, "VCPU budget is larger than VCPU period, "
>> + "VCPU budget should be no larger than VCPU period");
>> + return ERROR_INVAL;
>> + }
>> +
>> + rc = xc_sched_rt_domain_set(CTX->xch, domid, &sdom);
>> + if (rc < 0) {
>> + LOGE(ERROR, "setting domain sched rt");
>> + return ERROR_FAIL;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> int libxl_domain_sched_params_set(libxl_ctx *ctx, uint32_t domid,
>> const libxl_domain_sched_params
>> *scinfo)
>> {
>> @@ -5178,6 +5247,9 @@ int libxl_domain_sched_params_set(libxl_ctx *ctx,
>> uint32_t domid,
>> case LIBXL_SCHEDULER_ARINC653:
>> ret=sched_arinc653_domain_set(gc, domid, scinfo);
>> break;
>> + case LIBXL_SCHEDULER_RT_DS:
>> + ret=sched_rt_domain_set(gc, domid, scinfo);
>> + break;
>> default:
>> LOG(ERROR, "Unknown scheduler");
>> ret=ERROR_INVAL;
>> @@ -5208,6 +5280,9 @@ int libxl_domain_sched_params_get(libxl_ctx *ctx,
>> uint32_t domid,
>> case LIBXL_SCHEDULER_CREDIT2:
>> ret=sched_credit2_domain_get(gc, domid, scinfo);
>> break;
>> + case LIBXL_SCHEDULER_RT_DS:
>> + ret=sched_rt_domain_get(gc, domid, scinfo);
>> + break;
>> default:
>> LOG(ERROR, "Unknown scheduler");
>> ret=ERROR_INVAL;
>> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
>> index 460207b..dbe736c 100644
>> --- a/tools/libxl/libxl.h
>> +++ b/tools/libxl/libxl.h
>> @@ -1280,6 +1280,7 @@ int libxl_sched_credit_params_set(libxl_ctx *ctx,
>> uint32_t poolid,
>> #define LIBXL_DOMAIN_SCHED_PARAM_SLICE_DEFAULT -1
>> #define LIBXL_DOMAIN_SCHED_PARAM_LATENCY_DEFAULT -1
>> #define LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT -1
>> +#define LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT -1
>> int libxl_domain_sched_params_get(libxl_ctx *ctx, uint32_t domid,
>> libxl_domain_sched_params *params);
>> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
>> index 931c9e9..72f24fe 100644
>> --- a/tools/libxl/libxl_types.idl
>> +++ b/tools/libxl/libxl_types.idl
>> @@ -153,6 +153,7 @@ libxl_scheduler = Enumeration("scheduler", [
>> (5, "credit"),
>> (6, "credit2"),
>> (7, "arinc653"),
>> + (8, "rt_ds"),
>
>
> rtds
>
Roger, will change very rt_ds to rtds then. :-P
Thanks,
Meng
--
-----------
Meng Xu
PhD Student in Computer and Information Science
University of Pennsylvania
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |