Re: [PATCH 2/6] tools: arm: allocate superpages to guests.

[Julien Grall <julien.grall@linaro.org>]

On 06/11/2014 12:55 PM, Ian Campbell wrote:
> On Wed, 2014-06-11 at 12:50 +0100, Julien Grall wrote:
>> On 06/10/2014 04:16 PM, Ian Campbell wrote:
>>>>> +/*  >0: success, *nr_pfns set to number actually populated
>>>>> + *   0: didn't try with this pfn shift (e.g. misaligned base etc)
>>>>> + *  <0: ERROR
>>>>> + */
>>>>> +static int populate_one_size(struct xc_dom_image *dom, int pfn_shift,
>>>>> +                             xen_pfn_t base_pfn, xen_pfn_t *nr_pfns)
>>>>> +{
>>>>> +    uint64_t mask = ((uint64_t)1<<(pfn_shift))-1;
>>>>> +    uint64_t next_mask = ((uint64_t)1<<(LPAE_SHIFT))-1;
>>>>> +    int nr, i, count = min_t(int, 1024*1024, *nr_pfns >> pfn_shift);
>>>>
>>>> Stupid question, where does come from the 1024*1024?
>>>
>>> It was in the original as a backstop on allocsz. It would correspond to
>>> 4GB worth of 4K page I suppose
>>
>> Ah ok. I didn't pay attention about it.
>>
>>>
>>>>> +    xen_pfn_t extents[count];
>>>>
>>>> If I follow the count definition, it's possible to allocate 1024*1024
>>>> xen_pfn_t (about 8MB) on the stack.
>>>
>>> userspace stack isn't all that precious but 8MB does seem a little
>>> excessive. Previously this was using the already allocated host p2m so
>>> it wasn't an issue, but that doesn't work for allocations of page >4K.
>>>
>>> The tradeoff is that smaller batches mean it will take longer.
>>>
>>> I don't think it would be unreasonable to reduce this to be e.g. 1GB
>>> worth of entries (256*1024) or 2MB of stack.
>>
>> It seems the default stack size is 8MB, I'm wondering if we can have
>> some use case where this limit is smaller.
> 
> I think we effectively assume it is larger than any amount we would
> practically use.
> 
>> Is there any issue to allocate this variable with malloc?
> 
> Just more book keeping code really.

Looking to the code, it doesn't seem too difficult to add malloc and
handle error.

I would prefer to use the malloc rather the stack and therefore assuming
that's stack a quite big, even though this variable should never be too
big. It wouldn't be mad to have an OS using a 1M stack (or even less).

Anyway it seems that x86 libxc is using the same trick for superpage...

Regards,

-- 
Julien Grall