Re: [Xen-devel] [PATCH V7 07/12] xen: Introduce monitor_op domctl

[Tamas Lengyel <tamas.lengyel@xxxxxxxxxxxx>]

On Thu, Mar 26, 2015 at 1:55 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:

>>> On 26.03.15 at 13:30, <tamas.lengyel@xxxxxxxxxxxx> wrote:
> On Thu, Mar 26, 2015 at 11:50 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:

>>>>> On 12.03.15 at 18:58, <tamas.lengyel@xxxxxxxxxxxx> wrote:
>>> @@ -91,41 +88,55 @@ static int hvm_event_traps(uint64_t parameters, vm_event_request_t *req)
>>>Â Â Â return 1;
>>>Â }
>>>
>>> -static void hvm_event_cr(uint32_t reason, unsigned long value,
>>> -Â Â Â Â Â Â Â Â Â Â Â Â Âunsigned long old, uint64_t parameters)
>>> +static inline
>>> +void hvm_event_cr(uint32_t reason, unsigned long value,
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Âunsigned long old, bool_t onchangeonly, bool_t sync)
>>>Â {
>>> -Â Â vm_event_request_t req = {
>>> -Â Â Â Â .reason = reason,
>>> -Â Â Â Â .vcpu_id = current->vcpu_id,
>>> -Â Â Â Â .u.mov_to_cr.new_value = value,
>>> -Â Â Â Â .u.mov_to_cr.old_value = old
>>> -Â Â };
>>> -
>>> -Â Â if ( (parameters & HVMPME_onchangeonly) && (value == old) )
>>> +Â Â if ( onchangeonly && value == old )
>>> +Â Â {
>>>Â Â Â Â Â return;
>>> -
>>> -Â Â hvm_event_traps(parameters, &req);
>>> +Â Â }
>>> +Â Â else
>>> +Â Â {
>>> +Â Â Â Â vm_event_request_t req = {
>>> +Â Â Â Â Â Â .reason = reason,
>>> +Â Â Â Â Â Â .vcpu_id = current->vcpu_id,
>>> +Â Â Â Â Â Â .u.mov_to_cr.new_value = value,
>>> +Â Â Â Â Â Â .u.mov_to_cr.old_value = old
>>> +Â Â Â Â };
>>> +
>>> +Â Â Â Â hvm_event_traps(sync, &req);
>>> +Â Â }
>>
>> ... I'd really like to see such done without "else" (which would also
>> have resulted in a smaller change).
>>
>

> So the reason why I have it under an else clause so that the
> vm_event_request_t only gets pushed on the stack if there is a need
> for it. Otherwise it would be pushed on the stack even if the function
> returns right away which IMHO is not needed.

How much of that pushing actually happens before reaching the
early return depends entirely on the compiler. Just the other day
we had a similar discussion on another thread (I think it was with
Boris).

Jan

Right, but IMHO this way it's at least clear what the developer's intention would be in this case, which is not doing the push unless needed. How the compiler orders these instructions and then how the CPU actually executes the instructions is another matter..

Tamas

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