Re: [Xen-devel] [PATCH] Improve performance of IOCTL_PRIVCMD_MMAPBATCH_V2

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.11.12 at 11:51, Mats Petersson <mats.petersson@xxxxxxxxxx> wrote:
> On 21/11/12 10:29, Jan Beulich wrote:
>>>>> On 16.11.12 at 15:45, Mats Petersson <mats.petersson@xxxxxxxxxx> wrote:
>>> @@ -2526,12 +2540,25 @@ int xen_remap_domain_mfn_range(struct 
>>> vm_area_struct 
> *vma,
>>>             if (err)
>>>                     goto out;
>>>   
>>> -           err = HYPERVISOR_mmu_update(mmu_update, batch, NULL, domid);
>>> -           if (err < 0)
>>> -                   goto out;
>>> +           /* We record the error for each page that gives an error, but
>>> +            * continue mapping until the whole set is done */
>>> +           do {
>>> +                   err = HYPERVISOR_mmu_update(&mmu_update[index],
>>> +                                               batch_left, &done, domid);
>>> +                   if (err < 0) {
>>> +                           if (err_ptr)
>>> +                                   err_ptr[index] = err;
>> Shouldn't you increment "done" here, in order to not retry the failed
>> slot immediately?
> 
> Yes, good spot - for some reason, I have double checked the behaviour of 
> "done", and it returns the index of the item which gave the error, not 
> actually "how many were processed".

Isn't the index of failure equal to the number of successfully
processed ones?

> I have rewritten this part of code for V3 of this patch, but I think it 
> still requires an increment of "done" to make it work correctly.
>>
>>> +                           else /* exit if error and no err_ptr */
>>> +                                   goto out;
>>> +                   }
>>> +                   batch_left -= done;
>>> +                   index += done;
>>> +           } while (batch_left);
>>>   
>>>             nr -= batch;
>>>             addr += range;
>>> +           if (err_ptr)
>>> +                   err_ptr += batch;
>>>     }
>>>   
>>>     err = 0;
>>> @@ -303,6 +349,8 @@ static int mmap_return_errors_v1(void *data, void 
>>> *state)
>>>     return __put_user(*mfnp, st->user_mfn++);
>>>   }
>>>   
>>> +
>>> +
>> ???
> This is cleaned up in the V3 patch.

Did I miss that (and v2 too)?

Jan


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