Hi Jan,
> -----Original Message-----
> From: Jan Beulich <jbeul...@suse.com>
> Subject: Re: [PATCH v2] xen/arm: p2m: Populate pages for GICv2 mapping in
> arch_domain_create()
>
> 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 )
Kind regards,
Henry
>
> Jan
