Re: [PATCH v8 for Xen 4.7 1/4] xen: enable per-VCPU parameter for RTDS

[Chong Li <lichong659@gmail.com>]

On Mon, Mar 21, 2016 at 11:25 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 21.03.16 at 17:03, <lichong659@gmail.com> wrote:
>> On Mon, Mar 21, 2016 at 10:49 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 21.03.16 at 16:18, <lichong659@gmail.com> wrote:
>>>> On Mon, Mar 21, 2016 at 8:35 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>>> On 18.03.16 at 22:26, <lichong659@gmail.com> wrote:
>>>>>> --- a/xen/include/public/domctl.h
>>>>>> +++ b/xen/include/public/domctl.h
>>>>>> @@ -338,24 +338,64 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_max_vcpus_t);
>>>>>>  #define XEN_SCHEDULER_ARINC653 7
>>>>>>  #define XEN_SCHEDULER_RTDS     8
>>>>>>
>>>>>> -/* Set or get info? */
>>>>>> +typedef struct xen_domctl_sched_credit {
>>>>>> +    uint16_t weight;
>>>>>> +    uint16_t cap;
>>>>>> +} xen_domctl_sched_credit_t;
>>>>>> +
>>>>>> +typedef struct xen_domctl_sched_credit2 {
>>>>>> +    uint16_t weight;
>>>>>> +} xen_domctl_sched_credit2_t;
>>>>>> +
>>>>>> +typedef struct xen_domctl_sched_rtds {
>>>>>> +    uint32_t period;
>>>>>> +    uint32_t budget;
>>>>>> +} xen_domctl_sched_rtds_t;
>>>>>> +
>>>>>> +typedef struct xen_domctl_schedparam_vcpu {
>>>>>> +    union {
>>>>>> +        xen_domctl_sched_credit_t credit;
>>>>>> +        xen_domctl_sched_credit2_t credit2;
>>>>>> +        xen_domctl_sched_rtds_t rtds;
>>>>>> +    } u;
>>>>>> +    uint32_t vcpuid;
>>>>>> +    uint16_t padding[2];
>>>>>
>>>>> So why uint16_t[2] instead of just uint32_t? And what's the
>>>>> padding needed for in the first place?
>>>>
>>>> You're right. It's better to use uint32_t, which pads (the struct) to
>>>> the 64-bit boundary.
>>>
>>> Which doesn't answer the "why in the first place" part - I don't
>>> see why structure size needs to be a multiple of 64 bits.
>>>
>> In this patch post,
>>
>> http://lists.xen.org/archives/html/xen-devel/2015-07/msg02334.html
>>
>> you had a comment about the structure size, which I think you mean
>> the struct size should be a multiple of 64 bits.
>
> Looks like I had got mislead by there being struct
> xen_domctl_sched_sedf, but it not being part of the union. I'm
> sorry for that.
>
Ok. I've already fixed all problems pointed out by George and Jan.

Dario and Meng, do you have any other comments on this patch?

Chong



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

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