Re: [Xen-devel] [PATCH v5 for Xen 4.7 3/4] libxl: enable per-VCPU parameter settings for RTDS scheduler

[Chong Li <lichong659@xxxxxxxxx>]

On Tue, Feb 9, 2016 at 6:00 AM, Dario Faggioli
<dario.faggioli@xxxxxxxxxx> wrote:
> On Thu, 2016-02-04 at 16:50 -0600, Chong Li wrote:
>>

>> +{
>> +    int rc;
>> +    int i;
>> +    uint16_t max_vcpuid;
>> +    xc_dominfo_t info;
>> +    struct xen_domctl_schedparam_vcpu *vcpus;
>> +    uint32_t num_vcpus;
>> +
>> +    rc = xc_domain_getinfo(CTX->xch, domid, 1, &info);
>> +    if (rc < 0) {
>> +        LOGE(ERROR, "getting domain info");
>> +        return ERROR_FAIL;
>> +    }
>> +    max_vcpuid = info.max_vcpu_id;
>> +
>> +    if (scinfo->num_vcpus > 0) {
>> +        num_vcpus = scinfo->num_vcpus;
>> +        GCNEW_ARRAY(vcpus, num_vcpus);
>> +        for (i = 0; i < num_vcpus; i++) {
>> +            if (scinfo->vcpus[i].vcpuid < 0 ||
>> +                    scinfo->vcpus[i].vcpuid > max_vcpuid) {
>> +                LOG(ERROR, "VCPU index is out of range, "
>> +                           "valid values are within range from 0 to
>> %d",
>> +                           max_vcpuid);
>> +                return ERROR_INVAL;
>> +            }
>> +            vcpus[i].vcpuid = scinfo->vcpus[i].vcpuid;
>> +
>> +            rc = sched_rtds_validate_params(gc,
>> +                    scinfo->vcpus[i].period, scinfo-
>> >vcpus[i].budget,
>> +                    &vcpus[i].s.rtds.period,
>> &vcpus[i].s.rtds.budget);
>> +            if (rc)
>> +                return ERROR_INVAL;
>> +        }
>> +    } else {
>>
> So, it looks to me that this function can be split in two. One would be
> the actual sched_rtds_vcpus_params_set(), and it will do what is being
> done above here.
>
> The other one would be something like
> sched_rtds_vcpus_params_set_all(), and it will do what is being done
> below here.
>
> About scinfo->num_vcpus, I think it would be fine for
> sched_rtds_vcpus_params_set() to enforce it being > 0, and erroring out
> if not.
>
> On the other hand, in sched_rtds_vcpus_params_set_all(), since the
> semantic is "use this set of params for all vcpus", I think it would be
> fine to enforce scinfo->num_vcpus == 1 (and maybe even
> scinfo.vcpus[0].vcpuid == LIBXL_SCHED_PARAM_VCPU_INDEX_DEFAULT).
>
>
> Now, for external callers (like xl, but also like any other toolstack
> wanting to build on top of libxl).
>
> If you think a 'set all vcpus' function would be useufl (as it is
> probably the case), you can define a libxl API function called
> libxl_vcpus_params_set_all(), doing exactly the same thing that
> libxl_vcpus_params_set() is doing, but calling the
> sched_rtds_vcpus_params_set_all() internal function.
>
> Chong, do you think this could work?

I think it would work. Thanks for this suggestion.

Chong



-- 
Chong Li
Department of Computer Science and Engineering
Washington University in St.louis

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel