[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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.