Re: [PATCH v2 1/7] xen/mm: lift 32 item limit from mfn/gfn arrays

[Michał Leszczyński <michal.leszczynski@xxxxxxx>]

----- 19 cze 2020 o 13:48, Jan Beulich jbeulich@xxxxxxxx napisał(a):

> On 19.06.2020 13:36, Michał Leszczyński wrote:
>> ----- 19 cze 2020 o 13:34, Roger Pau Monné roger.pau@xxxxxxxxxx napisał(a):
>> 
>>> On Fri, Jun 19, 2020 at 01:38:00AM +0200, Michał Leszczyński wrote:
>>>> Replace on-stack array allocation with heap allocation
>>>> in order to lift the limit of 32 items in mfn/gfn arrays
>>>> when calling acquire_resource.
>>>
>>> I'm afraid this is not correct, you cannot allow unbounded amounts of
>>> items to be processed like this, it's likely that you manage to
>>> trigger the watchdog if the list is long enough, specially when doing
>>> set_foreign_p2m_entry.
>>>
>>> You need to process the items in batches (32 was IMO a good start), and
>>> then add support for hypercall continuations. Take a look at how
>>> XENMEM_populate_physmap just a couple of lines below makes use of
>>> hypercall_create_continuation.
>>>
>>> After processing every batch you need to check if
>>> hypercall_preempt_check returns true and if so use
>>> hypercall_create_continuation in order to encode a continuation.
>>>
>>> Thanks, Roger.
>> 
>> 
>> Somebody previously suggested that this limit could be lifted this way,
>> so I would like to hear some more opinions on that.
> 
> I did suggest the limit can be lifted, but not by processing all
> pieces in one go. Whether batches of 32 or 64 or 128 are chosen
> is a different thing, but you can't do arbitrary amounts without
> any preemption checks.
> 
> Jan


Okay. I will try to correct it within v3.

Best regards,
Michał Leszczyński
CERT Polska