Re: [Xen-devel] [PATCH V5 09/12] xen/vm_event: Decouple vm_event and mem_access.

[Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx>]

On Tue, Feb 17, 2015 at 3:17 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>> On 13.02.15 at 17:33, <tamas.lengyel@xxxxxxxxxxxx> wrote:
>> @@ -1293,56 +1293,30 @@ int p2m_mem_paging_prep(struct domain *d, unsigned 
>> long gfn, uint64_t buffer)
>>   *
>>   * If the gfn was dropped the vcpu needs to be unpaused.
>>   */
>> -void p2m_mem_paging_resume(struct domain *d)
>> +
>> +void p2m_mem_paging_resume(struct domain *d, vm_event_response_t *rsp)
>>  {
>>      struct p2m_domain *p2m = p2m_get_hostp2m(d);
>> -    vm_event_response_t rsp;
>>      p2m_type_t p2mt;
>>      p2m_access_t a;
>>      mfn_t mfn;
>>
>> -    /* Pull all responses off the ring */
>> -    while( vm_event_get_response(d, &d->vm_event->paging, &rsp) )
>> +    /* Fix p2m entry if the page was not dropped */
>> +    if ( !(rsp->u.mem_paging.flags & MEM_PAGING_DROP_PAGE) )
>>      {
>> -        struct vcpu *v;
>> -
>> -        if ( rsp.version != VM_EVENT_INTERFACE_VERSION )
>> -        {
>> -            printk(XENLOG_G_WARNING "vm_event interface version 
>> mismatch\n");
>> -            continue;
>> -        }
>> -
>> -#ifndef NDEBUG
>> -        if ( rsp.flags & VM_EVENT_FLAG_DUMMY )
>> -            continue;
>> -#endif
>> -
>> -        /* Validate the vcpu_id in the response. */
>> -        if ( (rsp.vcpu_id >= d->max_vcpus) || !d->vcpu[rsp.vcpu_id] )
>> -            continue;
>> -
>> -        v = d->vcpu[rsp.vcpu_id];
>> -
>> -        /* Fix p2m entry if the page was not dropped */
>> -        if ( !(rsp.u.mem_paging.flags & MEM_PAGING_DROP_PAGE) )
>> +        unsigned long gfn = rsp->u.mem_access.gfn;
>> +        gfn_lock(p2m, gfn, 0);
>
> Once again - blank line between declarations and statements please.
>
>> +        mfn = p2m->get_entry(p2m, gfn, &p2mt, &a, 0, NULL);
>> +        /* Allow only pages which were prepared properly, or pages which
>> +         * were nominated but not evicted */
>
> Coding style.
>
>> @@ -48,15 +46,15 @@ bool_t vm_event_check_ring(struct vm_event_domain *med);
>>   * succeed.
>>   */
>>  int __vm_event_claim_slot(struct domain *d, struct vm_event_domain *med,
>> -                            bool_t allow_sleep);
>> +                          bool_t allow_sleep);
>>  static inline int vm_event_claim_slot(struct domain *d,
>> -                                        struct vm_event_domain *med)
>> +                                      struct vm_event_domain *med)
>>  {
>>      return __vm_event_claim_slot(d, med, 1);
>>  }
>>
>>  static inline int vm_event_claim_slot_nosleep(struct domain *d,
>> -                                        struct vm_event_domain *med)
>> +                                              struct vm_event_domain *med)
>
> All these whitespace changes here and further down don't really
> belong in this patch - please again get this right when adding the
> code.

Same issue I mentioned in the other patch: git -M can't track the
files if indentation is fixed as part of the renaming process. As I
end up touching all the files that have with minor style issues like
this in the series as a result of the renaming, I fix them as I go
along. If that stretches the rules, I will need to add a whole new
separate patch just for indentation fixing.

>
> Jan

Thanks,
Tamas

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