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

[Tamas K Lengyel <tamas.lengyel@zentific.com>]

On Thu, Mar 26, 2015 at 11:50 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 12.03.15 at 18:58, <tamas.lengyel@zentific.com> 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.

Tamas