On 14.10.2022 12:53, Henry Wang wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeul...@suse.com>
>>
>> On 14.10.2022 12:38, Henry Wang wrote:
>>>> -----Original Message-----
>>>> From: Julien Grall <jul...@xen.org>
>>>>>>> + if ( d->arch.paging.p2m_total_pages != 0 )
>>>>>>> + {
>>>>>>> + spin_lock(&d->arch.paging.lock);
>>>>>>> + p2m_set_allocation(d, 0, NULL);
>>>>>>> + spin_unlock(&d->arch.paging.lock);
>>>>>>> + ASSERT(d->arch.paging.p2m_total_pages == 0);
>>>>>>> + }
>>>>>>
>>>>>> Is it intentional to largely open-code p2m_teardown_allocation() here?
>>>>>
>>>>> Yes, AFAICT p2m_teardown_allocation() is preemptible and we don't
>> want
>>>>> any preemption here.
>>>>
>>>> Like Jan, I would prefer if we can avoid the duplication. The loop
>>>> suggested by Jan should work.
>>>
>>> I am a little bit worried about the -ENOMEM, if -ENOMEM is
>>> returned from p2m_teardown_allocation(d), I think we are in
>>> the infinite loop, or did I miss understood the loop that Jan referred
>>> to?
>>
>> Where would -ENOMEM come from? We're firmly freeing memory here. -
>> ENOMEM
>> can only occur for a non-zero 2nd argument.
>
> My initial thought is the "else if" part in p2m_set_allocation. It might be
> wrong. Would the code below seems ok to you?
>
> int err;
>
> do {
> err = p2m_teardown_allocation(d)
> } while ( err == -ERESTART )
Sure, one of several ways of doing it.
Jan
